Skip to content

Commit

Permalink
clusterversion: bump min supported version to 23.1
Browse files Browse the repository at this point in the history
This commit changes the minimum supported version to 23.1. It is
intended to be backported to release-23.2.

Tests that had to be modified mostly fall in one of two categories:
 - tests that verify a release-specific behavior involving 22.2 or
   in-development versions of 23.1; these tests were deleted.
 - tests that verify general version handling logic but used specific
   versions; I tried to make these use the min supported version or a
   new `VCurrent_Start` constant.

The logic tests required a bunch of work because we were missing a
`local-mixed-23.1` config, which should have been added early in the
23.2 cycle. Without this config, all test directives related to 23.2
features were conditioned on `local-mixed-22-23.1` (i.e. these
features were lumped in with 23.1 features). I added the new config
and let the test failures guide me; most of the cases that needed to
be conditioned on `local-mixed-23.1` where around the bigger 23.2
features (isolation levels, procedures). In subsequent release cycles,
we will create the new config as soon as possible.

This commit does not remove the obsolete in-development 23.1 version
keys and related code; that will be done separately.

Informs: #111760
Epic: REL-506
Release note: None
  • Loading branch information
RaduBerinde committed Oct 12, 2023
1 parent db92e0a commit 5ee9b6c
Show file tree
Hide file tree
Showing 130 changed files with 319 additions and 4,441 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ if [ $exit_status = 0 ]; then
fi

# Generate a corpus for all mixed version variants
for config in local-mixed-22.2-23.1; do
for config in local-mixed-23.1; do
$BAZEL_BIN/pkg/cmd/bazci/bazci_/bazci test -- --config=ci \
//pkg/sql/logictest/tests/$config/... \
--test_arg=--declarative-corpus=$ARTIFACTS_DIR/corpus-mixed \
Expand Down
16 changes: 8 additions & 8 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ ALL_TESTS = [
"//pkg/ccl/logictestccl/tests/fakedist-vec-off:fakedist-vec-off_test",
"//pkg/ccl/logictestccl/tests/fakedist:fakedist_test",
"//pkg/ccl/logictestccl/tests/local-legacy-schema-changer:local-legacy-schema-changer_test",
"//pkg/ccl/logictestccl/tests/local-mixed-22.2-23.1:local-mixed-22_2-23_1_test",
"//pkg/ccl/logictestccl/tests/local-mixed-23.1:local-mixed-23_1_test",
"//pkg/ccl/logictestccl/tests/local-vec-off:local-vec-off_test",
"//pkg/ccl/logictestccl/tests/local:local_test",
"//pkg/ccl/logictestccl/tests/multiregion-15node-5region-3azs:multiregion-15node-5region-3azs_test",
Expand Down Expand Up @@ -452,7 +452,7 @@ ALL_TESTS = [
"//pkg/sql/logictest/tests/fakedist-vec-off:fakedist-vec-off_test",
"//pkg/sql/logictest/tests/fakedist:fakedist_test",
"//pkg/sql/logictest/tests/local-legacy-schema-changer:local-legacy-schema-changer_test",
"//pkg/sql/logictest/tests/local-mixed-22.2-23.1:local-mixed-22_2-23_1_test",
"//pkg/sql/logictest/tests/local-mixed-23.1:local-mixed-23_1_test",
"//pkg/sql/logictest/tests/local-vec-off:local-vec-off_test",
"//pkg/sql/logictest/tests/local:local_test",
"//pkg/sql/logictest/tests/multiregion-9node-3region-3azs:multiregion-9node-3region-3azs_test",
Expand All @@ -469,7 +469,7 @@ ALL_TESTS = [
"//pkg/sql/opt/exec/execbuilder/tests/fakedist-vec-off:fakedist-vec-off_test",
"//pkg/sql/opt/exec/execbuilder/tests/fakedist:fakedist_test",
"//pkg/sql/opt/exec/execbuilder/tests/local-legacy-schema-changer:local-legacy-schema-changer_test",
"//pkg/sql/opt/exec/execbuilder/tests/local-mixed-22.2-23.1:local-mixed-22_2-23_1_test",
"//pkg/sql/opt/exec/execbuilder/tests/local-mixed-23.1:local-mixed-23_1_test",
"//pkg/sql/opt/exec/execbuilder/tests/local-vec-off:local-vec-off_test",
"//pkg/sql/opt/exec/execbuilder/tests/local:local_test",
"//pkg/sql/opt/exec/execbuilder:execbuilder_test",
Expand Down Expand Up @@ -569,7 +569,7 @@ ALL_TESTS = [
"//pkg/sql/sqlitelogictest/tests/fakedist-vec-off:fakedist-vec-off_test",
"//pkg/sql/sqlitelogictest/tests/fakedist:fakedist_test",
"//pkg/sql/sqlitelogictest/tests/local-legacy-schema-changer:local-legacy-schema-changer_test",
"//pkg/sql/sqlitelogictest/tests/local-mixed-22.2-23.1:local-mixed-22_2-23_1_test",
"//pkg/sql/sqlitelogictest/tests/local-mixed-23.1:local-mixed-23_1_test",
"//pkg/sql/sqlitelogictest/tests/local-vec-off:local-vec-off_test",
"//pkg/sql/sqlitelogictest/tests/local:local_test",
"//pkg/sql/sqlliveness/slinstance:slinstance_test",
Expand Down Expand Up @@ -856,7 +856,7 @@ GO_TARGETS = [
"//pkg/ccl/logictestccl/tests/fakedist-vec-off:fakedist-vec-off_test",
"//pkg/ccl/logictestccl/tests/fakedist:fakedist_test",
"//pkg/ccl/logictestccl/tests/local-legacy-schema-changer:local-legacy-schema-changer_test",
"//pkg/ccl/logictestccl/tests/local-mixed-22.2-23.1:local-mixed-22_2-23_1_test",
"//pkg/ccl/logictestccl/tests/local-mixed-23.1:local-mixed-23_1_test",
"//pkg/ccl/logictestccl/tests/local-vec-off:local-vec-off_test",
"//pkg/ccl/logictestccl/tests/local:local_test",
"//pkg/ccl/logictestccl/tests/multiregion-15node-5region-3azs:multiregion-15node-5region-3azs_test",
Expand Down Expand Up @@ -1848,7 +1848,7 @@ GO_TARGETS = [
"//pkg/sql/logictest/tests/fakedist-vec-off:fakedist-vec-off_test",
"//pkg/sql/logictest/tests/fakedist:fakedist_test",
"//pkg/sql/logictest/tests/local-legacy-schema-changer:local-legacy-schema-changer_test",
"//pkg/sql/logictest/tests/local-mixed-22.2-23.1:local-mixed-22_2-23_1_test",
"//pkg/sql/logictest/tests/local-mixed-23.1:local-mixed-23_1_test",
"//pkg/sql/logictest/tests/local-vec-off:local-vec-off_test",
"//pkg/sql/logictest/tests/local:local_test",
"//pkg/sql/logictest/tests/multiregion-9node-3region-3azs:multiregion-9node-3region-3azs_test",
Expand All @@ -1874,7 +1874,7 @@ GO_TARGETS = [
"//pkg/sql/opt/exec/execbuilder/tests/fakedist-vec-off:fakedist-vec-off_test",
"//pkg/sql/opt/exec/execbuilder/tests/fakedist:fakedist_test",
"//pkg/sql/opt/exec/execbuilder/tests/local-legacy-schema-changer:local-legacy-schema-changer_test",
"//pkg/sql/opt/exec/execbuilder/tests/local-mixed-22.2-23.1:local-mixed-22_2-23_1_test",
"//pkg/sql/opt/exec/execbuilder/tests/local-mixed-23.1:local-mixed-23_1_test",
"//pkg/sql/opt/exec/execbuilder/tests/local-vec-off:local-vec-off_test",
"//pkg/sql/opt/exec/execbuilder/tests/local:local_test",
"//pkg/sql/opt/exec/execbuilder:execbuilder",
Expand Down Expand Up @@ -2113,7 +2113,7 @@ GO_TARGETS = [
"//pkg/sql/sqlitelogictest/tests/fakedist-vec-off:fakedist-vec-off_test",
"//pkg/sql/sqlitelogictest/tests/fakedist:fakedist_test",
"//pkg/sql/sqlitelogictest/tests/local-legacy-schema-changer:local-legacy-schema-changer_test",
"//pkg/sql/sqlitelogictest/tests/local-mixed-22.2-23.1:local-mixed-22_2-23_1_test",
"//pkg/sql/sqlitelogictest/tests/local-mixed-23.1:local-mixed-23_1_test",
"//pkg/sql/sqlitelogictest/tests/local-vec-off:local-vec-off_test",
"//pkg/sql/sqlitelogictest/tests/local:local_test",
"//pkg/sql/sqlitelogictest:sqlitelogictest",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
new-cluster name=s1 beforeVersion=23_1_Start disable-tenant
new-cluster name=s1 beforeVersion=23_2_Start disable-tenant
----

exec-sql
Expand All @@ -15,7 +15,7 @@ BACKUP INTO 'nodelocal://1/full_cluster_backup/';
# This is a cluster where the cluster version is behind the binary version. Such
# a condition only occurs when the user has upgraded the node to a new major
# version but has not yet finalized the upgrade.
new-cluster name=s2 beforeVersion=23_1_Start share-io-dir=s1 disable-tenant
new-cluster name=s2 beforeVersion=23_2_Start share-io-dir=s1 disable-tenant
----

exec-sql expect-error-regex=(pq: cluster restore not supported during major version upgrade: restore started at cluster version .* but binary version is.*)
Expand Down
14 changes: 7 additions & 7 deletions pkg/ccl/cliccl/ear_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,13 @@ func TestList(t *testing.T) {
000004.log:
env type: Data, AES128_CTR
keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
nonce: 31 d3 cd 5a 69 e2 13 64 21 53 57 64
counter: 3952287331
nonce: d1 05 79 53 68 35 a0 f1 44 01 22 79
counter: 1497766936
000005.sst:
env type: Data, AES128_CTR
keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
nonce: 23 d9 b2 e1 39 b0 87 ed f9 6d 49 20
counter: 3481614039
nonce: d0 b1 31 4b 08 b9 f6 08 7e e6 af 40
counter: 2167389540
COCKROACHDB_DATA_KEYS_000001_monolith:
env type: Store, AES128_CTR
keyID: f594229216d81add7811c4360212eb7629b578ef4eab6e5d05679b3c5de48867
Expand All @@ -190,11 +190,11 @@ marker.datakeys.000001.COCKROACHDB_DATA_KEYS_000001_monolith:
keyID: f594229216d81add7811c4360212eb7629b578ef4eab6e5d05679b3c5de48867
nonce: 55 d7 d4 27 6c 97 9b dd f1 5d 40 c8
counter: 467030050
marker.format-version.000009.010:
marker.format-version.000012.013:
env type: Data, AES128_CTR
keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
nonce: 6e 34 f4 3c 11 43 1a f5 69 ce 33 f1
counter: 2398097086
nonce: 86 a7 78 ad 4b da 62 56 d5 e2 d1 70
counter: 798955289
marker.manifest.000001.MANIFEST-000001:
env type: Data, AES128_CTR
keyID: bbb65a9d114c2a18740f27b6933b74f61018bd5adf545c153b48ffe6473336ef
Expand Down
3 changes: 2 additions & 1 deletion pkg/ccl/kvccl/kvtenantccl/upgradeccl/tenant_upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ func TestTenantUpgrade(t *testing.T) {
// - v0 corresponds to the bootstrapped version of the tenant,
// - v1, v2 correspond to adjacent releases.
func v0v1v2() (roachpb.Version, roachpb.Version, roachpb.Version) {
v0 := clusterversion.ByKey(clusterversion.V22_2)
v0 := clusterversion.ByKey(clusterversion.BinaryMinSupportedVersionKey)
v1 := clusterversion.TestingBinaryVersion
v2 := clusterversion.TestingBinaryVersion
if v1.Internal > 2 {
Expand All @@ -409,6 +409,7 @@ func v0v1v2() (roachpb.Version, roachpb.Version, roachpb.Version) {
func TestTenantUpgradeFailure(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
skip.WithIssue(t, 112209)

v0 := clusterversion.TestingBinaryMinSupportedVersion
v2 := clusterversion.TestingBinaryVersion
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/testdata/logic_test/changefeed
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# LogicTest: local local-mixed-22.2-23.1
# LogicTest: local local-mixed-23.1

statement ok
CREATE TABLE t ()
Expand Down
130 changes: 0 additions & 130 deletions pkg/ccl/logictestccl/testdata/logic_test/redact_descriptor
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ CREATE VIEW redacted_descriptors AS
# The legacy and declarative schema changer have slightly different implementation
# of `ADD COLUMN ... DEFAULT ... CHECK(...)`, resulting in slightly different
# descriptor. We will skip the config that uses legacy schema changer.
skipif config local-mixed-22.2-23.1
skipif config local-legacy-schema-changer
query T
SELECT descriptor from redacted_descriptors where id = 'collate_partition'::REGCLASS;
Expand Down Expand Up @@ -173,135 +172,6 @@ SELECT descriptor from redacted_descriptors where id = 'collate_partition'::REGC
}
}

onlyif config local-mixed-22.2-23.1
query T
SELECT descriptor from redacted_descriptors where id = 'collate_partition'::REGCLASS;
----
{
"table": {
"checks": [
{
"columnIds": [
2
],
"constraintId": 2,
"expr": "b != '_':::STRING",
"name": "check_b"
}
],
"columns": [
{
"id": 1,
"name": "a",
"type": {
"family": "CollatedStringFamily",
"locale": "da",
"oid": 25
}
},
{
"defaultExpr": "'_':::STRING",
"id": 2,
"name": "b",
"nullable": true,
"type": {
"family": "StringFamily",
"oid": 25
}
}
],
"createAsOfTime": {
"wallTime": "0"
},
"families": [
{
"columnIds": [
1,
2
],
"columnNames": [
"a",
"b"
],
"defaultColumnId": 2,
"name": "primary"
}
],
"formatVersion": 3,
"id": 106,
"modificationTime": {},
"name": "collate_partition",
"nextColumnId": 3,
"nextConstraintId": 3,
"nextFamilyId": 1,
"nextIndexId": 2,
"nextMutationId": 2,
"parentId": 104,
"primaryIndex": {
"compositeColumnIds": [
1
],
"constraintId": 1,
"createdAtNanos": "0",
"encodingType": 1,
"foreignKey": {},
"geoConfig": {},
"id": 1,
"interleave": {},
"keyColumnDirections": [
"ASC"
],
"keyColumnIds": [
1
],
"keyColumnNames": [
"a"
],
"name": "collate_partition_pkey",
"partitioning": {
"numColumns": 1,
"range": [
{
"fromInclusive": "Xw==",
"name": "p1",
"toExclusive": "Xw=="
}
]
},
"sharded": {},
"storeColumnIds": [
2
],
"storeColumnNames": [
"b"
],
"unique": true,
"version": 4
},
"privileges": {
"ownerProto": "root",
"users": [
{
"privileges": "2",
"userProto": "admin",
"withGrantOption": "2"
},
{
"privileges": "2",
"userProto": "root",
"withGrantOption": "2"
}
],
"version": 2
},
"replacementOf": {
"time": {}
},
"unexposedParentSchemaId": 105,
"version": "5"
}
}

statement ok
CREATE TABLE subpartition (
a INT, b INT, c INT,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
load("@io_bazel_rules_go//go:def.bzl", "go_test")

go_test(
name = "local-mixed-22_2-23_1_test",
name = "local-mixed-23_1_test",
size = "enormous",
srcs = ["generated_test.go"],
args = select({
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

51 changes: 30 additions & 21 deletions pkg/ccl/serverccl/server_startup_guardrails_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,26 @@ import (
func TestServerStartupGuardrails(t *testing.T) {
defer leaktest.AfterTest(t)()

// We need to conditionally apply the DevOffset for the version
// returned by this function to work both on master (where the dev
// offset applies) and on release branches (where it doesn't).
v := func(major, minor int32) roachpb.Version {
binaryVersion := clusterversion.ByKey(clusterversion.BinaryVersionKey)
var offset int32
if binaryVersion.Major > clusterversion.DevOffset {
offset = clusterversion.DevOffset
// The tests below will use the minimum supported version as the logical
// version.
logicalVersionKey := clusterversion.BinaryMinSupportedVersionKey
logicalVersion := clusterversion.ByKey(logicalVersionKey)

prev := func(v roachpb.Version) roachpb.Version {
t.Helper()
if v.Minor < 1 || v.Minor > 2 || v.Patch != 0 || v.Internal != 0 {
t.Fatalf("invalid version %v", v)
}
if v.Minor > 1 {
v.Minor--
} else {
v.Major--
v.Minor = 2
}
return roachpb.Version{Major: offset + major, Minor: minor}
return v
}
minusOne := prev(logicalVersion)
minusTwo := prev(minusOne)

tests := []struct {
storageBinaryVersion roachpb.Version
Expand All @@ -53,23 +62,23 @@ func TestServerStartupGuardrails(t *testing.T) {
// First test case ensures that a tenant server can start if the server binary
// version is not too low for the tenant logical version.
{
storageBinaryVersion: v(22, 2),
storageBinaryMinSupportedVersion: v(22, 1),
tenantBinaryVersion: v(22, 2),
tenantBinaryMinSupportedVersion: v(22, 2),
TenantLogicalVersionKey: clusterversion.V22_2,
storageBinaryVersion: logicalVersion,
storageBinaryMinSupportedVersion: minusOne,
tenantBinaryVersion: logicalVersion,
tenantBinaryMinSupportedVersion: logicalVersion,
TenantLogicalVersionKey: logicalVersionKey,
expErrMatch: "",
},
// Second test case ensures that a tenant server is prevented from starting if
// its binary version is too low for the current tenant logical version.
{
storageBinaryVersion: v(22, 2),
storageBinaryMinSupportedVersion: v(22, 1),
tenantBinaryVersion: v(22, 1),
tenantBinaryMinSupportedVersion: v(21, 2),
TenantLogicalVersionKey: clusterversion.V22_2,
storageBinaryVersion: logicalVersion,
storageBinaryMinSupportedVersion: minusOne,
tenantBinaryVersion: minusOne,
tenantBinaryMinSupportedVersion: minusTwo,
TenantLogicalVersionKey: logicalVersionKey,
expErrMatch: fmt.Sprintf("preventing SQL server from starting because its binary version is too low for the tenant active version: "+
"server binary version = %v, tenant active version = %v", v(22, 1), v(22, 2)),
"server binary version = %v, tenant active version = %v", minusOne, logicalVersion),
},
}

Expand All @@ -89,7 +98,7 @@ func TestServerStartupGuardrails(t *testing.T) {
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
BinaryVersionOverride: test.storageBinaryVersion,
BootstrapVersionKeyOverride: clusterversion.V22_2,
BootstrapVersionKeyOverride: logicalVersionKey,
DisableAutomaticVersionUpgrade: make(chan struct{}),
},
SQLEvalContext: &eval.TestingKnobs{
Expand Down
Loading

0 comments on commit 5ee9b6c

Please sign in to comment.