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

upgrades: fix race condition inside TestUpgradeSchemaChangerElements #97359

Merged

Conversation

fqazi
Copy link
Collaborator

@fqazi fqazi commented Feb 20, 2023

Previously, this test non-atomically update the job ID, which could lead to race conditions if jobs queries ran for any other reason than our expected one. This was inadequate because the race detector would detect issues with this value being modified. To address this, this patch only stores/loads the job ID atomically.

Fixes: #97284

Release note: None

@fqazi fqazi requested a review from a team February 20, 2023 14:22
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@chengxiong-ruan chengxiong-ruan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM mode the lint issue.

Previously, this test non-atomically update the job ID, which
could lead to race conditions if jobs queries ran for any other
reason than our expected one. This was inadequate because
the race detector would detect issues with this value being
modified. To address this, this patch only stores/loads the job ID
atomically.

Fixes: cockroachdb#97284

Release note: None
@fqazi fqazi force-pushed the TestUpgradeSchemaChangerElementsRace branch from 436be6d to 560c86e Compare February 21, 2023 19:59
@fqazi
Copy link
Collaborator Author

fqazi commented Feb 21, 2023

@chengxiong-ruan TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Feb 21, 2023

Build succeeded:

@craig craig bot merged commit 46631a4 into cockroachdb:master Feb 21, 2023
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.

upgrade/upgrades: TestUpgradeSchemaChangerElements failed
3 participants