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

roachtest: add ability to perform multiple upgrades in mixedversion #108773

Merged

Conversation

renatolabs
Copy link
Contributor

This commit adds the ability for mixedversion tests to perform
multiple upgrades in certain tests runs. It has been observed that
certain types of errors only manifest when features are enabled in
older releases, and this makes it possible for us to get some coverage
for these scenarios.

By default, mixedversion tests will perform a random number of
upgrades (up to a maximum of 3). Test authors are able to override
this default by providing a minimum, maximum, or exact number of
upgrades test runs should go through.

Epic: CRDB-19321

Release note: None

@renatolabs renatolabs added the backport-23.1.x Flags PRs that need to be backported to 23.1 label Aug 15, 2023
@renatolabs renatolabs requested a review from a team as a code owner August 15, 2023 13:58
@renatolabs renatolabs requested review from rachitgsrivastava and DarrylWong and removed request for a team August 15, 2023 13:58
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@renatolabs renatolabs requested review from herkolategan and srosenberg and removed request for rachitgsrivastava and DarrylWong August 15, 2023 14:00
@renatolabs
Copy link
Contributor Author

renatolabs commented Aug 15, 2023

There's a likely legit bug that causes backup-restore/mixed-version to fail on this branch when a RESTORE is attempted while the cluster is finalizing the 22.1 -> 22.2 upgrade:

[mixed-version-test/20_run-verify-some-backups] 01:51:05 runner.go:336: mixed-version test failure while running step 20 (run "verify some backups"): mixed-version: waiting for job to finish: job 891269071918759937 failed with error: version mismatch for descriptor 132, expected version 2, got 1

I'll let @cockroachdb/disaster-recovery deal with this failure as they see fit when it happens in the nightly build.

Copy link
Contributor

@smg260 smg260 left a comment

Choose a reason for hiding this comment

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

The feature generally makes sense to me.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @herkolategan, @renatolabs, and @srosenberg)


pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion.go line 929 at r1 (raw file):

	if test.options.minUpgrades > test.options.maxUpgrades {
		err := fmt.Errorf(
			"invalid test options: minUpgrades=%d maxUpgrades=%d",

minor: might make sense to be explicit about the error here.

e.g. maxUpgrades (%d) must be greater than minUpgrades (%d)

@renatolabs renatolabs force-pushed the rc/mixedversion-multiple-upgrades branch from dc5456a to 48d5fb6 Compare August 22, 2023 14:23
Copy link
Contributor Author

@renatolabs renatolabs left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @herkolategan, @smg260, and @srosenberg)


pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion.go line 929 at r1 (raw file):

Previously, smg260 (Miral Gadani) wrote…

minor: might make sense to be explicit about the error here.

e.g. maxUpgrades (%d) must be greater than minUpgrades (%d)

Makes sense, done.

This commit adds the ability for `mixedversion` tests to perform
multiple upgrades in certain tests runs. It has been observed that
certain types of errors only manifest when features are enabled in
older releases, and this makes it possible for us to get some coverage
for these scenarios.

By default, `mixedversion` tests will perform a random number of
upgrades (up to a maximum of 3). Test authors are able to override
this default by providing a minimum, maximum, or exact number of
upgrades test runs should go through.

Epic: CRDB-19321

Release note: None
Certain features and system tables used in the test are only available
in v22.2+. In addition, we are only validating our ability to restore
backups taken in the previous version for now.

Epic: CRDB-19321

Release note: None
@renatolabs renatolabs force-pushed the rc/mixedversion-multiple-upgrades branch from 48d5fb6 to 2b00746 Compare August 22, 2023 17:43
We bump this test's timeout since it can now go through up to 2
upgrades in a test run. In the future, we'll allow the test to perform
more upgrades, but more work is needed to identify known limitations
in 22.1

Epic: CRDB-19321

Release note: None
@renatolabs
Copy link
Contributor Author

Added another commit to adapt the new cdc/mixed-versions to multiple upgrades (also updated test ownership back to CDC; always happy to help debug mixed-version issues, but it makes more sense for cdc to own that test).

@jayshrivastava FYI.

@renatolabs
Copy link
Contributor Author

TFTR!

bors r=smg260

@craig
Copy link
Contributor

craig bot commented Aug 23, 2023

Build succeeded:

@craig craig bot merged commit 1c1f08d into cockroachdb:master Aug 23, 2023
@blathers-crl
Copy link

blathers-crl bot commented Aug 23, 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 2b00746 to blathers/backport-release-23.1-108773: 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.

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.

3 participants