From 570b8dd6decf1aafd1260a678b166e75250fd31c Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Wed, 14 Dec 2022 10:43:46 -0500 Subject: [PATCH] upgrades: fix desc ID sequence definition to match expectation Before this change, the test fails with: ``` Diff: @@ -29,11 +29,11 @@ "lastUpdated" TIMESTAMP NOT NULL DEFAULT now():::TIMESTAMP, "valueType" STRING NULL, CONSTRAINT "primary" PRIMARY KEY (name ASC), FAMILY "fam_0_name_value_lastUpdated_valueType" (name, value, "lastUpdated", "valueType") ); -CREATE SEQUENCE public.descriptor_id_seq MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1; +CREATE SEQUENCE public.descriptor_id_seq MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 104; CREATE TABLE public.tenants ( id INT8 NOT NULL, active BOOL NOT NULL DEFAULT true, info BYTES NULL, name STRING NULL AS (crdb_internal.pb_to_json('cockroach.sql.sqlbase.TenantInfo':::STRING, info)->>'name':::STRING) VIRTUAL, ---- ``` This was revealed by #93487 which fixed the broken roachtest. Expect a few more of these. Fixes: #93602 Release note: None --- pkg/cmd/roachtest/tests/BUILD.bazel | 1 + ...alidate_system_schema_after_version_upgrade.go | 15 +++++++++++++-- .../desc_id_sequence_for_system_tenant.go | 15 +++++++++++---- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/roachtest/tests/BUILD.bazel b/pkg/cmd/roachtest/tests/BUILD.bazel index 163cd7ee5e94..1552056f1301 100644 --- a/pkg/cmd/roachtest/tests/BUILD.bazel +++ b/pkg/cmd/roachtest/tests/BUILD.bazel @@ -247,6 +247,7 @@ go_library( "@com_github_kr_pretty//:pretty", "@com_github_lib_pq//:pq", "@com_github_montanaflynn_stats//:stats", + "@com_github_pmezard_go_difflib//difflib", "@com_github_prometheus_client_golang//api", "@com_github_prometheus_client_golang//api/prometheus/v1:prometheus", "@com_github_prometheus_common//model", diff --git a/pkg/cmd/roachtest/tests/validate_system_schema_after_version_upgrade.go b/pkg/cmd/roachtest/tests/validate_system_schema_after_version_upgrade.go index 716593ea90e9..2e7ecd1099a6 100644 --- a/pkg/cmd/roachtest/tests/validate_system_schema_after_version_upgrade.go +++ b/pkg/cmd/roachtest/tests/validate_system_schema_after_version_upgrade.go @@ -20,6 +20,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" + "github.com/pmezard/go-difflib/difflib" ) func registerValidateSystemSchemaAfterVersionUpgrade(r registry.Registry) { @@ -85,8 +86,18 @@ func registerValidateSystemSchemaAfterVersionUpgrade(r registry.Registry) { validateEquivalenceStep := func(str1, str2 *string) versionStep { return func(ctx context.Context, t test.Test, u *versionUpgradeTest) { if *str1 != *str2 { - t.Fatal("After upgrading, `USE system; SHOW CREATE ALL TABLES;` " + - "does not match expected output after version upgrade.\n") + diff, diffErr := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{ + A: difflib.SplitLines(*str1), + B: difflib.SplitLines(*str2), + Context: 5, + }) + if diffErr != nil { + diff = diffErr.Error() + t.Errorf("failed to produce diff: %v", diffErr) + } + t.Fatalf("After upgrading, `USE system; SHOW CREATE ALL TABLES;` "+ + "does not match expected output after version upgrade."+ + "\nDiff:\n%s", diff) } t.L().Printf("validating succeeded:\n%v", *str1) } diff --git a/pkg/upgrade/upgrades/desc_id_sequence_for_system_tenant.go b/pkg/upgrade/upgrades/desc_id_sequence_for_system_tenant.go index d39bd213549e..b176de7ba8e6 100644 --- a/pkg/upgrade/upgrades/desc_id_sequence_for_system_tenant.go +++ b/pkg/upgrade/upgrades/desc_id_sequence_for_system_tenant.go @@ -27,14 +27,21 @@ func descIDSequenceForSystemTenant( if !d.Codec.ForSystemTenant() { return nil } - mut := tabledesc.NewBuilder(systemschema.DescIDSequence.TableDesc()).BuildCreatedMutableTable() + mut := tabledesc.NewBuilder(systemschema.DescIDSequence.TableDesc()). + BuildCreatedMutableTable() return d.DB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { oldEntry, err := txn.GetForUpdate(ctx, keys.LegacyDescIDGenerator) if err != nil { return err } - mut.SequenceOpts.Start = oldEntry.ValueInt() - _, _, err = CreateSystemTableInTxn(ctx, d.Settings, txn, keys.SystemSQLCodec, mut) - return err + id, created, err := CreateSystemTableInTxn( + ctx, d.Settings, txn, keys.SystemSQLCodec, mut, + ) + if err != nil || !created { + return err + } + // Install the appropriate value for the sequence. Note that we use the + // existence of the sequence above to make this transaction idempotent. + return txn.Put(ctx, d.Codec.SequenceKey(uint32(id)), oldEntry.ValueInt()) }) }