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.

Sign up for GitHub

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

Jump to bottom

clusterversion: tracking issue for improvements and cleanup #112629

Closed
11 of 13 tasks
RaduBerinde opened this issue Oct 18, 2023 · 3 comments
Closed
11 of 13 tasks

clusterversion: tracking issue for improvements and cleanup #112629

RaduBerinde opened this issue Oct 18, 2023 · 3 comments
Assignees
@RaduBerinde
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@RaduBerinde
Copy link
Member

RaduBerinde commented Oct 18, 2023 •
edited
Loading

  • Use an array for the versions table ([numKeys]roachpb.Version) for cleaner definitions (and fast lookup)
  • Deprecate ByKey() in favor of a Key.Version() method (to avoid the annoying clusterversion.ByKey(clusterversion.VFoo)
  • Separate the DevOffset code to a different file, improve documentation
  • Rename BinaryVersion to Latest, MinBinarySupportedVersion to MinSupported
  • In VersionHandle rename binaryVersion to maxVersion and binaryMinSupportedVersion to minVersion
  • Remove TestingBinaryVersion etc
  • Rework scheme for intermediary versions: clusterversion: rework scheme for intermediary versions #112708 [too disruptive]
  • Make tests around upgrade that check an expected version only look at the release series part (so they don't need fixing when we mint a final version)
  • Adjust cockroach cli initialization use the previous release instead of MinSupported unless skipping is enabled via env var; update upgrade roachtests
  • Add a special nightly build that runs unit tests with MinSupported bumped up (using a build tag) [not worth triaging test failures from flaky tests]
  • Make a command that can dump new "initial values" files (for pkg/sql/catalog/bootstrap/data/)
  • Avoid unmarshaling in the IsActive hot path (clusterversion: proto Unmarshal on each version check #113385)

And, after a good chunk of these are done:

  • Update/rewrite the release runbooks as an md file in the repo.

Jira issue: CRDB-32524
@RaduBerinde RaduBerinde added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Oct 18, 2023
@RaduBerinde RaduBerinde self-assigned this Oct 18, 2023
@RaduBerinde
Copy link
Member Author

CC @celiala @rail these are some improvements I hope to be able to make on master in the following month or so (depending on release planning and other commitments).

@RaduBerinde RaduBerinde mentioned this issue Oct 30, 2023
Merged
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Oct 30, 2023
@RaduBerinde
clusterversion: cockroach versions cleanup
5cce778 
This commit cleans up the code and constants in
`cockroach_versions.go`:
 - we separate out the "dev offset" code and improve the
   documentation;
 - we replace the key-to-version slice with a table (array). This
   makes the declaration much cleaner and makes the lookup easier.
 - we replace `ByKey()` with a method, to make the call site less
   messy; e.g. `clusterversion.ByKey(clusterversion.V23_2)` becomes
   `clusterversion.V23_2.Version()`;
 - we deprecate `BinaryMinSupportedVersionKey` in favor of `MinSupported`;
 - we deprecate `BinaryVersionKey` in favor of `Latest`;
 - we remove `roachpb.Version` "constants" in favor of using the
   `Version()` method.

Informs: cockroachdb#112629
Release note: None
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Oct 31, 2023
@RaduBerinde
clusterversion: reword latest/min supported versions in Handle
ec9d53c 
Rename "binary version" to "latest version" and "binary min supported
version" to "min supported version" in `clusterversion.Handle` and
related code.

Informs: cockroachdb#112629
Release note: None
@pav-kv
Copy link
Collaborator

pav-kv commented Nov 2, 2023

Would #113385 be in scope for this work?

@RaduBerinde
Copy link
Member Author

Sure, I added it to the list.

@RaduBerinde RaduBerinde mentioned this issue Nov 2, 2023
Closed
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Nov 2, 2023
@RaduBerinde
clusterversion: cockroach versions cleanup
f3964a0 
This commit cleans up the code and constants in
`cockroach_versions.go`:
 - we separate out the "dev offset" code and improve the
   documentation;
 - we replace the key-to-version slice with a table (array). This
   makes the declaration much cleaner and makes the lookup easier.
 - we replace `ByKey()` with a method, to make the call site less
   messy; e.g. `clusterversion.ByKey(clusterversion.V23_2)` becomes
   `clusterversion.V23_2.Version()`;
 - we deprecate `BinaryMinSupportedVersionKey` in favor of `MinSupported`;
 - we deprecate `BinaryVersionKey` in favor of `Latest`;
 - we remove `roachpb.Version` "constants" in favor of using the
   `Version()` method.

Informs: cockroachdb#112629
Release note: None
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Nov 2, 2023
@RaduBerinde
clusterversion: cockroach versions cleanup
3b67dcb 
This commit cleans up the code and constants in
`cockroach_versions.go`:
 - we separate out the "dev offset" code and improve the
   documentation;
 - we replace the key-to-version slice with a table (array). This
   makes the declaration much cleaner and makes the lookup easier.
 - we replace `ByKey()` with a method, to make the call site less
   messy; e.g. `clusterversion.ByKey(clusterversion.V23_2)` becomes
   `clusterversion.V23_2.Version()`;
 - we deprecate `BinaryMinSupportedVersionKey` in favor of `MinSupported`;
 - we deprecate `BinaryVersionKey` in favor of `Latest`;
 - we remove `roachpb.Version` "constants" in favor of using the
   `Version()` method.

Informs: cockroachdb#112629
Release note: None
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Nov 3, 2023
@RaduBerinde
clusterversion: cockroach versions cleanup
9f4a5d8 
This commit cleans up the code and constants in
`cockroach_versions.go`:
 - we separate out the "dev offset" code and improve the
   documentation;
 - we replace the key-to-version slice with a table (array). This
   makes the declaration much cleaner and makes the lookup easier.
 - we replace `ByKey()` with a method, to make the call site less
   messy; e.g. `clusterversion.ByKey(clusterversion.V23_2)` becomes
   `clusterversion.V23_2.Version()`;
 - we deprecate `BinaryMinSupportedVersionKey` in favor of `MinSupported`;
 - we deprecate `BinaryVersionKey` in favor of `Latest`;
 - we remove `roachpb.Version` "constants" in favor of using the
   `Version()` method.

Informs: cockroachdb#112629
Release note: None
@RaduBerinde RaduBerinde mentioned this issue Nov 3, 2023
Merged
craig bot pushed a commit that referenced this issue Nov 3, 2023
@RaduBerinde
Merge #113314
694861a 
113314: clusterversion: cockroach versions cleanup r=RaduBerinde a=RaduBerinde

This commit cleans up the code and constants in
`cockroach_versions.go`:
 - we separate out the "dev offset" code and improve the documentation;
 - we replace the key-to-version slice with a table (array). This makes the declaration much cleaner and makes the lookup easier.
 - we replace `ByKey()` with a method, to make the call site less messy, e.g. `clusterversion.ByKey(clusterversion.V23_2)` becomes `clusterversion.V23_2.Version()`;
 - we deprecate `BinaryMinSupportedVersionKey` in favor of `MinSupported`;
 - we deprecate `BinaryVersionKey` in favor of `Latest`;
 - we remove `roachpb.Version` "constants" in favor of using the `Version()` method.

Informs: #112629
Release note: None

Co-authored-by: Radu Berinde <[email protected]>
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Nov 3, 2023
@RaduBerinde
clusterversion: rename latest/min supported versions in Handle
0dcacbd 
Rename "binary version" to "latest version" and "binary min supported
version" to "min supported version" in `clusterversion.Handle` and
associated code.

Informs: cockroachdb#112629
Release note: None
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Nov 3, 2023
@RaduBerinde
clusterversion: rename latest/min supported versions in Handle
93ee559 
Rename "binary version" to "latest version" and "binary min supported
version" to "min supported version" in `clusterversion.Handle` and
associated code.

Informs: cockroachdb#112629
Release note: None
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Nov 3, 2023
@RaduBerinde
clusterversion: rename latest/min supported versions in Handle
82d5721 
Rename "binary version" to "latest version" and "binary min supported
version" to "min supported version" in `clusterversion.Handle` and
associated code.

Informs: cockroachdb#112629
Release note: None
craig bot pushed a commit that referenced this issue Nov 3, 2023
@RaduBerinde
Merge #113727
60aab2a 
113727: clusterversion: rename latest/min supported versions in Handle r=RaduBerinde a=RaduBerinde

Rename "binary version" to "latest version" and "binary min supported
version" to "min supported version" in `clusterversion.Handle` and
associated code.

This change is mechanical and is not intended to have any functional changes.

Informs: #112629
Release note: None

Co-authored-by: Radu Berinde <[email protected]>
@RaduBerinde RaduBerinde mentioned this issue Nov 4, 2023
Merged
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Nov 4, 2023
@RaduBerinde
sql: add crdb_internal.release_series builtin
34543c2 
This commit adds a built-in that resolves dev versions (e.g. 23.2-10)
to the corresponding release series (e.g. 24.1). This allows us to
clean up some tests and make them stable - they will no longer need to
be adjusted when minting a release.

Informs: cockroachdb#112629
Release note: None
craig bot pushed a commit that referenced this issue Nov 13, 2023
@RaduBerinde
Merge #114306
1938a24 
114306: upgrade: remove pre-23.1 upgrades r=RaduBerinde a=RaduBerinde

This commit removes all non-permanent 22.2->23.1 upgrades, which are
no longer necessary.

Informs: #112629

Epic: none
Release note: None

Co-authored-by: Radu Berinde <[email protected]>
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Nov 14, 2023
@RaduBerinde
upgrades: reorganize files
dcada42 
In this commit we rename files implementing permanent upgrades to
start with `permanent_` and files implementing 23.2 upgrades to start
with `v23_2_`. This will make it easier to remove deprecated upgrades
later.

Informs: cockroachdb#112629
Release note: None
craig bot pushed a commit that referenced this issue Nov 14, 2023
@RaduBerinde
Merge #114308
1f280fe 
114308: upgrades: reorganize files r=RaduBerinde a=RaduBerinde

This PR is only for the top commit (it is on top of #114306).

#### upgrades: reorganize files

In this commit we rename files implementing permanent upgrades to
start with `permanent_` and files implementing 23.2 upgrades to start
with `v23_2_`. This will make it easier to remove deprecated upgrades
later.

Informs: #112629
Release note: None

Co-authored-by: Radu Berinde <[email protected]>
@RaduBerinde RaduBerinde mentioned this issue Nov 28, 2023
Merged
@RaduBerinde RaduBerinde mentioned this issue Nov 28, 2023
Merged
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Nov 29, 2023
@RaduBerinde
clusterversion: add runbooks
9cc2ca5 
This commit adds a `README.md` file which contains runbooks for
advancing versions.

Informs: cockroachdb#112629
Release note: None
craig bot pushed a commit that referenced this issue Nov 29, 2023
@RaduBerinde @msbutler
Merge #115208 #115209
942b1e2 
115208: clusterversion: remove ByKey r=RaduBerinde a=RaduBerinde

Remove `ByKey`, replacing usage with `.Version()`.

Epic: none
Release note: None

Informs #112629.

115209: roachtest: bump max latency to 5 mins on c2c/weekly/kv0 r=dt a=msbutler

This patch prevents the roachtest from failing on a temporary latency spike of 2 minutes. A 5 minute latency spike likely indicates something is wrong in the system.

Fixes: #115069

Release note: none

Co-authored-by: Radu Berinde <[email protected]>
Co-authored-by: Michael Butler <[email protected]>
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Nov 29, 2023
@RaduBerinde
clusterversion: add runbooks
5689f41 
This commit adds a `README.md` file which contains runbooks for
advancing versions.

Informs: cockroachdb#112629
Release note: None
@RaduBerinde RaduBerinde mentioned this issue Nov 30, 2023
Merged
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Nov 30, 2023
@RaduBerinde
clusterversion: change string for upgrade versions
6a1e312 
This change concerns cluster versions during upgrade. When starting an
upgrade from e.g. 23.1 to 23.2, we start with the cluster version 23.1
and then we go through a sequence of internal versions associated with
various upgrade steps. These versions are presented as `23.1-x`, e.g.
`23.1-8`.

This formatting doesn't make it clear what this version represents. It
can also be confusing - `23.1-8` looks very close to `23.1.8` which
might be an actual CockroachDB version.

This change renames versions during upgrade:
`23.1-upgrading-to-23.2-step-008`. The internal part is always
formatted to three digits (this is intended to reduce the chance of
confusing the internal part to a patch release).

Informs: cockroachdb#112629
Release note (general change): Versions during upgrades are renamed,
for example `23.1-8` is now `23.1-upgrading-to-23.2-step-008`.
craig bot pushed a commit that referenced this issue Nov 30, 2023
@RaduBerinde @aadityasondhi @rafiss
Merge #115213 #115262 #115347
364f3a6 
115213: clusterversion: add runbooks r=RaduBerinde a=RaduBerinde

This commit adds a `README.md` file which contains runbooks for
advancing versions.

Informs: #112629
Release note: None

Rendered file can be viewed here: https://github.com/RaduBerinde/cockroach/blob/version-doc/pkg/clusterversion/README.md

115262: admission: add extra info when deadline reached in flowcontrol r=sumeerbhola a=aadityasondhi

This patch improves the error message when context deadline is reached in flowcontrol wait queue. It includes priority, wait duration, stream.

Informs: #113990

Release note: None

115347: lease: skip TestRangefeedUpdatesHandledProperlyInTheFaceOfRaces under race r=rafiss a=rafiss

fixes #114324
Release note: None

Co-authored-by: Radu Berinde <[email protected]>
Co-authored-by: Aaditya Sondhi <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Nov 30, 2023
@RaduBerinde
clusterversion: change string for upgrade versions
afbe5ac 
This change concerns cluster versions during upgrade. When starting an
upgrade from e.g. 23.3 to 24.1, we start with the cluster version 23.2
and then we go through a sequence of internal versions associated with
various upgrade steps. These versions are presented as `23.2-x`, e.g.
`23.2-8`.

This formatting doesn't make it clear what this version represents. It
can also be confusing - `23.2-8` looks very close to `23.2.8` which
might be an actual CockroachDB version.

This change renames versions during upgrade:
`23.2-upgrading-to-24.1-step-008`. The internal part is always
formatted to three digits (this is intended to reduce the chance of
confusing the internal part to a patch release).

Informs: cockroachdb#112629
Release note (general change): Versions during upgrades are renamed,
for example `23.2-8` is now `23.2-upgrading-to-24.1-step-008`.
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Dec 1, 2023
@RaduBerinde
clusterversion: change string for upgrade versions
14f57af 
This change concerns cluster versions during upgrade. When starting an
upgrade from e.g. 23.3 to 24.1, we start with the cluster version 23.2
and then we go through a sequence of internal versions associated with
various upgrade steps. These versions are presented as `23.2-x`, e.g.
`23.2-8`.

This formatting doesn't make it clear what this version represents. It
can also be confusing - `23.2-8` looks very close to `23.2.8` which
might be an actual CockroachDB version.

This change renames versions during upgrade:
`23.2-upgrading-to-24.1-step-008`. The internal part is always
formatted to three digits (this is intended to reduce the chance of
confusing the internal part to a patch release).

Informs: cockroachdb#112629
Release note (general change): Versions during upgrades are renamed,
for example `23.2-8` is now `23.2-upgrading-to-24.1-step-008`.
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Dec 1, 2023
@RaduBerinde
clusterversion: change string for upgrade versions
31521ef 
This change concerns cluster versions during upgrade. When starting an
upgrade from e.g. 23.3 to 24.1, we start with the cluster version 23.2
and then we go through a sequence of internal versions associated with
various upgrade steps. These versions are presented as `23.2-x`, e.g.
`23.2-8`.

This formatting doesn't make it clear what this version represents. It
can also be confusing - `23.2-8` looks very close to `23.2.8` which
might be an actual CockroachDB version.

This change renames versions during upgrade:
`23.2-upgrading-to-24.1-step-008`. The internal part is always
formatted to three digits (this is intended to reduce the chance of
confusing the internal part to a patch release).

Informs: cockroachdb#112629
Release note (general change): Versions during upgrades are renamed,
for example `23.2-8` is now `23.2-upgrading-to-24.1-step-008`.
craig bot pushed a commit that referenced this issue Dec 1, 2023
@bdarnell @RaduBerinde
Merge #113999 #115223
19ce36d 
113999: engineccl: Add benchmark for ctr_stream encryption r=sumeerbhola a=bdarnell

Start measuring performance of this code in anticipation of improving it.

Epic: none

Release note: None

115223: clusterversion: change string for upgrade versions r=RaduBerinde a=RaduBerinde

#### clusterversion: move ReleaseSeries functionality to roacphb.Version

This change moves the implementation of
`clusterversion.Key.ReleaseSeries()` to `roachpb`. We now use a
hardcoded map of sucessor versions.

Epic: none
Release note: None

#### clusterversion: change string for upgrade versions

This change concerns cluster versions during upgrade. When starting an
upgrade from e.g. 23.3 to 24.1, we start with the cluster version 23.2
and then we go through a sequence of internal versions associated with
various upgrade steps. These versions are presented as `23.2-x`, e.g.
`23.2-8`.

This formatting doesn't make it clear what this version represents. It
can also be confusing - `23.2-8` looks very close to `23.2.8` which
might be an actual CockroachDB version.

This change renames versions during upgrade:
`23.2-upgrading-to-24.1-step-008`. The internal part is always
formatted to three digits (this is intended to reduce the chance of
confusing the internal part to a patch release).

Informs: #112629
Release note (general change): Versions during upgrades are renamed,
for example `23.2-8` is now `23.2-upgrading-to-24.1-step-008`.


Co-authored-by: Ben Darnell <[email protected]>
Co-authored-by: Radu Berinde <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@RaduBerinde RaduBerinde

Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pav-kv @RaduBerinde