sql: don't create system.zones table for secondary tenants

Relates to cockroachdb#48375.

Secondary tenants will not have a say over zone configurations, so
there's no reason to give them empty system.zones tables. This commit
addresses this by removing the table from secondary tenants. This allows
us to avoid populating this table with default zone configurations,
which would have been ignored.

pkg/config/keys.go

@@ -18,12 +18,12 @@ import (
 // MakeZoneKeyPrefix returns the key prefix for id's row in the system.zones
 // table.
 func MakeZoneKeyPrefix(id uint32) roachpb.Key {
-	return keys.TODOSQLCodec.ZoneKeyPrefix(id)
+	return keys.SystemSQLCodec.ZoneKeyPrefix(id)
 }
 
 // MakeZoneKey returns the key for id's entry in the system.zones table.
 func MakeZoneKey(id uint32) roachpb.Key {
-	return keys.TODOSQLCodec.ZoneKey(id)
+	return keys.SystemSQLCodec.ZoneKey(id)
 }
 
 // DecodeObjectID decodes the object ID from the front of key. It returns the

pkg/keys/sql.go

@@ -142,12 +142,18 @@ func (e sqlEncoder) DescIDSequenceKey() roachpb.Key {
 
 // ZoneKeyPrefix returns the key prefix for id's row in the system.zones table.
 func (e sqlEncoder) ZoneKeyPrefix(id uint32) roachpb.Key {
+	if !e.ForSystemTenant() {
+		panic("zone keys only exist in the system tenant's keyspace")
+	}
 	k := e.IndexPrefix(ZonesTableID, ZonesTablePrimaryIndexID)
 	return encoding.EncodeUvarintAscending(k, uint64(id))
 }
 
 // ZoneKey returns the key for id's entry in the system.zones table.
 func (e sqlEncoder) ZoneKey(id uint32) roachpb.Key {
+	if !e.ForSystemTenant() {
+		panic("zone keys only exist in the system tenant's keyspace")
+	}
 	k := e.ZoneKeyPrefix(id)
 	return MakeFamilyKey(k, uint32(ZonesTableConfigColumnID))
 }

pkg/server/node.go

@@ -188,11 +188,9 @@ func allocateStoreIDs(
 // GetBootstrapSchema returns the schema which will be used to bootstrap a new
 // server.
 func GetBootstrapSchema(
-	codec keys.SQLCodec,
-	defaultZoneConfig *zonepb.ZoneConfig,
-	defaultSystemZoneConfig *zonepb.ZoneConfig,
+	defaultZoneConfig *zonepb.ZoneConfig, defaultSystemZoneConfig *zonepb.ZoneConfig,
 ) sqlbase.MetadataSchema {
-	return sqlbase.MakeMetadataSchema(codec, defaultZoneConfig, defaultSystemZoneConfig)
+	return sqlbase.MakeMetadataSchema(keys.SystemSQLCodec, defaultZoneConfig, defaultSystemZoneConfig)
 }
 
 // bootstrapCluster initializes the passed-in engines for a new cluster.
@@ -244,7 +242,7 @@ func bootstrapCluster(
 		// not create the range, just its data. Only do this if this is the
 		// first store.
 		if i == 0 {
-			schema := GetBootstrapSchema(keys.SystemSQLCodec, defaultZoneConfig, defaultSystemZoneConfig)
+			schema := GetBootstrapSchema(defaultZoneConfig, defaultSystemZoneConfig)
 			initialValues, tableSplits := schema.GetInitialValues()
 			splits := append(config.StaticSplits(), tableSplits...)
 			sort.Slice(splits, func(i, j int) bool {

pkg/server/node_test.go

@@ -90,7 +90,7 @@ func TestBootstrapCluster(t *testing.T) {
 
 	// Add the initial keys for sql.
 	kvs, tableSplits := GetBootstrapSchema(
-		keys.SystemSQLCodec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(),
+		zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef(),
 	).GetInitialValues()
 	for _, kv := range kvs {
 		expectedKeys = append(expectedKeys, kv.Key)

pkg/sql/sqlbase/metadata.go

@@ -87,7 +87,9 @@ type metadataDescriptor struct {
 }
 
 // MakeMetadataSchema constructs a new MetadataSchema value which constructs
-// the "system" database.
+// the "system" database. Default zone configurations are required to create
+// a MetadataSchema for the system tenant, but do not need to be supplied for
+// any other tenant.
 func MakeMetadataSchema(
 	codec keys.SQLCodec,
 	defaultZoneConfig *zonepb.ZoneConfig,

pkg/sql/sqlbase/system.go

@@ -1535,20 +1535,6 @@ var (
 	}
 )
 
-// Create a kv pair for the zone config for the given key and config value.
-func createZoneConfigKV(
-	keyID int, codec keys.SQLCodec, zoneConfig *zonepb.ZoneConfig,
-) roachpb.KeyValue {
-	value := roachpb.Value{}
-	if err := value.SetProto(zoneConfig); err != nil {
-		panic(fmt.Sprintf("could not marshal ZoneConfig for ID: %d: %s", keyID, err))
-	}
-	return roachpb.KeyValue{
-		Key:   codec.ZoneKey(uint32(keyID)),
-		Value: value,
-	}
-}
-
 // addSystemDescriptorsToSchema populates the supplied MetadataSchema
 // with the system database and table descriptors. The descriptors for
 // these objects exist statically in this file, but a MetadataSchema
@@ -1562,17 +1548,17 @@ func addSystemDescriptorsToSchema(target *MetadataSchema) {
 	target.AddDescriptor(keys.SystemDatabaseID, &NamespaceTable)
 	target.AddDescriptor(keys.SystemDatabaseID, &DescriptorTable)
 	target.AddDescriptor(keys.SystemDatabaseID, &UsersTable)
-	target.AddDescriptor(keys.SystemDatabaseID, &ZonesTable)
+	if target.codec.ForSystemTenant() {
+		target.AddDescriptor(keys.SystemDatabaseID, &ZonesTable)
+	}
 	target.AddDescriptor(keys.SystemDatabaseID, &SettingsTable)
-
-	// Only add the descriptor ID sequence if this is a non-system tenant.
-	// System tenants use the global descIDGenerator key. See #48513.
 	if !target.codec.ForSystemTenant() {
+		// Only add the descriptor ID sequence if this is a non-system tenant.
+		// System tenants use the global descIDGenerator key. See #48513.
 		target.AddDescriptor(keys.SystemDatabaseID, &DescIDSequence)
 	}
-
-	// Only add the tenant table if this is the system tenant.
 	if target.codec.ForSystemTenant() {
+		// Only add the tenant table if this is the system tenant.
 		target.AddDescriptor(keys.SystemDatabaseID, &TenantsTable)
 	}
 
@@ -1606,16 +1592,38 @@ func addSystemDescriptorsToSchema(target *MetadataSchema) {
 	target.AddDescriptor(keys.SystemDatabaseID, &StatementDiagnosticsTable)
 }
 
-// addSystemDatabaseToSchema populates the supplied MetadataSchema with the
-// System database, its tables and zone configurations.
-func addSystemDatabaseToSchema(
+// addSplitIDs adds a split point for each of the PseudoTableIDs to the supplied
+// MetadataSchema.
+func addSplitIDs(target *MetadataSchema) {
+	target.AddSplitIDs(keys.PseudoTableIDs...)
+}
+
+// Create a kv pair for the zone config for the given key and config value.
+func createZoneConfigKV(
+	keyID int, codec keys.SQLCodec, zoneConfig *zonepb.ZoneConfig,
+) roachpb.KeyValue {
+	value := roachpb.Value{}
+	if err := value.SetProto(zoneConfig); err != nil {
+		panic(fmt.Sprintf("could not marshal ZoneConfig for ID: %d: %s", keyID, err))
+	}
+	return roachpb.KeyValue{
+		Key:   codec.ZoneKey(uint32(keyID)),
+		Value: value,
+	}
+}
+
+// addZoneConfigKVsToSchema adds a kv pair for each of the statically defined
+// zone configurations that should be populated in a newly bootstrapped cluster.
+func addZoneConfigKVsToSchema(
 	target *MetadataSchema,
 	defaultZoneConfig *zonepb.ZoneConfig,
 	defaultSystemZoneConfig *zonepb.ZoneConfig,
 ) {
-	addSystemDescriptorsToSchema(target)
-
-	target.AddSplitIDs(keys.PseudoTableIDs...)
+	// If this isn't the system tenant, don't add any zone configuration keys.
+	// Only the system tenant has a zone table.
+	if !target.codec.ForSystemTenant() {
+		return
+	}
 
 	// Adding a new system table? It should be added here to the metadata schema,
 	// and also created as a migration for older cluster. The includedInBootstrap
@@ -1655,6 +1663,18 @@ func addSystemDatabaseToSchema(
 		createZoneConfigKV(keys.ReplicationStatsTableID, target.codec, replicationStatsZoneConf))
 }
 
+// addSystemDatabaseToSchema populates the supplied MetadataSchema with the
+// System database, its tables and zone configurations.
+func addSystemDatabaseToSchema(
+	target *MetadataSchema,
+	defaultZoneConfig *zonepb.ZoneConfig,
+	defaultSystemZoneConfig *zonepb.ZoneConfig,
+) {
+	addSystemDescriptorsToSchema(target)
+	addSplitIDs(target)
+	addZoneConfigKVsToSchema(target, defaultZoneConfig, defaultSystemZoneConfig)
+}
+
 // IsSystemConfigID returns whether this ID is for a system config object.
 func IsSystemConfigID(id ID) bool {
 	return id > 0 && id <= keys.MaxSystemConfigDescID

pkg/sql/tenant.go

@@ -13,7 +13,6 @@ package sql
 import (
 	"context"
 
-	"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/pgwire/pgcode"
@@ -63,8 +62,8 @@ func (p *planner) CreateTenant(ctx context.Context, tenID uint64, tenInfo []byte
 	// Initialize the tenant's keyspace.
 	schema := sqlbase.MakeMetadataSchema(
 		keys.MakeSQLCodec(roachpb.MakeTenantID(tenID)),
-		zonepb.DefaultZoneConfigRef(),
-		zonepb.DefaultSystemZoneConfigRef(),
+		nil, /* defaultZoneConfig */
+		nil, /* defaultZoneConfig */
 	)
 	kvs, splits := schema.GetInitialValues()
 	b := p.Txn().NewBatch()

pkg/sql/tests/system_table_test.go

@@ -34,14 +34,16 @@ import (
 func TestInitialKeys(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	const keysPerDesc = 2
-	const nonDescKeys = 9
 
 	testutils.RunTrueAndFalse(t, "system tenant", func(t *testing.T, systemTenant bool) {
 		var codec keys.SQLCodec
+		var nonDescKeys int
 		if systemTenant {
 			codec = keys.SystemSQLCodec
+			nonDescKeys = 9
 		} else {
 			codec = keys.MakeSQLCodec(roachpb.MakeTenantID(5))
+			nonDescKeys = 2
 		}
 
 		ms := sqlbase.MakeMetadataSchema(codec, zonepb.DefaultZoneConfigRef(), zonepb.DefaultSystemZoneConfigRef())

pkg/sql/tests/testdata/initial_keys

@@ -96,12 +96,11 @@ initial-keys tenant=system
 
 initial-keys tenant=5
 ----
-65 keys:
+56 keys:
  /Tenant/5/Table/3/1/1/2/1
  /Tenant/5/Table/3/1/2/2/1
  /Tenant/5/Table/3/1/3/2/1
  /Tenant/5/Table/3/1/4/2/1
- /Tenant/5/Table/3/1/5/2/1
  /Tenant/5/Table/3/1/6/2/1
  /Tenant/5/Table/3/1/7/2/1
  /Tenant/5/Table/3/1/11/2/1
@@ -125,13 +124,6 @@ initial-keys tenant=5
  /Tenant/5/Table/3/1/34/2/1
  /Tenant/5/Table/3/1/35/2/1
  /Tenant/5/Table/3/1/36/2/1
- /Tenant/5/Table/5/1/0/2/1
- /Tenant/5/Table/5/1/1/2/1
- /Tenant/5/Table/5/1/16/2/1
- /Tenant/5/Table/5/1/17/2/1
- /Tenant/5/Table/5/1/22/2/1
- /Tenant/5/Table/5/1/25/2/1
- /Tenant/5/Table/5/1/27/2/1
  /Tenant/5/Table/7/1/0/0
  /Tenant/5/NamespaceTable/30/1/0/0/"system"/4/1
  /Tenant/5/NamespaceTable/30/1/1/0/"public"/4/1
@@ -161,7 +153,6 @@ initial-keys tenant=5
  /Tenant/5/NamespaceTable/30/1/1/29/"ui"/4/1
  /Tenant/5/NamespaceTable/30/1/1/29/"users"/4/1
  /Tenant/5/NamespaceTable/30/1/1/29/"web_sessions"/4/1
- /Tenant/5/NamespaceTable/30/1/1/29/"zones"/4/1
 26 splits:
  /Tenant/5/Table/11
  /Tenant/5/Table/12
@@ -192,12 +183,11 @@ initial-keys tenant=5
 
 initial-keys tenant=999
 ----
-65 keys:
+56 keys:
  /Tenant/999/Table/3/1/1/2/1
  /Tenant/999/Table/3/1/2/2/1
  /Tenant/999/Table/3/1/3/2/1
  /Tenant/999/Table/3/1/4/2/1
- /Tenant/999/Table/3/1/5/2/1
  /Tenant/999/Table/3/1/6/2/1
  /Tenant/999/Table/3/1/7/2/1
  /Tenant/999/Table/3/1/11/2/1
@@ -221,13 +211,6 @@ initial-keys tenant=999
  /Tenant/999/Table/3/1/34/2/1
  /Tenant/999/Table/3/1/35/2/1
  /Tenant/999/Table/3/1/36/2/1
- /Tenant/999/Table/5/1/0/2/1
- /Tenant/999/Table/5/1/1/2/1
- /Tenant/999/Table/5/1/16/2/1
- /Tenant/999/Table/5/1/17/2/1
- /Tenant/999/Table/5/1/22/2/1
- /Tenant/999/Table/5/1/25/2/1
- /Tenant/999/Table/5/1/27/2/1
  /Tenant/999/Table/7/1/0/0
  /Tenant/999/NamespaceTable/30/1/0/0/"system"/4/1
  /Tenant/999/NamespaceTable/30/1/1/0/"public"/4/1
@@ -257,7 +240,6 @@ initial-keys tenant=999
  /Tenant/999/NamespaceTable/30/1/1/29/"ui"/4/1
  /Tenant/999/NamespaceTable/30/1/1/29/"users"/4/1
  /Tenant/999/NamespaceTable/30/1/1/29/"web_sessions"/4/1
- /Tenant/999/NamespaceTable/30/1/1/29/"zones"/4/1
 26 splits:
  /Tenant/999/Table/11
  /Tenant/999/Table/12