Skip to content

Commit

Permalink
upgrades: add helper function to bump SystemDatabaseSchemaVersion
Browse files Browse the repository at this point in the history
This patch adds a helper function that should be used by upgrades that
create or modify the schema of system tables to bump the schema version
field on the `system` database descriptor.

Release note: None
  • Loading branch information
andyyang890 committed Oct 2, 2023
1 parent 9b0ad5c commit d793e67
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 3 deletions.
1 change: 1 addition & 0 deletions pkg/upgrade/upgrades/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ go_test(
"//pkg/util/intsets",
"//pkg/util/leaktest",
"//pkg/util/log",
"//pkg/util/protoutil",
"//pkg/util/syncutil",
"//pkg/util/timeutil",
"@com_github_cockroachdb_cockroach_go_v2//crdb",
Expand Down
4 changes: 2 additions & 2 deletions pkg/upgrade/upgrades/create_region_liveness.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (

// createRegionLivenessTables creates the system.region_liveness table.
func createRegionLivenessTables(
ctx context.Context, _ clusterversion.ClusterVersion, d upgrade.TenantDeps,
ctx context.Context, cs clusterversion.ClusterVersion, d upgrade.TenantDeps,
) error {
setDBLocality := false
// Since this is a re-use of an old key space, invalid data might exists,
Expand Down Expand Up @@ -96,5 +96,5 @@ func createRegionLivenessTables(
}
}

return nil
return bumpSystemDatabaseSchemaVersion(ctx, cs, d)
}
2 changes: 2 additions & 0 deletions pkg/upgrade/upgrades/create_region_liveness_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,6 @@ func TestRegionLivenessTableMigration(t *testing.T) {

_, err := db.Exec("SELECT * FROM system.region_liveness")
assert.NoError(t, err, "system.region_liveness exists")

upgrades.ValidateSystemDatabaseSchemaVersionBumped(t, db, clusterversion.V23_2_RegionaLivenessTable)
}
28 changes: 28 additions & 0 deletions pkg/upgrade/upgrades/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (

"github.com/cockroachdb/cockroach-go/v2/crdb"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
Expand All @@ -26,6 +27,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -198,3 +200,29 @@ func ExecForCountInTxns(
require.NoError(t, err)
}
}

// ValidateSystemDatabaseSchemaVersionBumped validates that the system database
// schema version has been bumped to the expected version.
func ValidateSystemDatabaseSchemaVersionBumped(
t *testing.T, sqlDB *gosql.DB, expectedVersion clusterversion.Key,
) {
expectedSchemaVersion := clusterversion.ByKey(expectedVersion)

var actualSchemaVersionBytes []byte
require.NoError(t, sqlDB.QueryRow(
`SELECT crdb_internal.json_to_pb(
'cockroach.roachpb.Version',
crdb_internal.pb_to_json(
'cockroach.sql.sqlbase.Descriptor',
descriptor,
false
)->'database'->'systemDatabaseSchemaVersion')
FROM system.descriptor WHERE id = $1`,
keys.SystemDatabaseID,
).Scan(&actualSchemaVersionBytes))

actualSchemaVersion := roachpb.Version{}
require.NoError(t, protoutil.Unmarshal(actualSchemaVersionBytes, &actualSchemaVersion))

require.Equal(t, expectedSchemaVersion, actualSchemaVersion)
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,5 @@ func stmtDiagForPlanGistMigration(
return err
}
}
return nil
return bumpSystemDatabaseSchemaVersion(ctx, cs, d)
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ func TestStmtDiagForPlanGistMigration(t *testing.T) {
)
// Validate that the table has new schema.
validateSchemaExists(true)

upgrades.ValidateSystemDatabaseSchemaVersionBumped(t, sqlDB, clusterversion.V23_2_StmtDiagForPlanGist)
}

// getOldStmtDiagReqsDescriptor returns the
Expand Down
26 changes: 26 additions & 0 deletions pkg/upgrade/upgrades/schema_changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/upgrade"
"github.com/cockroachdb/cockroach/pkg/util/log"
Expand Down Expand Up @@ -411,3 +412,28 @@ func onlyHasColumnFamily(
}
return true, nil
}

// bumpSystemDatabaseSchemaVersion bumps the SystemDatabaseSchemaVersion
// field for the system database descriptor. It should be called at the end
// of any upgrade that creates or modifies the schema of any system table.
func bumpSystemDatabaseSchemaVersion(
ctx context.Context, cs clusterversion.ClusterVersion, d upgrade.TenantDeps,
) error {
return d.DB.DescsTxn(ctx, func(ctx context.Context, txn descs.Txn) error {
systemDBDesc, err := txn.Descriptors().MutableByName(txn.KV()).Database(ctx, catconstants.SystemDatabaseName)
if err != nil {
return err
}
if sv := systemDBDesc.GetSystemDatabaseSchemaVersion(); sv != nil {
if cs.Version.Less(*sv) {
return errors.AssertionFailedf(
"new system schema version (%#v) is lower than previous system schema version (%#v)",
cs.Version,
*sv,
)
}
}
systemDBDesc.SystemDatabaseSchemaVersion = &cs.Version
return txn.Descriptors().WriteDesc(ctx, false /* kvTrace */, systemDBDesc, txn.KV())
})
}

0 comments on commit d793e67

Please sign in to comment.