catalog/bootstrap: add option to offset ids during system table creation

msbutler · msbutler · commit 008a5f49ab60 · 2025-09-11T16:13:25.000-04:00
This patch adds an option to MakeSetadataSchema to offset the dynamically allocated system table ids. So, if the caller sets this to say 1000, the vc will still have ids 0-49, but then ids 1050,1051, etc. A future patch will leverage this option during PCR reader tenant creation to ensure that replicating user table ids never collide with the reader tenant's system table ids. Informs #152909 Release note: none
diff --git a/pkg/config/system_test.go b/pkg/config/system_test.go
@@ -203,7 +203,7 @@ func TestGetLargestID(t *testing.T) {
 
 		// Real SQL layout.
 		func() testCase {
-			ms := bootstrap.MakeMetadataSchema(keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef())
+			ms := bootstrap.MakeMetadataSchema(keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(), bootstrap.NoOffset)
 			descIDs := ms.DescriptorIDs()
 			maxDescID := config.ObjectID(descIDs[len(descIDs)-1])
 			kvs, _ /* splits */ := ms.GetInitialValues()
@@ -321,7 +321,7 @@ func TestComputeSplitKeySystemRanges(t *testing.T) {
 
 	cfg := config.NewSystemConfig(zonepb.DefaultZoneConfigRef())
 	kvs, _ /* splits */ := bootstrap.MakeMetadataSchema(
-		keys.SystemSQLCodec, cfg.DefaultZoneConfig, zonepb.DefaultSystemZoneConfigRef(),
+		keys.SystemSQLCodec, cfg.DefaultZoneConfig, zonepb.DefaultSystemZoneConfigRef(), bootstrap.NoOffset,
 	).GetInitialValues()
 	cfg.SystemConfigEntries = config.SystemConfigEntries{
 		Values: kvs,
@@ -353,7 +353,7 @@ func TestComputeSplitKeyTableIDs(t *testing.T) {
 	minKey := roachpb.RKey(keys.TimeseriesPrefix.PrefixEnd())
 
 	schema := bootstrap.MakeMetadataSchema(
-		keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(),
+		keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(), bootstrap.NoOffset,
 	)
 	// Real system tables only.
 	baseSql, _ /* splits */ := schema.GetInitialValues()
@@ -460,7 +460,7 @@ func TestComputeSplitKeyTenantBoundaries(t *testing.T) {
 	minTenID, maxTenID := roachpb.MinTenantID.ToUint64(), roachpb.MaxTenantID.ToUint64()
 
 	schema := bootstrap.MakeMetadataSchema(
-		keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(),
+		keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(), bootstrap.NoOffset,
 	)
 	minKey := tkey(bootstrap.TestingUserDescID(0))
 
@@ -599,7 +599,7 @@ func TestGetZoneConfigForKey(t *testing.T) {
 	cfg := config.NewSystemConfig(zonepb.DefaultZoneConfigRef())
 
 	kvs, _ /* splits */ := bootstrap.MakeMetadataSchema(
-		keys.SystemSQLCodec, cfg.DefaultZoneConfig, zonepb.DefaultSystemZoneConfigRef(),
+		keys.SystemSQLCodec, cfg.DefaultZoneConfig, zonepb.DefaultSystemZoneConfigRef(), bootstrap.NoOffset,
 	).GetInitialValues()
 	cfg.SystemConfigEntries = config.SystemConfigEntries{
 		Values: kvs,
diff --git a/pkg/crosscluster/physical/stream_ingestion_planning.go b/pkg/crosscluster/physical/stream_ingestion_planning.go
@@ -331,7 +331,7 @@ func createReaderTenant(
 		}
 
 		readerInfo.ID = readerID.ToUint64()
-		_, err = sql.BootstrapTenant(ctx, p.ExecCfg(), p.Txn(), readerInfo, readerZcfg)
+		_, err = sql.BootstrapTenant(ctx, p.ExecCfg(), p.Txn(), readerInfo, readerZcfg, 0)
 		if err != nil {
 			return readerID, err
 		}
diff --git a/pkg/kv/kvserver/store_test.go b/pkg/kv/kvserver/store_test.go
@@ -128,7 +128,7 @@ type testStoreOpts struct {
 
 func (opts *testStoreOpts) splits() (_kvs []roachpb.KeyValue, _splits []roachpb.RKey) {
 	kvs, splits := bootstrap.MakeMetadataSchema(
-		keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(),
+		keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(), bootstrap.NoOffset,
 	).GetInitialValues()
 	if !opts.createSystemRanges {
 		return kvs, nil
diff --git a/pkg/server/node.go b/pkg/server/node.go
@@ -452,10 +452,10 @@ func allocateStoreIDs(
 
 // GetBootstrapSchema returns the schema which will be used to bootstrap a new
 // server.
-func GetBootstrapSchema(
+func GetBootstrapSchemaForTest(
 	defaultZoneConfig *zonepb.ZoneConfig, defaultSystemZoneConfig *zonepb.ZoneConfig,
 ) bootstrap.MetadataSchema {
-	return bootstrap.MakeMetadataSchema(keys.SystemSQLCodec, defaultZoneConfig, defaultSystemZoneConfig)
+	return bootstrap.MakeMetadataSchema(keys.SystemSQLCodec, defaultZoneConfig, defaultSystemZoneConfig, bootstrap.NoOffset)
 }
 
 // bootstrapCluster initializes the passed-in engines for a new cluster.
diff --git a/pkg/server/node_test.go b/pkg/server/node_test.go
@@ -105,7 +105,7 @@ func TestBootstrapCluster(t *testing.T) {
 	}
 
 	// Add the initial keys for sql.
-	kvs, tableSplits := GetBootstrapSchema(
+	kvs, tableSplits := GetBootstrapSchemaForTest(
 		zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(),
 	).GetInitialValues()
 	for _, kv := range kvs {
diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go
@@ -1886,7 +1886,7 @@ func ExpectedInitialRangeCount(
 	defaultZoneConfig *zonepb.ZoneConfig,
 	defaultSystemZoneConfig *zonepb.ZoneConfig,
 ) (int, error) {
-	_, splits := bootstrap.MakeMetadataSchema(codec, defaultZoneConfig, defaultSystemZoneConfig).GetInitialValues()
+	_, splits := bootstrap.MakeMetadataSchema(codec, defaultZoneConfig, defaultSystemZoneConfig, bootstrap.NoOffset).GetInitialValues()
 	// N splits means N+1 ranges.
 	return len(config.StaticSplits()) + len(splits) + 1, nil
 }
diff --git a/pkg/sql/catalog/bootstrap/BUILD.bazel b/pkg/sql/catalog/bootstrap/BUILD.bazel
@@ -62,6 +62,7 @@ go_test(
         "//pkg/security/securityassets",
         "//pkg/security/securitytest",
         "//pkg/server",
+        "//pkg/sql/catalog/descpb",
         "//pkg/testutils",
         "//pkg/testutils/datapathutils",
         "//pkg/testutils/serverutils",
diff --git a/pkg/sql/catalog/bootstrap/bootstrap_test.go b/pkg/sql/catalog/bootstrap/bootstrap_test.go
@@ -12,6 +12,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/config/zonepb"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/testutils/datapathutils"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -132,5 +133,27 @@ func makeMetadataSchema(tenantID uint64) MetadataSchema {
 	if tenantID > 0 {
 		codec = keys.MakeSQLCodec(roachpb.MustMakeTenantID(tenantID))
 	}
-	return MakeMetadataSchema(codec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef())
+	return MakeMetadataSchema(codec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(), NoOffset)
+}
+
+func TestDynamicSystemTableIDOffset(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	offset := uint32(1000)
+
+	defaultMetadata := MakeMetadataSchema(keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(), NoOffset)
+	offsetMetadata := MakeMetadataSchema(keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(), offset)
+
+	require.Len(t, defaultMetadata.descs, len(offsetMetadata.descs))
+
+	for i := range defaultMetadata.descs {
+		defaultID := defaultMetadata.descs[i].GetID()
+		if defaultID <= keys.MaxReservedDescID {
+			// Reserved IDs are not offset.
+			require.Equal(t, defaultID, offsetMetadata.descs[i].GetID())
+		} else {
+			require.Equal(t, defaultMetadata.descs[i].GetID()+descpb.ID(offset), offsetMetadata.descs[i].GetID())
+		}
+	}
 }
diff --git a/pkg/sql/catalog/bootstrap/initial_values.go b/pkg/sql/catalog/bootstrap/initial_values.go
@@ -23,10 +23,11 @@ import (
 // InitialValuesOpts is used to get initial values for system/secondary tenants
 // and allows overriding initial values with ones from previous releases.
 type InitialValuesOpts struct {
-	DefaultZoneConfig       *zonepb.ZoneConfig
-	DefaultSystemZoneConfig *zonepb.ZoneConfig
-	OverrideKey             clusterversion.Key
-	Codec                   keys.SQLCodec
+	DefaultZoneConfig          *zonepb.ZoneConfig
+	DefaultSystemZoneConfig    *zonepb.ZoneConfig
+	OverrideKey                clusterversion.Key
+	Codec                      keys.SQLCodec
+	DynamicSystemTableIDOffset uint32
 }
 
 // GenerateInitialValues generates the initial values with which to bootstrap a
@@ -84,7 +85,7 @@ var initialValuesFactoryByKey = map[clusterversion.Key]initialValuesFactoryFn{
 func buildLatestInitialValues(
 	opts InitialValuesOpts,
 ) (kvs []roachpb.KeyValue, splits []roachpb.RKey, _ error) {
-	schema := MakeMetadataSchema(opts.Codec, opts.DefaultZoneConfig, opts.DefaultSystemZoneConfig)
+	schema := MakeMetadataSchema(opts.Codec, opts.DefaultZoneConfig, opts.DefaultSystemZoneConfig, opts.DynamicSystemTableIDOffset)
 	kvs, splits = schema.GetInitialValues()
 	return kvs, splits, nil
 }
diff --git a/pkg/sql/catalog/bootstrap/metadata.go b/pkg/sql/catalog/bootstrap/metadata.go
@@ -43,16 +43,23 @@ type MetadataSchema struct {
 	otherSplitIDs []uint32
 	otherKV       []roachpb.KeyValue
 	ids           catalog.DescriptorIDSet
+
+	// dynamicSystemTableIDOffset offsets the dynamically allocated IDs. So, if
+	// this is set to 500, the ids will be 501, 502, etc.
+	dynamicSystemTableIDOffset uint32
 }
 
+const NoOffset = 0
+
 // MakeMetadataSchema constructs a new MetadataSchema value which constructs
 // the "system" database.
 func MakeMetadataSchema(
 	codec keys.SQLCodec,
 	defaultZoneConfig *zonepb.ZoneConfig,
 	defaultSystemZoneConfig *zonepb.ZoneConfig,
+	dynamicSystemTableIDOffset uint32,
 ) MetadataSchema {
-	ms := MetadataSchema{codec: codec}
+	ms := MetadataSchema{codec: codec, dynamicSystemTableIDOffset: dynamicSystemTableIDOffset}
 	addSystemDatabaseToSchema(&ms, defaultZoneConfig, defaultSystemZoneConfig)
 	return ms
 }
@@ -347,7 +354,7 @@ func (ms MetadataSchema) FirstNonSystemDescriptorID() descpb.ID {
 }
 
 func (ms MetadataSchema) allocateID() (nextID descpb.ID) {
-	maxID := descpb.ID(keys.MaxReservedDescID)
+	maxID := descpb.ID(keys.MaxReservedDescID) + descpb.ID(ms.dynamicSystemTableIDOffset)
 	for _, d := range ms.descs {
 		if d.GetID() > maxID {
 			maxID = d.GetID()
@@ -623,7 +630,7 @@ func addSystemTenantEntry(target *MetadataSchema) {
 }
 
 func testingMinUserDescID(codec keys.SQLCodec) uint32 {
-	ms := MakeMetadataSchema(codec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef())
+	ms := MakeMetadataSchema(codec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(), NoOffset)
 	return uint32(ms.FirstNonSystemDescriptorID())
 }
 
@@ -659,7 +666,7 @@ func GetAndHashInitialValuesToString(tenantID uint64) (initialValues string, has
 	if tenantID > 0 {
 		codec = keys.MakeSQLCodec(roachpb.MustMakeTenantID(tenantID))
 	}
-	ms := MakeMetadataSchema(codec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef())
+	ms := MakeMetadataSchema(codec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(), NoOffset)
 
 	initialValues = InitialValuesToString(ms)
 	h := sha256.Sum256([]byte(initialValues))
diff --git a/pkg/sql/catalog/internal/catkv/system_database_cache.go b/pkg/sql/catalog/internal/catkv/system_database_cache.go
@@ -50,6 +50,7 @@ func NewSystemDatabaseCache(codec keys.SQLCodec, settings *cluster.Settings) *Sy
 		codec,
 		zonepb.DefaultZoneConfigRef(),
 		zonepb.DefaultSystemZoneConfigRef(),
+		bootstrap.NoOffset,
 	)
 	var warm nstree.MutableCatalog
 	_ = ms.ForEachCatalogDescriptor(func(desc catalog.Descriptor) error {
diff --git a/pkg/sql/doctor/doctor.go b/pkg/sql/doctor/doctor.go
@@ -271,7 +271,7 @@ func descReport(stdout io.Writer, desc catalog.Descriptor, format string, args .
 // timestamp.
 func DumpSQL(out io.Writer, descTable DescriptorTable, namespaceTable NamespaceTable) error {
 	// Assume the target is an empty cluster with the same binary version
-	ms := bootstrap.MakeMetadataSchema(keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef())
+	ms := bootstrap.MakeMetadataSchema(keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(), bootstrap.NoOffset)
 	minUserDescID := ms.FirstNonSystemDescriptorID()
 	minUserCreatedDescID := minUserDescID + descpb.ID(len(catalogkeys.DefaultUserDBs))*2
 	// Print first transaction, which removes all predefined user descriptors.
diff --git a/pkg/sql/tenant_creation.go b/pkg/sql/tenant_creation.go
@@ -136,7 +136,7 @@ func (p *planner) createTenantInternal(
 		return tid, nil
 	}
 
-	return BootstrapTenant(ctx, p.execCfg, p.Txn(), info, initialTenantZoneConfig)
+	return BootstrapTenant(ctx, p.execCfg, p.Txn(), info, initialTenantZoneConfig, bootstrap.NoOffset)
 }
 
 // BootstrapTenant bootstraps the span of the newly created tenant identified in
@@ -147,6 +147,7 @@ func BootstrapTenant(
 	txn *kv.Txn,
 	info mtinfopb.TenantInfoWithUsage,
 	zfcg *zonepb.ZoneConfig,
+	dynamicSystemTableIDOffset uint32,
 ) (roachpb.TenantID, error) {
 	tid := roachpb.MustMakeTenantID(info.ID)
 
@@ -182,10 +183,11 @@ func BootstrapTenant(
 	}
 
 	initialValuesOpts := bootstrap.InitialValuesOpts{
-		DefaultZoneConfig:       zfcg,
-		DefaultSystemZoneConfig: zfcg,
-		OverrideKey:             bootstrapVersionOverride,
-		Codec:                   codec,
+		DefaultZoneConfig:          zfcg,
+		DefaultSystemZoneConfig:    zfcg,
+		OverrideKey:                bootstrapVersionOverride,
+		Codec:                      codec,
+		DynamicSystemTableIDOffset: dynamicSystemTableIDOffset,
 	}
 	kvs, splits, err := initialValuesOpts.GenerateInitialValues()
 	if err != nil {
diff --git a/pkg/sql/tests/system_table_test.go b/pkg/sql/tests/system_table_test.go
@@ -54,7 +54,7 @@ func TestInitialKeys(t *testing.T) {
 			nonDescKeys = 8
 		}
 
-		ms := bootstrap.MakeMetadataSchema(codec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef())
+		ms := bootstrap.MakeMetadataSchema(codec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(), bootstrap.NoOffset)
 		kv, _ /* splits */ := ms.GetInitialValues()
 		expected := nonDescKeys + keysPerDesc*ms.SystemDescriptorCount()
 		if actual := len(kv); actual != expected {
@@ -129,7 +129,7 @@ func TestInitialKeysAndSplits(t *testing.T) {
 			}
 
 			ms := bootstrap.MakeMetadataSchema(
-				codec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(),
+				codec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(), bootstrap.NoOffset,
 			)
 			kvs, splits := ms.GetInitialValues()
 
diff --git a/pkg/testutils/localtestcluster/local_test_cluster.go b/pkg/testutils/localtestcluster/local_test_cluster.go
@@ -288,7 +288,7 @@ func (ltc *LocalTestCluster) Start(t testing.TB, initFactory InitFactoryFn) {
 	var splits []roachpb.RKey
 	if !ltc.DontCreateSystemRanges {
 		schema := bootstrap.MakeMetadataSchema(
-			keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(),
+			keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(), bootstrap.NoOffset,
 		)
 		var tableSplits []roachpb.RKey
 		initialValues, tableSplits = schema.GetInitialValues()

Original file line number	Diff line number	Diff line change
`@@ -331,7 +331,7 @@ func createReaderTenant(`
`331`	`331`	`}`
`332`	`332`
`333`	`333`	`readerInfo.ID = readerID.ToUint64()`
`334`		`- _, err = sql.BootstrapTenant(ctx, p.ExecCfg(), p.Txn(), readerInfo, readerZcfg)`
	`334`	`+ _, err = sql.BootstrapTenant(ctx, p.ExecCfg(), p.Txn(), readerInfo, readerZcfg, 0)`
`335`	`335`	`if err != nil {`
`336`	`336`	`return readerID, err`
`337`	`337`	`}`
Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,7 @@ func TestBootstrapCluster(t *testing.T) {`
`105`	`105`	`}`
`106`	`106`
`107`	`107`	`// Add the initial keys for sql.`
`108`		`- kvs, tableSplits := GetBootstrapSchema(`
	`108`	`+ kvs, tableSplits := GetBootstrapSchemaForTest(`
`109`	`109`	`zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(),`
`110`	`110`	`).GetInitialValues()`
`111`	`111`	`for _, kv := range kvs {`
Original file line number	Diff line number	Diff line change
`@@ -1886,7 +1886,7 @@ func ExpectedInitialRangeCount(`
`1886`	`1886`	`defaultZoneConfig *zonepb.ZoneConfig,`
`1887`	`1887`	`defaultSystemZoneConfig *zonepb.ZoneConfig,`
`1888`	`1888`	`) (int, error) {`
`1889`		`- _, splits := bootstrap.MakeMetadataSchema(codec, defaultZoneConfig, defaultSystemZoneConfig).GetInitialValues()`
	`1889`	`+ _, splits := bootstrap.MakeMetadataSchema(codec, defaultZoneConfig, defaultSystemZoneConfig, bootstrap.NoOffset).GetInitialValues()`
`1890`	`1890`	`// N splits means N+1 ranges.`
`1891`	`1891`	`return len(config.StaticSplits()) + len(splits) + 1, nil`
`1892`	`1892`	`}`