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

clusterversion: mint 21.2 cluster version #69826

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

celiala
Copy link
Collaborator

@celiala celiala commented Sep 3, 2021

Shortly after Creating a release branch for release-21.2, we'll want to merge PRs as instructed by the New major version checklist.

This PR is for Step 1a of the New major version checklist, where we add a cluster version Start${vBRANCH_CUT} for the corresponding .0 release.

This PR is one of the tasks detailed in #70751, which tracks all the steps relevant to creating a release branch for ${vBRANCH_CUT} and preparing master for the ${vNEXT} major version.

Release justification: Non-production code change.
Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@celiala
Copy link
Collaborator Author

celiala commented Sep 3, 2021

This should not be merged until after Tuesday's branch cut.

Updated: These should NOT be merged until we've released a beta

@@ -90,6 +90,7 @@ func (kv keyedVersions) Validate() error {
// In the above example, we would tolerate 20.1-x but not 19.2-x.
// Currently we're actually a few versions behind in enforcing a ban on old
// versions/migrations. See #47447.
// TODO(celia) should 5 be updated back to 4?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TODO(celia) - who might know the answer to this?

@@ -1,4 +1,5 @@
// Code generated by "stringer -type=Key"; DO NOT EDIT.
// TODO(celia) -- how do I re-autogenerate this file?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TODO(celia) - who might know the answer to this?

@celiala celiala force-pushed the mint-21.2-cluster-version branch from 3c84787 to db5e675 Compare September 8, 2021 21:41
@celiala celiala requested review from knz and dt September 8, 2021 22:35
@dt
Copy link
Member

dt commented Sep 8, 2021

I think we need to hold off on minting 21.2 much longer than just the branch cut, and instead until we have a released beta that is selected for our planned internal deployment -- once we mint the version, we cannot add new migrations/version gates under it without requiring a wipe of any clusters that ran a build including the mint.

@celiala
Copy link
Collaborator Author

celiala commented Sep 9, 2021

I think we need to hold off on minting 21.2 much longer than just the branch cut, and instead until we have a released beta that is selected for our planned internal deployment

Ack: I don't merge this until we've released a beta. Thanks!

I've updated the PR note and wiki runbook accordingly:

**This should NOT be merged until we've released a beta ** (adding do-not-merge until this happens)

Copy link
Contributor

@irfansharif irfansharif left a comment

Choose a reason for hiding this comment

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

Code changes here LGTM (deferring to y'all for when this is to be merged).

Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 7 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner, @celiala, @dt, and @irfansharif)


pkg/clusterversion/key_string.go, line 2 at r1 (raw file):

Previously, celiala wrote…

TODO(celia) - who might know the answer to this?

answer to what?


pkg/clusterversion/keyed_versions.go, line 93 at r1 (raw file):

Previously, celiala wrote…

TODO(celia) - who might know the answer to this?

I believe this issue has been fixed. I think @dt or @irfansharif can confirm.

irfansharif added a commit to irfansharif/cockroach that referenced this pull request Sep 20, 2021
This reverts commit 6464de2. That PR
broke few of our roachtests since we haven't release the 21.2 beta yet.
For our roachtests that exercised the upgrade path, we were effectively
upgrading from 21.1 to 22.1 code (as of that PR) that asserted on the
completion of the long running migration removing the legacy raft
truncated state -- something that would only happen when going through
21.2. Given that, we temporarily revert cockroachdb#69887 while our beta gets
prepared. cockroachdb#69887 (or rather, the revert of this commit) will be
re-introduced to master once cockroachdb#69826 lands.

Fixes cockroachdb#70244.
Fixes cockroachdb#70252.
Fixes cockroachdb#70253.
Fixes cockroachdb#70283.
Fixes cockroachdb#70350.
Fixes cockroachdb#70390.

Release note: None
irfansharif added a commit to irfansharif/cockroach that referenced this pull request Sep 20, 2021
This reverts commit 6464de2. That PR
broke few of our roachtests since we haven't release the 21.2 beta yet.
For our roachtests that exercised the upgrade path, we were effectively
upgrading from 21.1 to 22.1 code (as of that PR) that asserted on the
completion of the long running migration removing the legacy raft
truncated state -- something that would only happen when going through
21.2. Given that, we temporarily revert cockroachdb#69887 while our beta gets
prepared. cockroachdb#69887 (or rather, the revert of this commit) will be
re-introduced to master once cockroachdb#69826 lands.

Fixes cockroachdb#70244.
Fixes cockroachdb#70252.
Fixes cockroachdb#70253.
Fixes cockroachdb#70283.
Fixes cockroachdb#70350.
Fixes cockroachdb#70390.

Release note: None
craig bot pushed a commit that referenced this pull request Sep 20, 2021
70325: vendor: Add dependency on prometheus r=dhartunian a=rimadeodhar

This PR adds an external dependency on prometheus. We need
the promql library in order to enforce validity of promql
expressions which will be contained in upcoming alerting
and aggregation rules. These rule implementations are
upcoming as a part of the new metrics upgrade.

Resolves #69796

Release note: None

70347: pgcode: use XC instead of CDB r=knz a=otan

Release note (sql change): Change the pgerror code XC instead of CD
for CockroachDB specific errors. This is because the "C" class is
reserved for the SQL standard. The pgcode `CDB00` used for
unsatisfiable bounded staleness is now `XCUBS`.

70374: ui: updates the jobs table styling r=maryliag a=maryliag

This commit updates the style of the table on the Jobs page
and adds tooltips to its columns.

Resolves #70149

Before
<img width="924" alt="Screen Shot 2021-09-17 at 3 35 04 PM" src="https://user-images.githubusercontent.com/1017486/133844066-3168bec7-db52-4194-9f97-c7b10628d98e.png">

After
<img width="880" alt="Screen Shot 2021-09-17 at 3 35 52 PM" src="https://user-images.githubusercontent.com/1017486/133844146-86e94611-ca99-4764-a97e-c8ca4e09f269.png">


Release note (ui change): Updating job table style to
match all other tables on the console.

70432: Revert "kv,migration: rm code handling legacy raft truncated state" r=irfansharif a=irfansharif

This reverts commit 6464de2. That PR
broke few of our roachtests since we haven't release the 21.2 beta yet.
For our roachtests that exercised the upgrade path, we were effectively
upgrading from 21.1 to 22.1 code (as of that PR) that asserted on the
completion of the long running migration removing the legacy raft
truncated state -- something that would only happen when going through
21.2. Given that, we temporarily revert #69887 while our beta gets
prepared. #69887 (or rather, the revert of _this_ commit) will be
re-introduced to master once #69826 lands.

Fixes #70244.
Fixes #70252.
Fixes #70253.
Fixes #70283.
Fixes #70350.
Fixes #70390.

Release note: None

70436: spanconfig: fix an erroneous usage of timeutil.Timer r=irfansharif a=irfansharif

The contract for timeutil.Timer indicates that we should only be
setting .Read when reading from the timer channel, not unconditionally
before a call to .Reset().

Release note: None

Co-authored-by: rimadeodhar <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
Co-authored-by: Marylia Gutierrez <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
irfansharif added a commit to irfansharif/cockroach that referenced this pull request Sep 21, 2021
…ate"

This reverts commit ef1dd6f. cockroachdb#70432
reverted cockroachdb#69887, as temporary stop-gap until we release the first 21.2
beta. See the discussion over on cockroachdb#70432 for why we want to queue up this
revert to the original revert; this should only be merged after cockroachdb#69826
lands.

Release note: None
@celiala celiala force-pushed the mint-21.2-cluster-version branch 2 times, most recently from 3669b62 to 3d2804b Compare September 26, 2021 02:50
@celiala celiala removed the do-not-merge bors won't merge a PR with this label. label Sep 26, 2021
@celiala
Copy link
Collaborator Author

celiala commented Sep 26, 2021

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 26, 2021

🔒 Permission denied

Existing reviewers: click here to make celiala a reviewer

@RaduBerinde
Copy link
Member

Hi Celia,

CC is making some changes to the costing model and it requires a change to the tenant_usage table. Could you hold off 1/2 day on merging this change?

@celiala
Copy link
Collaborator Author

celiala commented Sep 27, 2021

CC is making some changes to the costing model and it requires a change to the tenant_usage table. Could you hold off 1/2 day on merging this change?

Ack. Will hold off until given green light to move forward, thanks! (re-adding do-not-merge label)

@celiala celiala added the do-not-merge bors won't merge a PR with this label. label Sep 27, 2021
@celiala celiala force-pushed the mint-21.2-cluster-version branch from 3d2804b to 558def7 Compare September 28, 2021 21:03
@RaduBerinde
Copy link
Member

My PR (#70778) is merged. Sorry it took longer than predicted.

Release justification: Non-production code change.
Release note: None
@celiala celiala force-pushed the mint-21.2-cluster-version branch from 558def7 to ea24703 Compare September 30, 2021 21:30
@celiala
Copy link
Collaborator Author

celiala commented Oct 1, 2021

TFTRs!

bors r+

@craig
Copy link
Contributor

craig bot commented Oct 1, 2021

👎 Rejected by label

@celiala celiala removed the do-not-merge bors won't merge a PR with this label. label Oct 1, 2021
@celiala
Copy link
Collaborator Author

celiala commented Oct 1, 2021

TFTRs!

bors r+

@craig
Copy link
Contributor

craig bot commented Oct 1, 2021

Build succeeded:

@craig craig bot merged commit 5f53feb into cockroachdb:master Oct 1, 2021
@blathers-crl
Copy link

blathers-crl bot commented Oct 1, 2021

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 ea24703 to blathers/backport-release-21.2-69826: 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 21.2.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@celiala
Copy link
Collaborator Author

celiala commented Oct 1, 2021

FYI - I'm now wondering aloud if I merged this too soon (this morning I just put 2+2 together around the context behind this change, as well as noticed one note that recommended we hold off under later -- perhaps until after an RC).

Because of that, I'll plan to hold off on backporting this until it seems like we have a pretty solid RC.

@dt
Copy link
Member

dt commented Oct 1, 2021

waiting to backport until there's an RC tagged sounds about right to me. once we backport and release a 21.2 beta/rc with this change, that'll close the door on any future version gates or migrations being backported to 21.2, so we tend to wait until we get to an RC aka zero known release or GA blockers, just in case the resolution for any of those blockers ends up needing a gate.

irfansharif added a commit to irfansharif/cockroach that referenced this pull request Oct 25, 2021
…ate"

This reverts commit ef1dd6f. cockroachdb#70432
reverted cockroachdb#69887, as temporary stop-gap until we release the first 21.2
beta. See the discussion over on cockroachdb#70432 for why we want to queue up this
revert to the original revert; this should only be merged after cockroachdb#69826
lands.

Release note: None
craig bot pushed a commit that referenced this pull request Oct 26, 2021
70464: Re-introduce "kv,migration: rm code handling legacy raft truncated st… r=irfansharif a=irfansharif

…ate"

This reverts commit ef1dd6f. #70432
reverted #69887, as temporary stop-gap until we release the first 21.2
beta. See the discussion over on #70432 for why we want to queue up this
revert to the original revert; this should only be merged after #69826
lands.

Release note: None

71962: backup: mark some settings public r=dt a=dt

This marks some of BACKUP's cluster settings as public as they are
intended for user-tuning to match their desired workload / requirements,
such as the delay before invoking priority reads or the target file size.

Release note (ops change): Some existing settings related to BACKUP execution are now listed  by SHOW CLUSTER SETTINGS.

Fixes #71786.

Co-authored-by: irfan sharif <[email protected]>
Co-authored-by: David Taylor <[email protected]>
@celiala celiala deleted the mint-21.2-cluster-version branch April 5, 2022 04:25
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.

6 participants