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

upgrademanager: deflake TestConcurrentMigrationAttempts #104601

Merged

Conversation

rafiss
Copy link
Collaborator

@rafiss rafiss commented Jun 8, 2023

fixes #101021

This test was flaky since due to a pretty edge-casey interaction with the upgrade that adds the system.job_info table. It does not seem worth debugging that interaction further, since the purpose of this test is to test the concurrency of the upgrade framework, not any specific upgrade. In fact, the only reason this test uses "real" upgrades is due to a limitation in the MakeTestingClusterSettingsWithVersions function.

So instead, we'll just change the test so it doesn't run that problematic upgrade.

Release note: None

This test was flaky since due to a pretty edge-casey interaction with
the upgrade that adds the system.job_info table. It does not seem worth
debugging that interaction further, since the purpose of this test is to
test the concurrency of the upgrade framework, not any specific upgrade.
In fact, the only reason this test uses "real" upgrades is due to a
limitation in the `MakeTestingClusterSettingsWithVersions` function.

So instead, we'll just change the test so it doesn't run that
problematic upgrade.

Release note: None
@rafiss rafiss requested a review from chengxiong-ruan June 8, 2023 15:29
@rafiss rafiss requested a review from a team as a code owner June 8, 2023 15:29
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rafiss rafiss added the backport-23.1.x Flags PRs that need to be backported to 23.1 label Jun 8, 2023
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!

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!

@rafiss
Copy link
Collaborator Author

rafiss commented Jun 8, 2023

tftr!

bors r+

@craig
Copy link
Contributor

craig bot commented Jun 8, 2023

Build failed:

@rafiss
Copy link
Collaborator Author

rafiss commented Jun 8, 2023

bors r+

@craig
Copy link
Contributor

craig bot commented Jun 8, 2023

Timed out.

@rafiss
Copy link
Collaborator Author

rafiss commented Jun 8, 2023

bors r+

@craig
Copy link
Contributor

craig bot commented Jun 8, 2023

Build succeeded:

@craig craig bot merged commit 84e7dd8 into cockroachdb:master Jun 8, 2023
@blathers-crl
Copy link

blathers-crl bot commented Jun 8, 2023

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 6b9ec05 to blathers/backport-release-23.1-104601: 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 23.1.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@rafiss rafiss deleted the fix-TestConcurrentMigrationAttempts branch June 9, 2023 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-23.1.x Flags PRs that need to be backported to 23.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pkg/upgrade/upgrademanager/upgrademanager_test: TestConcurrentMigrationAttempts failed
3 participants