Skip to content

clusterversion: bump minBinary to 22.2#96656

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
celiala:diff.22_1-vs-22_2
Feb 7, 2023
Merged

clusterversion: bump minBinary to 22.2#96656
craig[bot] merged 1 commit intocockroachdb:masterfrom
celiala:diff.22_1-vs-22_2

Conversation

@celiala
Copy link
Copy Markdown
Collaborator

@celiala celiala commented Feb 6, 2023

This PR bumps binaryMinSupportedVersion to 22.2 before cutting the 23.1
release branch, as part of the stability period process.

In previous stability periods, the work of deleting pre-22.2 gates/tests
and bumping binaryMinSupportedVersion was done at the same time. For 23.1,
we will break this up into smaller tasks:

  • This PR will just bump binaryMinSupportedVersion to 22.2, skipping 22.2
    mixed-version tests as needed to bump this value.
  • The clean-up of pre-22.2 gates and 22.2 mixed-version tests will be done
    via follow-up work/issues.

Issue: #95530
Epic: CRDB-23572

Release note: None

@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Feb 6, 2023

This change is Reviewable

@RaduBerinde
Copy link
Copy Markdown
Member

RaduBerinde commented Feb 7, 2023

The version upgrade test fails with:

failed to create engines: store last used with cockroach version v22.2 is too old for running version v1000022.2-36 (which requires data from v1000022.2 or later)

This is caused by the 1000000 DevOffset. When we want to allow upgrades to dev (which is needed in upgrade tests), all keys other than the first non-primordial get offset. We want the min-binary-version = v22_2 key to not be offset, so it needs to be the first (non-primordial) key.

Here is what I think is needed:

  • remove all code that uses clusterversion.TODOPreV22_1
  • remove all keys before V22_2 (i.e. V22_1 and all V22_2foo)
  • add a clusterversion.TODOPreV22_2 and replace all occurrences of removed keys with this

Maybe @dt or @andreimatei can double check that this makes sense.

@RaduBerinde
Copy link
Copy Markdown
Member

RaduBerinde commented Feb 7, 2023

I opened #96701 for the clusterversion.TODOPreV22_1 removal.

@RaduBerinde
Copy link
Copy Markdown
Member

RaduBerinde commented Feb 7, 2023

We should also update the DevOffset -ing code to either not offset anything up to BinaryMinSupportedVersionKey, or assert that the first non-primordial key is BinaryMinSupportedVersionKey.

@RaduBerinde
Copy link
Copy Markdown
Member

RaduBerinde commented Feb 7, 2023

We should also update the DevOffset -ing code to either not offset anything up to BinaryMinSupportedVersionKey,

Opened #96702 for this. With that, we won't need to remove the older keys in the same change (thanks @dt for the suggestion!).

@RaduBerinde
Copy link
Copy Markdown
Member

RaduBerinde commented Feb 7, 2023 •
edited
Loading

@celiala #96702 merged, I think (and hope) the version upgrade failure will go away if you rebase.

@celiala celiala mentioned this pull request Feb 7, 2023
Closed
@celiala
Copy link
Copy Markdown
Collaborator Author

celiala commented Feb 7, 2023

@celiala #96702 merged, I think (and hope) the version upgrade failure will go away if you rebase.

Thank you for all the help unblocking this! Okay, I've rebased and will (hopefully) pass

@celiala celiala marked this pull request as ready for review February 7, 2023 18:32
@celiala celiala requested a review from a team February 7, 2023 18:32
@celiala celiala requested a review from a team as a code owner February 7, 2023 18:32
@celiala celiala requested a review from itsbilal February 7, 2023 18:32
@RaduBerinde
Copy link
Copy Markdown
Member

RaduBerinde commented Feb 7, 2023

acceptance/version-upgrade passed :)

Looks like we need to skip BenchmarkTimeBoundIterate for now.

@celiala celiala requested review from RaduBerinde and dt February 7, 2023 18:35
@celiala celiala force-pushed the diff.22_1-vs-22_2 branch 2 times, most recently from a234b9e to a8d485e Compare February 7, 2023 18:57
@celiala
clusterversion: bump minBinary to 22.2
76fc52c 
This PR bumps binaryMinSupportedVersion to 22.2 before cutting the 23.1
release branch, as part of the stability period process.

In previous stability periods, the work of deleting pre-22.2 gates/tests
and bumping binaryMinSupportedVersion was done at the same time. For 23.1,
we will break this up into smaller tasks:

- This PR will just bump binaryMinSupportedVersion to 22.2, skipping 22.2
  mixed-version tests as needed to bump this value.
- The clean-up of pre-22.2 gates and 22.2 mixed-version tests will be done
  via follow-up work/issues.

Issue: cockroachdb#95530
Epic: CRDB-23572

Release note: None
@celiala
Copy link
Copy Markdown
Collaborator Author

celiala commented Feb 7, 2023

The tests passed 🎉 -- thanks again for fixing @RaduBerinde!

This is RFAL!

RaduBerinde
RaduBerinde approved these changes Feb 7, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @dt and @itsbilal)

@celiala
Copy link
Copy Markdown
Collaborator Author

celiala commented Feb 7, 2023

TFTR!

bors r=RaduBerinde

This was referenced Feb 7, 2023
Closed
Closed
Open
Closed
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Feb 7, 2023

Build succeeded:

@craig craig bot merged commit 618ea05 into cockroachdb:master Feb 7, 2023
@renatolabs renatolabs mentioned this pull request Feb 15, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RaduBerinde RaduBerinde RaduBerinde approved these changes

@itsbilal itsbilal Awaiting requested review from itsbilal itsbilal is a code owner automatically assigned from cockroachdb/storage

@dt dt Awaiting requested review from dt

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@celiala @cockroach-teamcity @RaduBerinde