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

release-23.2: upgrade: use high priority txn's to update the cluster version #115034

Merged
merged 2 commits into from Nov 29, 2023

Conversation

fqazi
Copy link
Collaborator

@fqazi fqazi commented Nov 23, 2023

Backport 2/2 commits from #113996.

/cc @cockroachdb/release


Previously, it was possible for the leasing subsystem to starve out attempts to set the cluster version during upgrades, since the leasing subsystem uses high priority txn for renewals. To address this, this patch makes the logic to set the cluster version high priority so it can't be pushed out by lease renewals.

Fixes: #113908

Release note (bug fix): Addressed a bug that could cause cluster version finalization to get starved out by descriptor lease renewals on larger clusters.

Release justification: Low risk and forces direct use for KV primitives when updating the cluster version number. This code should be hit fairly frequently on the unit tests/ roach tests.

@fqazi fqazi requested a review from a team November 23, 2023 14:36
Copy link

blathers-crl bot commented Nov 23, 2023

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Backports should only be created for serious
    issues
    or test-only changes.
  • Backports should not break backwards-compatibility.
  • Backports should change as little code as possible.
  • Backports should not change on-disk formats or node communication protocols.
  • Backports should not add new functionality (except as defined
    here).
  • Backports must not add, edit, or otherwise modify cluster versions; or add version gates.
  • All backports must be reviewed by the owning areas TL and one additional
    TL. For more information as to how that review should be conducted, please consult the backport
    policy
    .
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters. State changes must be further protected such that nodes running old binaries will not be negatively impacted by the new state (with a mixed version test added).
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.
  • Your backport must be accompanied by a post to the appropriate Slack
    channel (#db-backports-point-releases or #db-backports-XX-X-release) for awareness and discussion.

Also, please add a brief release justification to the body of your PR to justify this
backport.

@blathers-crl blathers-crl bot added the backport Label PR's that are backports to older release branches label Nov 23, 2023
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@fqazi fqazi marked this pull request as ready for review November 27, 2023 16:04
@fqazi fqazi requested review from a team as code owners November 27, 2023 16:04
Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

lgtm! can you get the request in #db-backports-23-2-release ?

the CI needs fixing - but looks to be test only changes to use different variable names.

@fqazi
Copy link
Collaborator Author

fqazi commented Nov 27, 2023

lgtm! can you get the request in #db-backports-23-2-release ?

the CI needs fixing - but looks to be test only changes to use different variable names.

Should be fixed, let me post for this one too

@fqazi fqazi requested review from nvanbenschoten and ajstorm and removed request for ajstorm November 27, 2023 18:51
To be able to have minimal high priority txns for bumping cluster
version, we need the ability to write the setting table using KV calls.
This patch adds, the functions EncodeSettingKey and EncodeSettingValue
to allow directly generate values for Put/Get calls.

Release note: None
Previously, it was possible for the leasing subsystem to starve out
attempts to set the cluster version during upgrades, since the leasing
subsystem uses high priority txn for renewals. To address this, this
patch makes the logic to set the cluster version high priority so
it can't be pushed out by lease renewals.

Fixes: cockroachdb#113908

Release note (bug fix): Addressed a bug that could cause cluster version
finalization to get starved out by descriptor lease renewals on larger
clusters.
@fqazi fqazi merged commit 6517bb1 into cockroachdb:release-23.2 Nov 29, 2023
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Label PR's that are backports to older release branches
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants