From 9f4a5d83b9587137f9efc07e8214d271e264ad07 Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Thu, 2 Nov 2023 14:25:33 -0700 Subject: [PATCH] clusterversion: cockroach versions cleanup 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 --- pkg/clusterversion/BUILD.bazel | 4 +- pkg/clusterversion/clusterversion.go | 2 +- pkg/clusterversion/clusterversion_test.go | 2 +- pkg/clusterversion/cockroach_versions.go | 615 ++++++------------ pkg/clusterversion/cockroach_versions_test.go | 158 ++--- pkg/clusterversion/dev_offset.go | 87 +++ pkg/clusterversion/keyed_versions.go | 106 --- pkg/clusterversion/setting.go | 2 +- pkg/clusterversion/setting_test.go | 4 +- pkg/clusterversion/testutils.go | 6 +- pkg/clusterversion/utilversions.go | 2 +- pkg/roachpb/version.go | 8 +- pkg/sql/catalog/bootstrap/bootstrap_test.go | 28 +- pkg/sql/catalog/bootstrap/initial_values.go | 2 +- pkg/storage/bench_data_test.go | 4 +- pkg/upgrade/upgrades/builtins_test.go | 4 +- 16 files changed, 364 insertions(+), 670 deletions(-) create mode 100644 pkg/clusterversion/dev_offset.go delete mode 100644 pkg/clusterversion/keyed_versions.go diff --git a/pkg/clusterversion/BUILD.bazel b/pkg/clusterversion/BUILD.bazel index 8126305b0da9..eff3aedf3013 100644 --- a/pkg/clusterversion/BUILD.bazel +++ b/pkg/clusterversion/BUILD.bazel @@ -7,7 +7,7 @@ go_library( srcs = [ "clusterversion.go", "cockroach_versions.go", - "keyed_versions.go", + "dev_offset.go", "setting.go", "testutils.go", "utilversions.go", @@ -25,7 +25,6 @@ go_library( "//pkg/util/timeutil", "@com_github_cockroachdb_errors//:errors", "@com_github_cockroachdb_redact//:redact", - "@com_github_kr_pretty//:pretty", ], ) @@ -48,7 +47,6 @@ go_test( "//pkg/util/protoutil", "@com_github_cockroachdb_errors//:errors", "@com_github_cockroachdb_redact//:redact", - "@com_github_dustin_go_humanize//:go-humanize", "@com_github_stretchr_testify//require", ], ) diff --git a/pkg/clusterversion/clusterversion.go b/pkg/clusterversion/clusterversion.go index 062553678c55..e460a04f17a2 100644 --- a/pkg/clusterversion/clusterversion.go +++ b/pkg/clusterversion/clusterversion.go @@ -171,7 +171,7 @@ var _ Handle = (*handleImpl)(nil) // supported versions initialized to this binary's build and it's minimum // supported versions respectively. func MakeVersionHandle(sv *settings.Values) Handle { - return MakeVersionHandleWithOverride(sv, binaryVersion, binaryMinSupportedVersion) + return MakeVersionHandleWithOverride(sv, Latest.Version(), MinSupported.Version()) } // MakeVersionHandleWithOverride returns a Handle that has its diff --git a/pkg/clusterversion/clusterversion_test.go b/pkg/clusterversion/clusterversion_test.go index f3304c70afda..9324f6592030 100644 --- a/pkg/clusterversion/clusterversion_test.go +++ b/pkg/clusterversion/clusterversion_test.go @@ -34,7 +34,7 @@ func TestClusterVersionOnChange(t *testing.T) { "test description", &cvs.VersionSetting) - handle := newHandleImpl(cvs, &sv, binaryVersion, binaryMinSupportedVersion) + handle := newHandleImpl(cvs, &sv, Latest.Version(), MinSupported.Version()) newCV := ClusterVersion{ Version: roachpb.Version{ Major: 1, diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go index f292db788c42..9be2d60ff32d 100644 --- a/pkg/clusterversion/cockroach_versions.go +++ b/pkg/clusterversion/cockroach_versions.go @@ -10,10 +10,7 @@ package clusterversion -import ( - "github.com/cockroachdb/cockroach/pkg/roachpb" - "github.com/cockroachdb/cockroach/pkg/util/envutil" -) +import "github.com/cockroachdb/cockroach/pkg/roachpb" // Key is a unique identifier for a version of CockroachDB. type Key int @@ -93,13 +90,13 @@ type Key int // (ii) V21_1Start (keyed to v20.2.0-1}) // // You'll then want to backport (i) to the release branch itself (i.e. -// release-20.2). You'll also want to bump binaryMinSupportedVersion. In the +// release-20.2). You'll also want to bump MinSupported. In the // example above, you'll set it to V20_2. This indicates that the -// minimum binary version required in a cluster with nodes running +// minimum cluster version required in a cluster with nodes running // v21.1 binaries (including pre-release alphas) is v20.2, i.e. that an -// upgrade into such a binary must start out from at least v20.2 nodes. +// upgrade to v21.1 must start out from at least v20.2 nodes. // -// Aside: At the time of writing, the binary min supported version is the +// Aside: At the time of writing, the min supported version is the // last major release, though we may consider relaxing this in the future // (i.e. for example could skip up to one major release) as we move to a more // frequent release schedule. @@ -134,22 +131,22 @@ type Key int // // Versions and non-permanent upgrades can be removed once they are no longer // going to be exercised. This is primarily driven by the -// BinaryMinSupportedVersion, which declares the oldest *cluster* (not binary) +// MinSupported version, which declares the oldest *cluster* (not binary) // version of CockroachDB that may interface with the running node. It typically // trails the current version by one release. For example, if the current branch -// is a `21.1.x` release, you will have a BinaryMinSupportedVersion of `21.0`, +// is a `21.1.x` release, you will have MinSupported=`21.0`, // meaning that the versions 20.2.0-1, 20.2.0-2, etc are always going to be // active on any peer and thus can be "baked in"; similarly all upgrades // attached to any of these versions can be assumed to have run (or not having // been necessary due to the cluster having been initialized at a higher version // in the first place). Note that this implies that all peers will have a -// *binary* version of at least the MinSupportedVersion as well, as this is a +// *binary* version of at least the MinSupported version as well, as this is a // prerequisite for running at that cluster version. Finally, note that even // when all cluster versions known to the current binary are active (i.e. most // of the time), you still need to be able to inter-op with older *binary* // and/or *cluster* versions. This is because *tenants* are allowed to run at // any binary version compatible with (i.e. greater than or equal to) the -// MinSupportedVersion. To give a concrete example, a fully up-to-date v21.1 KV +// MinSupported version. To give a concrete example, a fully up-to-date v21.1 KV // host cluster can have tenants running against it that use the v21.0 binary // and any cluster version known to that binary (v20.2-0 ... v20.2-50 or // thereabouts). @@ -165,19 +162,17 @@ type Key int // block below. // // Permanent upgrades and their associated version key cannot be removed (even -// if it is below the BinaryMinSupportedVersion). The version key should start -// with `Permanent_` to make this more explicit. The version numbers should not -// be changed - we want all nodes in a mixed-version cluster to agree on what -// version a certain upgrade step is tied to (in the unlikely scenario that we -// have mixed-version nodes while bootstrapping a cluster). +// if it is below MinSupported). The version key should start with `Permanent_` +// to make this more explicit. The version numbers should not be changed - we +// want all nodes in a mixed-version cluster to agree on what version a certain +// upgrade step is tied to (in the unlikely scenario that we have mixed-version +// nodes while bootstrapping a cluster). const ( - invalidVersionKey Key = iota - 1 // want first named one to start at zero - // VPrimordial versions are associated with permanent upgrades that exist for // historical reasons; no new primordial versions should be added, and no new // upgrades should be tied to existing primordial versions. - VPrimordial1 + VPrimordial1 Key = iota VPrimordial2 VPrimordial3 VPrimordial4 @@ -498,23 +493,11 @@ const ( // Step (1) Add new versions here. // Do not add new versions to a patch release. // ************************************************* -) -// VCurrent_Start is an alias for last Start version key (i.e the first internal -// version of the release in development). Tests should use this constant so -// they don't need to be updated when the versions change. -const VCurrent_Start = V24_1Start - -func (k Key) String() string { - return ByKey(k).String() -} - -// Offset every version +1M major versions into the future if this is a dev branch. -const DevOffset = 1000000 + numKeys +) -// rawVersionsSingleton lists all historical versions here in chronological -// order, with comments describing what backwards-incompatible features were -// introduced. +// versionsTable lists all historical versions here in chronological order. // // A roachpb.Version has the colloquial form MAJOR.MINOR[.PATCH][-INTERNAL], // where the PATCH and INTERNAL components can be omitted if zero. Keep in mind @@ -530,306 +513,94 @@ const DevOffset = 1000000 // minor version until we are absolutely sure that no new migrations will need // to be added (i.e., when cutting the final release candidate). // -// rawVersionsSingleton is converted to versionsSingleton below, by adding a -// large number to every major if building from master, so as to ensure that -// master builds cannot be upgraded to release-branch builds. -var rawVersionsSingleton = keyedVersions{ - { - Key: VPrimordial1, - Version: roachpb.Version{Major: 0, Minor: 0, Internal: 2}, - }, - { - Key: VPrimordial2, - Version: roachpb.Version{Major: 0, Minor: 0, Internal: 4}, - }, - { - Key: VPrimordial3, - Version: roachpb.Version{Major: 0, Minor: 0, Internal: 6}, - }, - { - Key: VPrimordial4, - Version: roachpb.Version{Major: 0, Minor: 0, Internal: 8}, - }, - { - Key: VPrimordial5, - Version: roachpb.Version{Major: 0, Minor: 0, Internal: 10}, - }, - { - Key: VPrimordial6, - Version: roachpb.Version{Major: 0, Minor: 0, Internal: 12}, - }, - { - Key: VPrimordialMax, - Version: roachpb.Version{Major: 0, Minor: 0, Internal: 424242}, - }, - - // v22.2 versions. Internal versions must be even. - { - Key: Permanent_V22_2SQLSchemaTelemetryScheduledJobs, - Version: roachpb.Version{Major: 22, Minor: 1, Internal: 42}, - }, - { - Key: TODO_Delete_V22_2, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 0}, - }, - { - Key: TODO_Delete_V23_1Start, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 2}, - }, - { - Key: TODO_Delete_V23_1TenantNamesStateAndServiceMode, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 4}, - }, - { - Key: TODO_Delete_V23_1DescIDSequenceForSystemTenant, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 6}, - }, - { - Key: TODO_Delete_V23_1AddPartialStatisticsColumns, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 8}, - }, - { - Key: TODO_Delete_V23_1CreateSystemJobInfoTable, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 10}, - }, - { - Key: TODO_Delete_V23_1RoleMembersTableHasIDColumns, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 12}, - }, - { - Key: TODO_Delete_V23_1RoleMembersIDColumnsBackfilled, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 14}, - }, - { - Key: TODO_Delete_V23_1ScheduledChangefeeds, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 16}, - }, - { - Key: TODO_Delete_V23_1AddTypeColumnToJobsTable, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 18}, - }, - { - Key: TODO_Delete_V23_1BackfillTypeColumnInJobsTable, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 20}, - }, - { - Key: TODO_Delete_V23_1_AlterSystemStatementStatisticsAddIndexesUsage, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 22}, - }, - { - Key: TODO_Delete_V23_1AlterSystemSQLInstancesAddSQLAddr, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 28}, - }, - { - Key: TODO_Delete_V23_1_ChangefeedExpressionProductionReady, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 30}, - }, - { - Key: Permanent_V23_1KeyVisualizerTablesAndJobs, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 32}, - }, - { - Key: TODO_Delete_V23_1_KVDirectColumnarScans, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 34}, - }, - { - Key: TODO_Delete_V23_1_DeleteDroppedFunctionDescriptors, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 36}, - }, - { - Key: Permanent_V23_1_CreateJobsMetricsPollingJob, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 38}, - }, - { - Key: TODO_Delete_V23_1AllocatorCPUBalancing, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 40}, - }, - { - Key: TODO_Delete_V23_1SystemPrivilegesTableHasUserIDColumn, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 42}, - }, - { - Key: TODO_Delete_V23_1SystemPrivilegesTableUserIDColumnBackfilled, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 44}, - }, - { - Key: TODO_Delete_V23_1WebSessionsTableHasUserIDColumn, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 46}, - }, - { - Key: TODO_Delete_V23_1WebSessionsTableUserIDColumnBackfilled, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 48}, - }, - { - Key: TODO_Delete_V23_1_SchemaChangerDeprecatedIndexPredicates, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 50}, - }, - { - Key: TODO_Delete_V23_1AlterSystemPrivilegesAddIndexOnPathAndUsername, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 52}, - }, - { - Key: TODO_Delete_V23_1DatabaseRoleSettingsHasRoleIDColumn, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 54}, - }, - { - Key: TODO_Delete_V23_1DatabaseRoleSettingsRoleIDColumnBackfilled, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 56}, - }, - { - Key: TODO_Delete_V23_1TenantCapabilities, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 60}, - }, - { - Key: TODO_Delete_V23_1DeprecateClusterVersionKey, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 62}, - }, - { - Key: TODO_Delete_V23_1_SystemRbrDualWrite, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 66}, - }, - { - Key: TODO_Delete_V23_1_SystemRbrReadNew, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 68}, - }, - { - Key: TODO_Delete_V23_1_SystemRbrSingleWrite, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 70}, - }, - { - Key: TODO_Delete_V23_1_SystemRbrCleanup, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 72}, - }, - { - Key: TODO_Delete_V23_1ExternalConnectionsTableHasOwnerIDColumn, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 74}, - }, - { - Key: TODO_Delete_V23_1ExternalConnectionsTableOwnerIDColumnBackfilled, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 76}, - }, - { - Key: TODO_Delete_V23_1AllowNewSystemPrivileges, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 78}, - }, - { - Key: TODO_Delete_V23_1JobInfoTableIsBackfilled, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 80}, - }, - { - Key: TODO_Delete_V23_1EnableFlushableIngest, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 82}, - }, - { - Key: TODO_Delete_V23_1_UseDelRangeInGCJob, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 84}, - }, - { - Key: TODO_Delete_V23_1WaitedForDelRangeInGCJob, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 86}, - }, - { - Key: TODO_Delete_V23_1_TaskSystemTables, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 88}, - }, - { - Key: Permanent_V23_1_CreateAutoConfigRunnerJob, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 90}, - }, - { - Key: TODO_Delete_V23_1AddSQLStatsComputedIndexes, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 92}, - }, - { - Key: TODO_Delete_V23_1AddSystemActivityTables, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 94}, - }, - { - Key: TODO_Delete_V23_1StopWritingPayloadAndProgressToSystemJobs, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 96}, - }, - { - Key: Permanent_V23_1ChangeSQLStatsTTL, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 98}, - }, - { - Key: TODO_Delete_V23_1_TenantIDSequence, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 100}, - }, - { - Key: Permanent_V23_1CreateSystemActivityUpdateJob, - Version: roachpb.Version{Major: 22, Minor: 2, Internal: 102}, - }, - { - Key: V23_1, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 0}, - }, - { - Key: V23_2Start, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 2}, - }, - { - Key: V23_2_EnableRangeCoalescingForSystemTenant, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 8}, - }, - { - Key: V23_2_UseACRaftEntryEntryEncodings, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 10}, - }, - { - Key: V23_2_PebbleFormatDeleteSizedAndObsolete, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 12}, - }, - { - Key: V23_2_UseSizedPebblePointTombstones, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 14}, - }, - { - Key: V23_2_PebbleFormatVirtualSSTables, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 16}, - }, - { - Key: V23_2_StmtDiagForPlanGist, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 18}, - }, - { - Key: V23_2_RegionaLivenessTable, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 20}, - }, - { - Key: V23_2_RemoveLockTableWaiterTouchPush, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 22}, - }, - { - Key: V23_2_ChangefeedLaggingRangesOpts, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 24}, - }, - { - Key: V23_2_GrantExecuteToPublic, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 26}, - }, - { - Key: V23_2_EnablePebbleFormatVirtualSSTables, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 28}, - }, - { - Key: Permanent_V23_2_MVCCStatisticsTable, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 30}, - }, - { - Key: V23_2_AddSystemExecInsightsTable, - Version: roachpb.Version{Major: 23, Minor: 1, Internal: 32}, - }, - - { - Key: V23_2, - Version: roachpb.Version{Major: 23, Minor: 2, Internal: 0}, - }, - - { - Key: V24_1Start, - Version: roachpb.Version{Major: 23, Minor: 2, Internal: 2}, - }, +// Note that versions in this table can be modified by applying a "dev offset" +// to ensure that upgrades don't occur between in-development and released +// versions (see developmentBranch and maybeApplyDevOffset). +var versionTable = [numKeys]roachpb.Version{ + VPrimordial1: {Major: 0, Minor: 0, Internal: 2}, + VPrimordial2: {Major: 0, Minor: 0, Internal: 4}, + VPrimordial3: {Major: 0, Minor: 0, Internal: 6}, + VPrimordial4: {Major: 0, Minor: 0, Internal: 8}, + VPrimordial5: {Major: 0, Minor: 0, Internal: 10}, + VPrimordial6: {Major: 0, Minor: 0, Internal: 12}, + VPrimordialMax: {Major: 0, Minor: 0, Internal: 424242}, + + // Permanent upgrades from previous versions. + Permanent_V22_2SQLSchemaTelemetryScheduledJobs: {Major: 22, Minor: 1, Internal: 42}, + + TODO_Delete_V22_2: {Major: 22, Minor: 2, Internal: 0}, + + // v23.1 versions. Internal versions must be even. + TODO_Delete_V23_1Start: {Major: 22, Minor: 2, Internal: 2}, + TODO_Delete_V23_1TenantNamesStateAndServiceMode: {Major: 22, Minor: 2, Internal: 4}, + TODO_Delete_V23_1DescIDSequenceForSystemTenant: {Major: 22, Minor: 2, Internal: 6}, + TODO_Delete_V23_1AddPartialStatisticsColumns: {Major: 22, Minor: 2, Internal: 8}, + TODO_Delete_V23_1CreateSystemJobInfoTable: {Major: 22, Minor: 2, Internal: 10}, + TODO_Delete_V23_1RoleMembersTableHasIDColumns: {Major: 22, Minor: 2, Internal: 12}, + TODO_Delete_V23_1RoleMembersIDColumnsBackfilled: {Major: 22, Minor: 2, Internal: 14}, + TODO_Delete_V23_1ScheduledChangefeeds: {Major: 22, Minor: 2, Internal: 16}, + TODO_Delete_V23_1AddTypeColumnToJobsTable: {Major: 22, Minor: 2, Internal: 18}, + TODO_Delete_V23_1BackfillTypeColumnInJobsTable: {Major: 22, Minor: 2, Internal: 20}, + TODO_Delete_V23_1_AlterSystemStatementStatisticsAddIndexesUsage: {Major: 22, Minor: 2, Internal: 22}, + TODO_Delete_V23_1AlterSystemSQLInstancesAddSQLAddr: {Major: 22, Minor: 2, Internal: 28}, + TODO_Delete_V23_1_ChangefeedExpressionProductionReady: {Major: 22, Minor: 2, Internal: 30}, + Permanent_V23_1KeyVisualizerTablesAndJobs: {Major: 22, Minor: 2, Internal: 32}, + TODO_Delete_V23_1_KVDirectColumnarScans: {Major: 22, Minor: 2, Internal: 34}, + TODO_Delete_V23_1_DeleteDroppedFunctionDescriptors: {Major: 22, Minor: 2, Internal: 36}, + Permanent_V23_1_CreateJobsMetricsPollingJob: {Major: 22, Minor: 2, Internal: 38}, + TODO_Delete_V23_1AllocatorCPUBalancing: {Major: 22, Minor: 2, Internal: 40}, + TODO_Delete_V23_1SystemPrivilegesTableHasUserIDColumn: {Major: 22, Minor: 2, Internal: 42}, + TODO_Delete_V23_1SystemPrivilegesTableUserIDColumnBackfilled: {Major: 22, Minor: 2, Internal: 44}, + TODO_Delete_V23_1WebSessionsTableHasUserIDColumn: {Major: 22, Minor: 2, Internal: 46}, + TODO_Delete_V23_1WebSessionsTableUserIDColumnBackfilled: {Major: 22, Minor: 2, Internal: 48}, + TODO_Delete_V23_1_SchemaChangerDeprecatedIndexPredicates: {Major: 22, Minor: 2, Internal: 50}, + TODO_Delete_V23_1AlterSystemPrivilegesAddIndexOnPathAndUsername: {Major: 22, Minor: 2, Internal: 52}, + TODO_Delete_V23_1DatabaseRoleSettingsHasRoleIDColumn: {Major: 22, Minor: 2, Internal: 54}, + TODO_Delete_V23_1DatabaseRoleSettingsRoleIDColumnBackfilled: {Major: 22, Minor: 2, Internal: 56}, + TODO_Delete_V23_1TenantCapabilities: {Major: 22, Minor: 2, Internal: 60}, + TODO_Delete_V23_1DeprecateClusterVersionKey: {Major: 22, Minor: 2, Internal: 62}, + TODO_Delete_V23_1_SystemRbrDualWrite: {Major: 22, Minor: 2, Internal: 66}, + TODO_Delete_V23_1_SystemRbrReadNew: {Major: 22, Minor: 2, Internal: 68}, + TODO_Delete_V23_1_SystemRbrSingleWrite: {Major: 22, Minor: 2, Internal: 70}, + TODO_Delete_V23_1_SystemRbrCleanup: {Major: 22, Minor: 2, Internal: 72}, + TODO_Delete_V23_1ExternalConnectionsTableHasOwnerIDColumn: {Major: 22, Minor: 2, Internal: 74}, + TODO_Delete_V23_1ExternalConnectionsTableOwnerIDColumnBackfilled: {Major: 22, Minor: 2, Internal: 76}, + TODO_Delete_V23_1AllowNewSystemPrivileges: {Major: 22, Minor: 2, Internal: 78}, + TODO_Delete_V23_1JobInfoTableIsBackfilled: {Major: 22, Minor: 2, Internal: 80}, + TODO_Delete_V23_1EnableFlushableIngest: {Major: 22, Minor: 2, Internal: 82}, + TODO_Delete_V23_1_UseDelRangeInGCJob: {Major: 22, Minor: 2, Internal: 84}, + TODO_Delete_V23_1WaitedForDelRangeInGCJob: {Major: 22, Minor: 2, Internal: 86}, + TODO_Delete_V23_1_TaskSystemTables: {Major: 22, Minor: 2, Internal: 88}, + Permanent_V23_1_CreateAutoConfigRunnerJob: {Major: 22, Minor: 2, Internal: 90}, + TODO_Delete_V23_1AddSQLStatsComputedIndexes: {Major: 22, Minor: 2, Internal: 92}, + TODO_Delete_V23_1AddSystemActivityTables: {Major: 22, Minor: 2, Internal: 94}, + TODO_Delete_V23_1StopWritingPayloadAndProgressToSystemJobs: {Major: 22, Minor: 2, Internal: 96}, + Permanent_V23_1ChangeSQLStatsTTL: {Major: 22, Minor: 2, Internal: 98}, + TODO_Delete_V23_1_TenantIDSequence: {Major: 22, Minor: 2, Internal: 100}, + Permanent_V23_1CreateSystemActivityUpdateJob: {Major: 22, Minor: 2, Internal: 102}, + + V23_1: {Major: 23, Minor: 1, Internal: 0}, + + // v23.2 versions. Internal versions must be even. + V23_2Start: {Major: 23, Minor: 1, Internal: 2}, + V23_2_EnableRangeCoalescingForSystemTenant: {Major: 23, Minor: 1, Internal: 8}, + V23_2_UseACRaftEntryEntryEncodings: {Major: 23, Minor: 1, Internal: 10}, + V23_2_PebbleFormatDeleteSizedAndObsolete: {Major: 23, Minor: 1, Internal: 12}, + V23_2_UseSizedPebblePointTombstones: {Major: 23, Minor: 1, Internal: 14}, + V23_2_PebbleFormatVirtualSSTables: {Major: 23, Minor: 1, Internal: 16}, + V23_2_StmtDiagForPlanGist: {Major: 23, Minor: 1, Internal: 18}, + V23_2_RegionaLivenessTable: {Major: 23, Minor: 1, Internal: 20}, + V23_2_RemoveLockTableWaiterTouchPush: {Major: 23, Minor: 1, Internal: 22}, + V23_2_ChangefeedLaggingRangesOpts: {Major: 23, Minor: 1, Internal: 24}, + V23_2_GrantExecuteToPublic: {Major: 23, Minor: 1, Internal: 26}, + V23_2_EnablePebbleFormatVirtualSSTables: {Major: 23, Minor: 1, Internal: 28}, + Permanent_V23_2_MVCCStatisticsTable: {Major: 23, Minor: 1, Internal: 30}, + V23_2_AddSystemExecInsightsTable: {Major: 23, Minor: 1, Internal: 32}, + + V23_2: {Major: 23, Minor: 2, Internal: 0}, + + // v24.1 versions. Internal versions must be even. + V24_1Start: {Major: 23, Minor: 2, Internal: 2}, // ************************************************* // Step (2): Add new versions here. @@ -837,127 +608,107 @@ var rawVersionsSingleton = keyedVersions{ // ************************************************* } -// developmentBranch must true on the main development branch but -// should be set to false on a release branch once the set of versions -// becomes append-only and associated upgrade implementations are -// frozen. It can be forced to a specific value in two circumstances: -// 1. forced to `false` on development branches: this is used for -// upgrade testing purposes and should never be done in real clusters; -// 2. forced to `false` on release branches: this allows running a -// release binary in a dev cluster. -var developmentBranch = !envutil.EnvOrDefaultBool("COCKROACH_TESTING_FORCE_RELEASE_BRANCH", false) || - envutil.EnvOrDefaultBool("COCKROACH_FORCE_DEV_VERSION", false) +// Latest is always the highest version key. This is the maximum logical cluster +// version supported by this branch. +const Latest Key = numKeys - 1 -const ( - // finalVersion should be set on a release branch to the minted final cluster - // version key, e.g. to V22_2 on the release-22.2 branch once it is minted. - // Setting it has the effect of ensuring no versions are subsequently added. - finalVersion = invalidVersionKey -) +// MinSupported is the minimum logical cluster version supported by this branch. +const MinSupported Key = V23_1 -var allowUpgradeToDev = envutil.EnvOrDefaultBool("COCKROACH_UPGRADE_TO_DEV_VERSION", false) - -var versionsSingleton = func() keyedVersions { - if developmentBranch { - // If this is a dev branch, we offset every version +1M major versions into - // the future. This means a cluster that runs the migrations in a dev build, - // while they are still in flux, will persist this offset version, and thus - // cannot then "upgrade" to the released build, as its non-offset versions - // would then be a downgrade, which is blocked. - // - // By default, when offsetting versions in a dev binary, we offset *all of - // them*, which includes the minimum version from upgrades are supported. - // This means a dev binary cannot join, resume or upgrade a release version - // cluster, which is by design as it avoids unintentionally but irreversibly - // upgrading a cluster to dev versions. Opting in to such an upgrade is - // possible however via setting COCKROACH_UPGRADE_TO_DEV_VERSION. Doing so - // skips offsetting the earliest version this binary supports, meaning it - // will support an upgrade from as low as that released version that then - // advances into the dev-numbered versions. - // - // Note that such upgrades may in fact be a *downgrade* of the logical - // version! For example, on a cluster that is on released version 3, a dev - // binary containing versions 1, 2, 3, and 4 started with this flag would - // renumber only 2-4 to be +1M. It would then step from 3 "up" to 1000002 -- - // which conceptually is actually back down to 2 -- then back to 1000003, - // then on to 1000004, etc. - for i := range rawVersionsSingleton { - // VPrimordial versions are not offset; they don't matter for the logic - // offsetting is used for. - if rawVersionsSingleton[i].Major == rawVersionsSingleton.MustByKey(VPrimordialMax).Major { - continue - } - - if allowUpgradeToDev && rawVersionsSingleton[i].Key <= BinaryMinSupportedVersionKey { - // Support upgrading from the non-development version of BinaryMinSupportedVersionKey. - continue - } - - rawVersionsSingleton[i].Major += DevOffset - } - } - return rawVersionsSingleton -}() +// PreviousRelease is the logical cluster version of the previous release. +// +// Note: this is always the last element of SupportedPreviousReleases(); it is +// also provided as a constant for convenience. +const PreviousRelease Key = V23_2 // V24_1 is a placeholder that will eventually be replaced by the actual 24.1 // version Key, but in the meantime it points to the latest Key. The placeholder // is defined so that it can be referenced in code that simply wants to check if // a cluster is running 24.1 and has completed all associated migrations; most -// version gates can use this instead of defining their own version key if all -// simply need to check is that the cluster has upgraded to 24.1. -var V24_1 = versionsSingleton[len(versionsSingleton)-1].Key +// version gates can use this instead of defining their own version key if they +// only need to check that the cluster has upgraded to 24.1. +const V24_1 = Latest -const ( - BinaryMinSupportedVersionKey = V23_1 - PreviousReleaseVersionKey = V23_1 -) +// DEPRECATED: use MinSupported. +const BinaryMinSupportedVersionKey = MinSupported -// TODO(irfansharif): clusterversion.binary{,MinimumSupported}Version -// feels out of place. A "cluster version" and a "binary version" are two -// separate concepts. -var ( - // binaryMinSupportedVersion is the earliest version of data supported by - // this binary. If this binary is started using a store marked with an older - // version than binaryMinSupportedVersion, then the binary will exit with - // an error. This typically trails the current release by one (see top-level - // comment). - binaryMinSupportedVersion = ByKey(BinaryMinSupportedVersionKey) - - BinaryVersionKey = V24_1 - // binaryVersion is the version of this binary. - // - // This is the version that a new cluster will use when created. - binaryVersion = ByKey(BinaryVersionKey) -) +// DEPRECATED: use Latest. +const BinaryVersionKey = Latest + +// developmentBranch must be true on the main development branch but should be +// set to false on a release branch once the set of versions becomes append-only +// and associated upgrade implementations are frozen. +// +// It can be forced to a specific value in two circumstances: +// 1. forced to `false` on development branches: this is used for upgrade +// testing purposes and should never be done in real clusters; +// 2. forced to `true` on release branches: this allows running a release +// binary in a dev cluster. +// +// See devOffsetKeyStart for more details. +const developmentBranch = true + +// finalVersion should be set on a release branch to the minted final cluster +// version key, e.g. to V23_2 on the release-23.2 branch once it is minted. +// Setting it has the effect of ensuring no versions are subsequently added. +const finalVersion Key = -1 func init() { - if finalVersion > invalidVersionKey { - if binaryVersion != ByKey(finalVersion) { - panic("binary version does not match final version") + if finalVersion >= 0 { + if Latest != finalVersion { + panic("latest version does not match final version") } - } else if binaryVersion.Internal == 0 { - panic("a non-upgrade cluster version must be the final version") + if developmentBranch { + panic("final version set on development branch") + } + } else if Latest.IsFinal() { + panic("finalVersion not set but latetest version is final") } } +// Version returns the roachpb.Version corresponding to a key. +func (k Key) Version() roachpb.Version { + version := versionTable[k] + return maybeDevOffset(k, version) +} + +// IsFinal returns true if the key is a final version (as opposed to a +// transitional internal version during upgrade). +func (k Key) IsFinal() bool { + return k.Version().IsFinal() +} + +func (k Key) String() string { + return k.Version().String() +} + +// SupportedPreviousReleases returns the list of final versions for previous +// that are supported by this branch (and from which we can upgrade an existing +// cluster). +func SupportedPreviousReleases() []Key { + res := make([]Key, 0, 2) + for k := MinSupported; k < Latest; k++ { + if k.IsFinal() { + res = append(res, k) + } + } + return res +} + // ByKey returns the roachpb.Version for a given key. // It is a fatal error to use an invalid key. +// DEPRECATED: use key.Version() instead. func ByKey(key Key) roachpb.Version { - return versionsSingleton.MustByKey(key) + return key.Version() } // ListBetween returns the list of cluster versions in the range // (from, to]. func ListBetween(from, to roachpb.Version) []roachpb.Version { - return listBetweenInternal(from, to, versionsSingleton) -} - -func listBetweenInternal(from, to roachpb.Version, vs keyedVersions) []roachpb.Version { var cvs []roachpb.Version - for _, keyedV := range vs { - // Read: "from < keyedV <= to". - if from.Less(keyedV.Version) && keyedV.Version.LessEq(to) { - cvs = append(cvs, keyedV.Version) + for k := Key(0); k < numKeys; k++ { + if v := k.Version(); from.Less(v) && v.LessEq(to) { + cvs = append(cvs, v) } } return cvs diff --git a/pkg/clusterversion/cockroach_versions_test.go b/pkg/clusterversion/cockroach_versions_test.go index ebf405127b1d..b3c49c6db951 100644 --- a/pkg/clusterversion/cockroach_versions_test.go +++ b/pkg/clusterversion/cockroach_versions_test.go @@ -16,52 +16,82 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/redact" - "github.com/dustin/go-humanize" "github.com/stretchr/testify/require" ) -func TestVersionsAreValid(t *testing.T) { +// TestVersionTable runs sanity checks on the versions table: +// - all keys have a non-zero version; +// - versions are strictly increasing; +// - we have no odd Internal values. +func TestVersionTable(t *testing.T) { defer leaktest.AfterTest(t)() - require.NoError(t, versionsSingleton.Validate()) + var prev roachpb.Version + for k := Key(0); k < numKeys; k++ { + v := versionTable[k] + require.NotEqual(t, roachpb.Version{}, v) + // We don't use Patch for cluster versions. + require.Zero(t, v.Patch) + // All internal versions must be even (since 23.1). + require.Zero(t, v.Internal%2) + if k > 0 { + require.True(t, prev.Less(v)) + } + prev = v + } } -// TestPreserveVersionsForMinBinaryVersion ensures that versions -// at or above binaryMinSupportedVersion are not deleted. -func TestPreserveVersionsForMinBinaryVersion(t *testing.T) { - defer leaktest.AfterTest(t)() - - prevVersion := keyedVersion{ - Key: -1, - Version: roachpb.Version{Major: 1, Minor: 1}, +// TestNoMissingVersions checks that starting with MinSupported, we have no gaps +// in Internal versions (which would happen if a version was incorrectly +// removed). +func TestNoMissingVersions(t *testing.T) { + // removedVersions lists versions that have been purposely removed during a + // development cycle. + // + // Any additions here should be accompanied by a comment explaining the + // reason. The versions in the list must be in order. + removedVersions := []roachpb.Version{ + // In the 23.2 cycle V23_2TTLAllowDescPK and V23_2_PartiallyVisibleIndexes + // were introduced but then later removed (since they were redundant). + {Major: 23, Minor: 1, Internal: 4}, + {Major: 23, Minor: 1, Internal: 6}, } - for _, namedVersion := range versionsSingleton { - v := namedVersion.Version - if v.Less(binaryMinSupportedVersion) { - prevVersion = namedVersion - continue - } - expectedDiff := int32(2) - if prevVersion.Key == V23_2Start { - // In 23.2 cycle V23_2TTLAllowDescPK and - // V23_2_PartiallyVisibleIndexes were introduced but then later - // removed (since they were redundant), so we exempt them from not - // being deleted. (There were more versions introduced and then - // removed, but they were at the tail of 23.2 versions, so they - // don't require a similar exemption.) - expectedDiff = 6 - } - if v.Major == prevVersion.Major && v.Minor == prevVersion.Minor { - require.Equalf(t, prevVersion.Internal+expectedDiff, v.Internal, - "version(s) between %s (%s) and %s (%s) is(are) at or above minBinaryVersion (%s) and should not be removed", - prevVersion.Key, prevVersion.Version, - namedVersion.Key, namedVersion.Version, - binaryMinSupportedVersion) + + for k := MinSupported + 1; k < numKeys; k++ { + prev := (k - 1).Version() + v := k.Version() + if v.Major == prev.Major && v.Minor == prev.Minor { + expectedInternal := prev.Internal + 2 + + // Allow exceptions. This loop assumes the removed versions are increasing + // (the condition could hit multiple times). + for _, removed := range removedVersions { + if (removed.Major == v.Major || removed.Major+DevOffset == v.Major) && + removed.Minor == v.Minor && + removed.Internal == expectedInternal { + expectedInternal += 2 + } + } + + require.Equalf(t, expectedInternal, v.Internal, + "version gap between %s (%s) and %s (%s)", k-1, prev, k, v, + ) } - prevVersion = namedVersion } } +// TestKeyConstants runs sanity checks on version key constants. +func TestKeyConstants(t *testing.T) { + require.Equal(t, numKeys-1, Latest) + // MinSupported should be a final release. + require.True(t, MinSupported.IsFinal()) + + supported := SupportedPreviousReleases() + require.Equal(t, MinSupported, supported[0]) + // Check PreviousRelease. + require.Equal(t, PreviousRelease, supported[len(supported)-1]) +} + func TestVersionFormat(t *testing.T) { defer leaktest.AfterTest(t)() @@ -117,63 +147,3 @@ func TestClusterVersionPrettyPrint(t *testing.T) { } } } - -func TestGetVersionsBetween(t *testing.T) { - defer leaktest.AfterTest(t)() - - // Define a list of versions v3..v9 - var vs keyedVersions - for i := 3; i < 10; i++ { - vs = append(vs, keyedVersion{ - Key: Key(42), - Version: roachpb.Version{Major: int32(i)}, - }) - } - v := func(major int32) roachpb.Version { - return roachpb.Version{Major: major} - } - list := func(first, last int32) []roachpb.Version { - var cvs []roachpb.Version - for i := first; i <= last; i++ { - cvs = append(cvs, v(i)) - } - return cvs - } - - tests := []struct { - from, to roachpb.Version - exp []roachpb.Version - }{ - {v(5), v(8), list(6, 8)}, - {v(1), v(1), []roachpb.Version{}}, - {v(7), v(7), []roachpb.Version{}}, - {v(1), v(5), list(3, 5)}, - {v(6), v(12), list(7, 9)}, - {v(4), v(5), list(5, 5)}, - } - - for _, test := range tests { - actual := listBetweenInternal(test.from, test.to, vs) - if len(actual) != len(test.exp) { - t.Errorf("expected %d versions, got %d", len(test.exp), len(actual)) - } - - for i := range test.exp { - if actual[i] != test.exp[i] { - t.Errorf("%s version incorrect: expected %s, got %s", humanize.Ordinal(i), test.exp[i], actual[i]) - } - } - } -} - -// TestEnsureConsistentBinaryVersion ensures that BinaryVersionKey maps to a -// version equal to binaryVersion. -func TestEnsureConsistentBinaryVersion(t *testing.T) { - require.Equal(t, ByKey(BinaryVersionKey), binaryVersion) -} - -// TestEnsureConsistentMinBinaryVersion ensures that BinaryMinSupportedVersionKey -// maps to a version equal to binaryMinSupportedVersion. -func TestEnsureConsistentMinBinaryVersion(t *testing.T) { - require.Equal(t, ByKey(BinaryMinSupportedVersionKey), binaryMinSupportedVersion) -} diff --git a/pkg/clusterversion/dev_offset.go b/pkg/clusterversion/dev_offset.go new file mode 100644 index 000000000000..988752d31b61 --- /dev/null +++ b/pkg/clusterversion/dev_offset.go @@ -0,0 +1,87 @@ +// Copyright 2023 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package clusterversion + +import ( + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/util/envutil" + "github.com/cockroachdb/errors" +) + +// devOffsetKeyStart is the key at or above which we apply the DevOffset major +// version adjustment. +// +// If this is a dev branch, we offset every version +1M major versions into the +// future. This means a cluster that runs the migrations in a dev build, while +// they are still in flux, will persist this offset version, and thus cannot +// then "upgrade" to the released build, as its non-offset versions would then +// be a downgrade, which is blocked. +// +// By default, when offsetting versions in a dev binary, we offset *all of +// them*, which includes the minimum version from which upgrades are supported. +// This means a dev binary cannot join, resume or upgrade a release version +// cluster, which is by design as it avoids unintentionally but irreversibly +// upgrading a cluster to dev versions. Opting in to such an upgrade is possible +// however via setting COCKROACH_UPGRADE_TO_DEV_VERSION. Doing so skips +// offsetting the earliest version this binary supports, meaning it will support +// an upgrade from as low as that released version that then advances into the +// dev-numbered versions. +// +// COCKROACH_UPGRADE_TO_DEV_VERSION must be used very carefully - it can +// effectively cause *downgrades* of the logical version! For example, on a +// cluster that is on released version 3 with minimum supported version 2, a dev +// binary containing versions 1, 2, 3, and 4 started with this flag would +// renumber only 2-4 to be +1M. It would then step from 3 "up" to 1000002 - +// which conceptually is actually back down to 2 - then back to 1000003, then on +// to 1000004, etc. +// +// Version offsetting is controlled by the developmentBranch constant, but can +// be overridden with env vars: +// - COCKROACH_TESTING_FORCE_RELEASE_BRANCH=1 disables version offsetting +// unconditionally; it is used for certain upgrade tests that are trying to +// simulate a "real" upgrade. +// - COCKROACH_FORCE_DEV_VERSION=1 enables version offsetting unconditionally; +// it is useful if we want to run a final release binary in a cluster that +// was already created with a dev version. +var devOffsetKeyStart = func() Key { + forceDev := envutil.EnvOrDefaultBool("COCKROACH_FORCE_DEV_VERSION", false) + forceRelease := envutil.EnvOrDefaultBool("COCKROACH_TESTING_FORCE_RELEASE_BRANCH", false) + if forceDev && forceRelease { + panic(errors.AssertionFailedf("cannot set both COCKROACH_FORCE_DEV_VERSION and COCKROACH_TESTING_FORCE_RELEASE_BRANCH")) + } + isDev := (developmentBranch || forceDev) && !forceRelease + if !isDev { + // No dev offsets. + return numKeys + 1 + } + // If COCKROACH_UPGRADE_TO_DEV_VERSION is set, we allow updating from the + // minimum supported release, so we only apply the dev offset to subsequent + // versions. + allowUpgradeToDev := envutil.EnvOrDefaultBool("COCKROACH_UPGRADE_TO_DEV_VERSION", false) + if allowUpgradeToDev { + return MinSupported + 1 + } + // Apply the dev offset to all versions (except VPrimordial versions, which + // don't matter for offsetting logic). + return VPrimordialMax + 1 +}() + +// DevOffset is the offset applied to major versions into the future if this is +// a dev branch. +const DevOffset = 1_000_000 + +// maybeDevOffset applies DevOffset to the major version, if appropriate. +func maybeDevOffset(key Key, v roachpb.Version) roachpb.Version { + if key >= devOffsetKeyStart { + v.Major += DevOffset + } + return v +} diff --git a/pkg/clusterversion/keyed_versions.go b/pkg/clusterversion/keyed_versions.go deleted file mode 100644 index ccdb07e67dc4..000000000000 --- a/pkg/clusterversion/keyed_versions.go +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright 2017 The Cockroach Authors. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -package clusterversion - -import ( - "context" - - "github.com/cockroachdb/cockroach/pkg/roachpb" - "github.com/cockroachdb/cockroach/pkg/util/log" - "github.com/cockroachdb/errors" - "github.com/kr/pretty" -) - -// keyedVersion associates a key to a version. -type keyedVersion struct { - Key Key - roachpb.Version -} - -// keyedVersions is a container for managing the versions of CockroachDB. -type keyedVersions []keyedVersion - -// MustByKey asserts that the version specified by this key exists, and returns it. -func (kv keyedVersions) MustByKey(k Key) roachpb.Version { - key := int(k) - if key >= len(kv) || key < 0 { - log.Fatalf(context.Background(), "version with key %d does not exist, have:\n%s", - key, pretty.Sprint(kv)) - } - return kv[key].Version -} - -// Validate makes sure that the keyedVersions are sorted chronologically, that -// their keys correspond to their position in the list, and that no obsolete -// versions (i.e. known to always be active) are present. -// -// For versions introduced in v21.1 and beyond, the internal versions must be -// even. -func (kv keyedVersions) Validate() error { - type majorMinor struct { - major, minor int32 - vs []keyedVersion - } - // byRelease maps major.minor to a slice of versions that were first - // released right after major.minor. For example, a version 20.1-12 would - // first be released in 20.2, and would be slotted under 20.1. We'll need - // this to determine which versions are always active with a binary built - // from the current SHA. - var byRelease []majorMinor - for i, namedVersion := range kv { - if int(namedVersion.Key) != i { - return errors.Errorf("version %s should have key %d but has %d", - namedVersion, i, namedVersion.Key) - } - if i > 0 { - prev := kv[i-1] - if !prev.Version.Less(namedVersion.Version) { - return errors.Errorf("version %s must be larger than %s", namedVersion, prev) - } - } - mami := majorMinor{major: namedVersion.Major, minor: namedVersion.Minor} - n := len(byRelease) - if n == 0 || byRelease[n-1].major != mami.major || byRelease[n-1].minor != mami.minor { - // Add new entry to the slice. - byRelease = append(byRelease, mami) - n++ - } - // Add to existing entry. - byRelease[n-1].vs = append(byRelease[n-1].vs, namedVersion) - } - - // Check to see that for versions introduced in v21.1 and beyond, the - // internal versions are always even. The odd versions are used for internal - // book-keeping. - for _, release := range byRelease { - // v21.1 versions (20.2-x) will be slotted under 20.2. Ignore all other - // releases. - if release.major < 20 { - continue - } - if release.major == 20 && release.minor == 1 { - continue - } - - for _, v := range release.vs { - if (v.Internal % 2) != 0 { - return errors.Errorf("found version %s with odd-numbered internal version (%s);"+ - " versions introduced in 21.1+ must have even-numbered internal versions", v.Key, v.Version) - } - } - } - - return nil -} - -func (kv keyedVersions) LastVersion() roachpb.Version { - return kv[len(kv)-1].Version -} diff --git a/pkg/clusterversion/setting.go b/pkg/clusterversion/setting.go index 67240d9d8e46..800b5e4c4e2f 100644 --- a/pkg/clusterversion/setting.go +++ b/pkg/clusterversion/setting.go @@ -221,7 +221,7 @@ func (cv *clusterVersionSetting) ValidateBinaryVersions( // SettingsListDefault is part of the VersionSettingImpl interface. func (cv *clusterVersionSetting) SettingsListDefault() string { - return binaryVersion.String() + return Latest.Version().String() } func (cv *clusterVersionSetting) validateBinaryVersions( diff --git a/pkg/clusterversion/setting_test.go b/pkg/clusterversion/setting_test.go index 2d14dc805b2d..52d57320508f 100644 --- a/pkg/clusterversion/setting_test.go +++ b/pkg/clusterversion/setting_test.go @@ -62,11 +62,11 @@ func TestMakeMetricsAndRegisterOnVersionChangeCallback(t *testing.T) { } func BenchmarkClusterVersionSettingIsActive(b *testing.B) { - s := cluster.MakeTestingClusterSettingsWithVersions(clusterversion.TestingBinaryVersion, clusterversion.TestingBinaryMinSupportedVersion, true) + s := cluster.MakeTestingClusterSettings() ctx := context.Background() active := true for i := 0; i < b.N; i++ { - active = s.Version.IsActive(ctx, clusterversion.VCurrent_Start) && active + active = s.Version.IsActive(ctx, clusterversion.Latest) && active } require.True(b, active) } diff --git a/pkg/clusterversion/testutils.go b/pkg/clusterversion/testutils.go index 86a8ed07c8c3..2ca54e71de8b 100644 --- a/pkg/clusterversion/testutils.go +++ b/pkg/clusterversion/testutils.go @@ -12,11 +12,13 @@ package clusterversion // TestingBinaryVersion is a binary version that tests can use when they don't // want to go through a Settings object. -var TestingBinaryVersion = binaryVersion +// TODO(radu): remove this. +var TestingBinaryVersion = Latest.Version() // TestingBinaryMinSupportedVersion is a minimum supported version that // tests can use when they don't want to go through a Settings object. -var TestingBinaryMinSupportedVersion = binaryMinSupportedVersion +// TODO(radu): remove this. +var TestingBinaryMinSupportedVersion = MinSupported.Version() // TestingClusterVersion is a ClusterVersion that tests can use when they don't // want to go through a Settings object. diff --git a/pkg/clusterversion/utilversions.go b/pkg/clusterversion/utilversions.go index 5af4478232f0..68c09ad7c94f 100644 --- a/pkg/clusterversion/utilversions.go +++ b/pkg/clusterversion/utilversions.go @@ -11,4 +11,4 @@ package clusterversion // DoctorBinaryVersion level used by the debug doctor when validating any files. -var DoctorBinaryVersion = binaryVersion +var DoctorBinaryVersion = Latest.Version() diff --git a/pkg/roachpb/version.go b/pkg/roachpb/version.go index 6fb5baaf773c..22f8ffa0ec80 100644 --- a/pkg/roachpb/version.go +++ b/pkg/roachpb/version.go @@ -59,13 +59,19 @@ func (v Version) String() string { return redact.StringWithoutMarkers(v) } // SafeFormat implements the redact.SafeFormatter interface. func (v Version) SafeFormat(p redact.SafePrinter, _ rune) { - if v.Internal == 0 { + if v.IsFinal() { p.Printf("%d.%d", v.Major, v.Minor) return } p.Printf("%d.%d-%d", v.Major, v.Minor, v.Internal) } +// IsFinal returns true if this is a final version (as opposed to a +// transitional internal version during upgrade). +func (v Version) IsFinal() bool { + return v.Internal == 0 +} + // PrettyPrint returns the value in a format that makes it apparent whether or // not it is a fence version. func (v Version) PrettyPrint() string { diff --git a/pkg/sql/catalog/bootstrap/bootstrap_test.go b/pkg/sql/catalog/bootstrap/bootstrap_test.go index 89cb3b1c3f72..50388698d7e6 100644 --- a/pkg/sql/catalog/bootstrap/bootstrap_test.go +++ b/pkg/sql/catalog/bootstrap/bootstrap_test.go @@ -13,7 +13,6 @@ package bootstrap import ( "crypto/sha256" "encoding/hex" - "reflect" "testing" "github.com/cockroachdb/cockroach/pkg/clusterversion" @@ -40,23 +39,16 @@ func TestSupportedReleases(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - expected := make(map[roachpb.Version]struct{}) - earliest := clusterversion.ByKey(clusterversion.BinaryMinSupportedVersionKey) - latest := clusterversion.ByKey(clusterversion.BinaryVersionKey) - var incumbent roachpb.Version - for _, v := range clusterversion.ListBetween(earliest, latest) { - if v.Major != incumbent.Major || v.Minor != incumbent.Minor { - incumbent = roachpb.Version{ - Major: v.Major, - Minor: v.Minor, - } - expected[incumbent] = struct{}{} - } + // Verify that the current version has an entry. + require.Contains(t, initialValuesFactoryByKey, clusterversion.Latest) + + // Verify that all previously supported versions have an entry. + for _, key := range clusterversion.SupportedPreviousReleases() { + require.Contains(t, initialValuesFactoryByKey, key) } - expected[latest] = struct{}{} - actual := make(map[roachpb.Version]struct{}) + + // Verify that all entries work. for k := range initialValuesFactoryByKey { - actual[clusterversion.ByKey(k)] = struct{}{} opts := InitialValuesOpts{ DefaultZoneConfig: zonepb.DefaultZoneConfigRef(), DefaultSystemZoneConfig: zonepb.DefaultZoneConfigRef(), @@ -69,10 +61,6 @@ func TestSupportedReleases(t *testing.T) { _, _, err = opts.GenerateInitialValues() require.NoErrorf(t, err, "error generating initial values for non-system codec in version %s", k) } - require.Truef(t, reflect.DeepEqual(actual, expected), - "expected supported releases %v, actual %v\n"+ - "see comments in test definition if this message appears", - expected, actual) } func TestInitialValuesToString(t *testing.T) { diff --git a/pkg/sql/catalog/bootstrap/initial_values.go b/pkg/sql/catalog/bootstrap/initial_values.go index 4b44aa62b8f1..d94c2e3dbd68 100644 --- a/pkg/sql/catalog/bootstrap/initial_values.go +++ b/pkg/sql/catalog/bootstrap/initial_values.go @@ -68,7 +68,7 @@ type initialValuesFactoryFn = func(opts InitialValuesOpts) ( ) var initialValuesFactoryByKey = map[clusterversion.Key]initialValuesFactoryFn{ - clusterversion.BinaryVersionKey: buildLatestInitialValues, + clusterversion.Latest: buildLatestInitialValues, clusterversion.V23_2: hardCodedInitialValues{ system: system232, systemHash: system232SHA256, diff --git a/pkg/storage/bench_data_test.go b/pkg/storage/bench_data_test.go index b30d7c655623..3437c9fdfb32 100644 --- a/pkg/storage/bench_data_test.go +++ b/pkg/storage/bench_data_test.go @@ -60,9 +60,7 @@ type engineWithLocation struct { Location } -var previousReleaseFormatMajorVersion = pebbleFormatVersion( - clusterversion.ByKey(clusterversion.PreviousReleaseVersionKey), -) +var previousReleaseFormatMajorVersion = pebbleFormatVersion(clusterversion.PreviousRelease.Version()) var previousReleaseFormatMajorVersionOpt ConfigOption = func(cfg *engineConfig) error { cfg.PebbleConfig.Opts.FormatMajorVersion = previousReleaseFormatMajorVersion diff --git a/pkg/upgrade/upgrades/builtins_test.go b/pkg/upgrade/upgrades/builtins_test.go index d114ced7a03d..73a989b580b8 100644 --- a/pkg/upgrade/upgrades/builtins_test.go +++ b/pkg/upgrade/upgrades/builtins_test.go @@ -32,7 +32,7 @@ func TestIsAtLeastVersionBuiltin(t *testing.T) { Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ DisableAutomaticVersionUpgrade: make(chan struct{}), - BinaryVersionOverride: clusterversion.ByKey(clusterversion.BinaryMinSupportedVersionKey), + BinaryVersionOverride: clusterversion.MinSupported.Version(), }, }, }, @@ -46,7 +46,7 @@ func TestIsAtLeastVersionBuiltin(t *testing.T) { ) defer tc.Stopper().Stop(ctx) - v := clusterversion.ByKey(clusterversion.VCurrent_Start).String() + v := clusterversion.Latest.Version().String() // Check that the builtin returns false when comparing against the new // version because we are still on the bootstrap version. sqlDB.CheckQueryResults(t, "SELECT crdb_internal.is_at_least_version('"+v+"')", [][]string{{"false"}})