Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

importer: use ctx in progress push, use ctxgroup #93782

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

stevendanna
Copy link
Collaborator

@stevendanna stevendanna commented Dec 16, 2022

This ensures that goroutines started in the importer are shut down
when the processor is shut down.

While the previous comment indicates there is no need to wait on the
goroutine, that isn't true in the case of cancellation in which the
existing coordination is insufficient.

Informs #91418

Release note: None

@stevendanna stevendanna requested a review from a team December 16, 2022 10:38
@cockroach-teamcity
Copy link
Member

This change is Reviewable

This ensures that goroutines started in the importer are shut down
when the processor is shut down.

While the previous comment indicates there is no need to wait on the
goroutine, that isn't true in the case of cancellation in which the
existing coordination is insufficient.

Informs cockroachdb#91418

Release note: None
@adityamaru
Copy link
Contributor

Theres a couple of issues this will hoepfully close:
#92910
#91828

@stevendanna
Copy link
Collaborator Author

bors r=adityamaru

@craig
Copy link
Contributor

craig bot commented Dec 16, 2022

Build succeeded:

@craig craig bot merged commit ce2450e into cockroachdb:master Dec 16, 2022
@blathers-crl
Copy link

blathers-crl bot commented Dec 16, 2022

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from c75676d to blathers/backport-release-22.1-93782: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.1.x failed. See errors above.


error creating merge commit from c75676d to blathers/backport-release-22.2-93782: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.2.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@rafiss
Copy link
Collaborator

rafiss commented Dec 20, 2022

Just double checking if there are still plans to backport this?

stevendanna added a commit to stevendanna/cockroach that referenced this pull request Dec 20, 2022
This unskips the test and adds a memory limit. The test completed
1500+ runs under stress. I believe cockroachdb#93782 resolved the main source of
flakiness here.

Fixes cockroachdb#91828

Release note: None
stevendanna added a commit to stevendanna/cockroach that referenced this pull request Jan 3, 2023
This unskips the test and adds a memory limit. The test completed
1500+ runs under stress. I believe cockroachdb#93782 resolved the main source of
flakiness here.

Fixes cockroachdb#91828

Release note: None
craig bot pushed a commit that referenced this pull request Jan 3, 2023
94010: sql/importer: unskip TestCSVImportCanBeResumed r=rafiss a=stevendanna

This unskips the test and adds a memory limit. The test completed 1500+ runs under stress. I believe #93782 resolved the main source of flakiness here.

Fixes #91828

Release note: None

94345: storage: include range key stats in ScanStats r=erikgrinaker a=jbowens

Include the new, low-level Pebble range key iterator stats (introduced in cockroachdb/pebble#1871) in roachpb.ScanStats.

Informs #77580.
Close #93326.

Epic: None
Release note: None

94458: logictest: fix flakes from mixed version testing r=ZhouXing19 a=rafiss

fixes #92637

The main fix was to wait for each node to be reachable before beginning the upgrade process. This also includes a version bump that has better logging to make it easier to debug.

Release note: None

Co-authored-by: Steven Danna <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants