Merge #39638

39638: config: propagate zone config info for non-gossiped system tables r=nvanbenschoten a=ajwerner

Before this PR we would allow setting zone configurations on system tables but
they would not propagate because the GetZoneConfig function would return the
zone config for the entire system database. This leaves us in a still weird
situation where some system tables will ignore their zone configs but not
others.

Fixes #39605.

Release note (bug fix): Propagate zone configuration to non-gossiped system
tables.

Co-authored-by: Andrew Werner <[email protected]>

pkg/cmd/roachtest/gossip.go

@@ -350,6 +350,7 @@ func runGossipRestartNodeOne(ctx context.Context, t *test, c *cluster) {
 	run(`ALTER DATABASE system %[1]s CONFIGURE ZONE %[2]s 'constraints: {"-rack=0"}'`)
 	run(`ALTER RANGE meta %[1]s CONFIGURE ZONE %[2]s 'constraints: {"-rack=0"}'`)
 	run(`ALTER RANGE liveness %[1]s CONFIGURE ZONE %[2]s 'constraints: {"-rack=0"}'`)
+	run(`ALTER TABLE system.jobs %[1]s CONFIGURE ZONE %[2]s 'constraints: {"-rack=0"}'`)
 
 	var lastReplCount int
 	if err := retry.ForDuration(2*time.Minute, func() error {

pkg/config/system.go

@@ -255,11 +255,14 @@ func (s *SystemConfig) GetZoneConfigForKey(key roachpb.RKey) (*ZoneConfig, error
 	if !ok {
 		// Not in the structured data namespace.
 		objectID = keys.RootNamespaceID
-	} else if objectID <= keys.MaxReservedDescID {
-		// For now, you can only set a zone config on the system database as a whole,
-		// not on any of its constituent tables. This is largely because all the
+	} else if objectID <= keys.MaxSystemConfigDescID || isPseudoTableID(objectID) {
+		// For now, you cannot set the zone config on gossiped tables. The only
+		// way to set a zone config on these tables is to modify config for the
+		// system database as a whole. This is largely because all the
 		// "system config" tables are colocated in the same range by default and
 		// thus couldn't be managed separately.
+		// Furthermore pseudo-table ids should be considered to be a part of the
+		// system database as they aren't real tables.
 		objectID = keys.SystemDatabaseID
 	}
 
@@ -279,6 +282,16 @@ func (s *SystemConfig) GetZoneConfigForKey(key roachpb.RKey) (*ZoneConfig, error
 	return s.getZoneConfigForKey(objectID, keySuffix)
 }
 
+// isPseudoTableID returns true if id is in keys.PseudoTableIDs.
+func isPseudoTableID(id uint32) bool {
+	for _, pseudoTableID := range keys.PseudoTableIDs {
+		if id == pseudoTableID {
+			return true
+		}
+	}
+	return false
+}
+
 // GetZoneConfigForObject returns the combined zone config for the given object
 // identifier.
 // NOTE: any subzones from the zone placeholder will be automatically merged

pkg/config/system_test.go

@@ -402,11 +402,23 @@ func TestGetZoneConfigForKey(t *testing.T) {
 		{roachpb.RKey(keys.TimeseriesPrefix.PrefixEnd()), keys.SystemRangesID},
 		{roachpb.RKey(keys.TableDataMin), keys.SystemDatabaseID},
 		{roachpb.RKey(keys.SystemConfigSplitKey), keys.SystemDatabaseID},
+
+		// Gossiped system tables should refer to the SystemDatabaseID.
 		{tkey(keys.NamespaceTableID), keys.SystemDatabaseID},
 		{tkey(keys.ZonesTableID), keys.SystemDatabaseID},
-		{tkey(keys.LeaseTableID), keys.SystemDatabaseID},
-		{tkey(keys.JobsTableID), keys.SystemDatabaseID},
-		{tkey(keys.LocationsTableID), keys.SystemDatabaseID},
+
+		// Non-gossiped system tables should refer to themselves.
+		{tkey(keys.LeaseTableID), keys.LeaseTableID},
+		{tkey(keys.JobsTableID), keys.JobsTableID},
+		{tkey(keys.LocationsTableID), keys.LocationsTableID},
+
+		// Pseudo-tables should refer to the SystemDatabaseID.
+		{tkey(keys.MetaRangesID), keys.SystemDatabaseID},
+		{tkey(keys.SystemRangesID), keys.SystemDatabaseID},
+		{tkey(keys.TimeseriesRangesID), keys.SystemDatabaseID},
+		{tkey(keys.LivenessRangesID), keys.SystemDatabaseID},
+
+		// User tables should refer to themselves.
 		{tkey(keys.MinUserDescID), keys.MinUserDescID},
 		{tkey(keys.MinUserDescID + 22), keys.MinUserDescID + 22},
 		{roachpb.RKeyMax, keys.RootNamespaceID},

pkg/storage/client_replica_test.go

@@ -1783,6 +1783,13 @@ func TestSystemZoneConfigs(t *testing.T) {
 	testutils.SucceedsSoon(t, waitForReplicas)
 	log.Info(ctx, "TestSystemZoneConfig: down-replication of timeseries ranges succeeded")
 
+	// Up-replicate the system.jobs table to demonstrate that it is configured
+	// independently from the system database.
+	sqlutils.SetZoneConfig(t, sqlDB, "TABLE system.jobs", "num_replicas: 7")
+	expectedReplicas += 2
+	testutils.SucceedsSoon(t, waitForReplicas)
+	log.Info(ctx, "TestSystemZoneConfig: up-replication of jobs table succeeded")
+
 	// Finally, verify the system ranges. Note that in a new cluster there are
 	// two system ranges, which we have to take into account here.
 	sqlutils.SetZoneConfig(t, sqlDB, "RANGE system", "num_replicas: 7")