Merge #76606 #76801 #76900

76606: spanconfigsqltranslator: emit all SystemTarget span configs when required r=arulajmani a=adityamaru

This change teaches the SQLTranslator to emit SpanConfigurations
corresponding to `spanconfig.SystemTargets`. Today, these SpanConfigurations
only contain ProtectionPolicies, corresponding to protected timestamp
records that may be written by the host tenant to protect its cluster,
a secondary tenant to protect its cluster, or the host tenant to protect
a secondary tenant.

The SystemTarget span configurations will be applied to a SystemSpanConfig
store that will be introduced in a follow up PR.

Informs: #73727

Release note: None

76801: sql: fix cached gists panic'ing after schema changes r=mgartner a=cucaroach

Previously we didn't guard against out of bounds column ids in gists,
if a gist is created on a table and then that table drops columns we
would hit a runtime index out of bounds panic. By design gist decoding
is best effort and should silently ignore these columns when this
happens. The columns are used for equality conditions, ie:

             └── • lookup join
                 │ table: broker@broker_pkey
                 │ equality: (tr_s_symb) = (b_id)

So they are a nice to have. Doing anything more sophisticated would
dramatically increase the gist size so isn't worth it.

Fixes: #76800

Release note: None


76900: mon: when creating inherited BytesMonitor don't copy metrics r=yuzefovich a=aliher1911

Previously monitor inherited parameters like name, resource, limits etc
from parent and also included metrics.
This is not good as metrics were updated twice when borrowing resources,
one time by child monitor and again by parent monitor which lead to
misreporting.
This patch removes metrics from inheritance. It is safe to do as metrics
could be safely set to nil. It could lead to some memory that is reserved
by child pool reported as allocated, but it is better than doing x2.

Release note: None

Fixes #76898

Co-authored-by: Aditya Maru <[email protected]>
Co-authored-by: Tommy Reilly <[email protected]>
Co-authored-by: Oleg Afanasyev <[email protected]>

pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/datadriven_test.go

@@ -119,6 +119,8 @@ func TestDataDriven(t *testing.T) {
 		}
 
 		datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string {
+			var generateSystemSpanConfigs bool
+			var descIDs []descpb.ID
 			switch d.Cmd {
 			case "exec-sql":
 				tenant.Exec(d.Input)
@@ -130,45 +132,54 @@ func TestDataDriven(t *testing.T) {
 				return output
 
 			case "translate":
-				// Parse the args to get the object ID we're looking to
+				// Parse the args to get the descriptor ID we're looking to
 				// translate.
-				var objID descpb.ID
 				switch {
 				case d.HasArg("named-zone"):
 					var zone string
 					d.ScanArgs(t, "named-zone", &zone)
 					namedZoneID, found := zonepb.NamedZones[zonepb.NamedZone(zone)]
 					require.Truef(t, found, "unknown named zone: %s", zone)
-					objID = descpb.ID(namedZoneID)
+					descIDs = []descpb.ID{descpb.ID(namedZoneID)}
 				case d.HasArg("id"):
 					var scanID int
 					d.ScanArgs(t, "id", &scanID)
-					objID = descpb.ID(scanID)
+					descIDs = []descpb.ID{descpb.ID(scanID)}
 				case d.HasArg("database"):
 					var dbName string
 					d.ScanArgs(t, "database", &dbName)
 					if d.HasArg("table") {
 						var tbName string
 						d.ScanArgs(t, "table", &tbName)
-						objID = tenant.LookupTableByName(ctx, dbName, tbName).GetID()
+						descIDs = []descpb.ID{tenant.LookupTableByName(ctx, dbName, tbName).GetID()}
 					} else {
-						objID = tenant.LookupDatabaseByName(ctx, dbName).GetID()
+						descIDs = []descpb.ID{tenant.LookupDatabaseByName(ctx, dbName).GetID()}
 					}
+				case d.HasArg("system-span-configurations"):
+					generateSystemSpanConfigs = true
 				default:
 					d.Fatalf(t, "insufficient/improper args (%v) provided to translate", d.CmdArgs)
 				}
 
 				sqlTranslator := tenant.SpanConfigSQLTranslator().(spanconfig.SQLTranslator)
-				records, _, err := sqlTranslator.Translate(ctx, descpb.IDs{objID})
+				records, _, err := sqlTranslator.Translate(ctx, descIDs, generateSystemSpanConfigs)
 				require.NoError(t, err)
 				sort.Slice(records, func(i, j int) bool {
 					return records[i].Target.Less(records[j].Target)
 				})
 
 				var output strings.Builder
 				for _, record := range records {
-					output.WriteString(fmt.Sprintf("%-42s %s\n", record.Target.GetSpan(),
-						spanconfigtestutils.PrintSpanConfigDiffedAgainstDefaults(record.Config)))
+					switch {
+					case record.Target.IsSpanTarget():
+						output.WriteString(fmt.Sprintf("%-42s %s\n", record.Target.GetSpan(),
+							spanconfigtestutils.PrintSpanConfigDiffedAgainstDefaults(record.Config)))
+					case record.Target.IsSystemTarget():
+						output.WriteString(fmt.Sprintf("%-42s %s\n", record.Target.GetSystemTarget(),
+							spanconfigtestutils.PrintSystemSpanConfigDiffedAgainstDefault(record.Config)))
+					default:
+						panic("unsupported target type")
+					}
 				}
 				return output.String()
 

pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/protectedts

@@ -1,7 +1,5 @@
 # Create a database with some tables and write protected timestamps on the
 # tables and database. Check that span configurations are as we expect.
-# TODO(adityamaru): Add tests with Cluster, Tenant target once the translator
-# has been taught to generate SystemSpanConfigs.
 
 exec-sql
 CREATE DATABASE db;
@@ -41,10 +39,44 @@ translate database=db
 /Table/10{6-7}                             num_replicas=7 num_voters=5 pts=[1 2]
 /Table/10{7-8}                             num_replicas=7 pts=[2]
 
+# Write a protected timestamp on the cluster.
+protect record-id=3 ts=3
+cluster
+----
+
+# Write a protected timestamp on some tenants.
+protect record-id=4 ts=4
+tenants 111,112,113
+----
+
+# Write another protected timestamp on a subset of the tenants.
+protect record-id=5 ts=3
+tenants 111,112
+----
+
+translate system-span-configurations
+----
+{entire-keyspace}                          pts=[3]
+{source=1,target=111}                      pts=[3 4]
+{source=1,target=112}                      pts=[3 4]
+{source=1,target=113}                      pts=[4]
+
+translate database=db
+----
+/Table/10{6-7}                             num_replicas=7 num_voters=5 pts=[1 2]
+/Table/10{7-8}                             num_replicas=7 pts=[2]
+
 # Release the protected timestamp on table t1
 release record-id=1
 ----
 
+translate system-span-configurations
+----
+{entire-keyspace}                          pts=[3]
+{source=1,target=111}                      pts=[3 4]
+{source=1,target=112}                      pts=[3 4]
+{source=1,target=113}                      pts=[4]
+
 translate database=db
 ----
 /Table/10{6-7}                             num_replicas=7 num_voters=5 pts=[2]
@@ -54,24 +86,44 @@ translate database=db
 release record-id=2
 ----
 
+translate system-span-configurations
+----
+{entire-keyspace}                          pts=[3]
+{source=1,target=111}                      pts=[3 4]
+{source=1,target=112}                      pts=[3 4]
+{source=1,target=113}                      pts=[4]
+
 translate database=db
 ----
 /Table/10{6-7}                             num_replicas=7 num_voters=5
 /Table/10{7-8}                             num_replicas=7
 
+
+# Release the protected timestamp on the cluster and tenants.
+release record-id=3
+----
+
+release record-id=4
+----
+
 # Create an index on t1 to ensure that subzones also see protected timestamps.
 exec-sql
 CREATE INDEX idx ON db.t1(id);
 ALTER INDEX db.t1@idx CONFIGURE ZONE USING gc.ttlseconds = 1;
 ----
 
-protect record-id=3 ts=3
+protect record-id=6 ts=6
 descs 106
 ----
 
+translate system-span-configurations
+----
+{source=1,target=111}                      pts=[3]
+{source=1,target=112}                      pts=[3]
+
 translate database=db
 ----
-/Table/106{-/2}                            num_replicas=7 num_voters=5 pts=[3]
-/Table/106/{2-3}                           ttl_seconds=1 num_replicas=7 num_voters=5 pts=[3]
-/Table/10{6/3-7}                           num_replicas=7 num_voters=5 pts=[3]
+/Table/106{-/2}                            num_replicas=7 num_voters=5 pts=[6]
+/Table/106/{2-3}                           ttl_seconds=1 num_replicas=7 num_voters=5 pts=[6]
+/Table/10{6/3-7}                           num_replicas=7 num_voters=5 pts=[6]
 /Table/10{7-8}                             num_replicas=7

pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/tenant/protectedts

@@ -8,9 +8,9 @@ CREATE TABLE db.t2();
 ----
 
 # Schema object IDs
-# db: 54
-# t1: 56
-# t2: 57
+# db: 104
+# t1: 106
+# t2: 107
 
 # Alter zone config fields on the database and one of the tables to ensure
 # things are cascading.
@@ -21,37 +21,65 @@ ALTER TABLE db.t1 CONFIGURE ZONE USING num_voters=5;
 
 # Write a protected timestamp on t1.
 protect record-id=1 ts=1
-descs 56
+descs 106
 ----
 
 translate database=db
 ----
-/Tenant/10/Table/10{6-7}                   num_replicas=7 num_voters=5
+/Tenant/10/Table/10{6-7}                   num_replicas=7 num_voters=5 pts=[1]
 /Tenant/10/Table/10{7-8}                   num_replicas=7
 
 # Write a protected timestamp on db, so we should see it on both t1 and t2.
 protect record-id=2 ts=2
-descs 54
+descs 104
 ----
 
 translate database=db
 ----
-/Tenant/10/Table/10{6-7}                   num_replicas=7 num_voters=5
-/Tenant/10/Table/10{7-8}                   num_replicas=7
+/Tenant/10/Table/10{6-7}                   num_replicas=7 num_voters=5 pts=[1 2]
+/Tenant/10/Table/10{7-8}                   num_replicas=7 pts=[2]
+
+
+# Write a protected timestamp on the tenant cluster.
+protect record-id=3 ts=3
+cluster
+----
+
+# Write another protected timestamp on the tenant cluster.
+protect record-id=4 ts=3
+cluster
+----
+
+translate system-span-configurations
+----
+{source=10,target=10}                      pts=[3 3]
+
+translate database=db
+----
+/Tenant/10/Table/10{6-7}                   num_replicas=7 num_voters=5 pts=[1 2]
+/Tenant/10/Table/10{7-8}                   num_replicas=7 pts=[2]
 
 # Release the protected timestamp on table t1
 release record-id=1
 ----
 
+translate system-span-configurations
+----
+{source=10,target=10}                      pts=[3 3]
+
 translate database=db
 ----
-/Tenant/10/Table/10{6-7}                   num_replicas=7 num_voters=5
-/Tenant/10/Table/10{7-8}                   num_replicas=7
+/Tenant/10/Table/10{6-7}                   num_replicas=7 num_voters=5 pts=[2]
+/Tenant/10/Table/10{7-8}                   num_replicas=7 pts=[2]
 
 # Release the protected timestamp on database db
 release record-id=2
 ----
 
+translate system-span-configurations
+----
+{source=10,target=10}                      pts=[3 3]
+
 translate database=db
 ----
 /Tenant/10/Table/10{6-7}                   num_replicas=7 num_voters=5
@@ -63,13 +91,21 @@ CREATE INDEX idx ON db.t1(id);
 ALTER INDEX db.t1@idx CONFIGURE ZONE USING gc.ttlseconds = 1;
 ----
 
-protect record-id=3 ts=3
-descs 56
+protect record-id=5 ts=5
+descs 106
+----
+
+# Release the protected timestamp on the tenant cluster.
+release record-id=3
+----
+
+translate system-span-configurations
 ----
+{source=10,target=10}                      pts=[3]
 
 translate database=db
 ----
-/Tenant/10/Table/106{-/2}                  num_replicas=7 num_voters=5
-/Tenant/10/Table/106/{2-3}                 ttl_seconds=1 num_replicas=7 num_voters=5
-/Tenant/10/Table/10{6/3-7}                 num_replicas=7 num_voters=5
+/Tenant/10/Table/106{-/2}                  num_replicas=7 num_voters=5 pts=[5]
+/Tenant/10/Table/106/{2-3}                 ttl_seconds=1 num_replicas=7 num_voters=5 pts=[5]
+/Tenant/10/Table/10{6/3-7}                 num_replicas=7 num_voters=5 pts=[5]
 /Tenant/10/Table/10{7-8}                   num_replicas=7

pkg/roachpb/span_config.go

@@ -108,6 +108,12 @@ func TestingDefaultSpanConfig() SpanConfig {
 	}
 }
 
+// TestingDefaultSystemSpanConfiguration exports the default span config that
+// applies to spanconfig.SystemTargets for testing purposes.
+func TestingDefaultSystemSpanConfiguration() SpanConfig {
+	return SpanConfig{}
+}
+
 // TestingSystemSpanConfig exports the system span config for testing purposes.
 func TestingSystemSpanConfig() SpanConfig {
 	config := TestingDefaultSpanConfig()

pkg/spanconfig/spanconfig.go

@@ -98,8 +98,11 @@ type KVSubscriber interface {
 type SQLTranslator interface {
 	// Translate generates the span configuration state given a list of
 	// {descriptor, named zone} IDs. Entries are unique, and are omitted for IDs
-	// that don't exist. The timestamp at which the translation is valid is also
-	// returned.
+	// that don't exist.
+	// Additionally, if `generateSystemSpanConfigurations` is set to true,
+	// Translate will generate all the span configurations that apply to
+	// `spanconfig.SystemTargets`. The timestamp at which the translation is valid
+	// is also returned.
 	//
 	// For every ID we first descend the zone configuration hierarchy with the
 	// ID as the root to accumulate IDs of all leaf objects. Leaf objects are
@@ -109,7 +112,8 @@ type SQLTranslator interface {
 	// for each one of these accumulated IDs, we generate <span, config> tuples
 	// by following up the inheritance chain to fully hydrate the span
 	// configuration. Translate also accounts for and negotiates subzone spans.
-	Translate(ctx context.Context, ids descpb.IDs) ([]Record, hlc.Timestamp, error)
+	Translate(ctx context.Context, ids descpb.IDs,
+		generateSystemSpanConfigurations bool) ([]Record, hlc.Timestamp, error)
 }
 
 // FullTranslate translates the entire SQL zone configuration state to the span
@@ -119,7 +123,8 @@ func FullTranslate(ctx context.Context, s SQLTranslator) ([]Record, hlc.Timestam
 	// As RANGE DEFAULT is the root of all zone configurations (including other
 	// named zones for the system tenant), we can construct the entire span
 	// configuration state by starting from RANGE DEFAULT.
-	return s.Translate(ctx, descpb.IDs{keys.RootNamespaceID})
+	return s.Translate(ctx, descpb.IDs{keys.RootNamespaceID},
+		true /* generateSystemSpanConfigurations */)
 }
 
 // SQLWatcherHandler is the signature of a handler that can be passed into

pkg/spanconfig/spanconfigreconciler/reconciler.go

@@ -369,15 +369,19 @@ func (r *incrementalReconciler) reconcile(
 				return callback(checkpoint) // nothing to do; propagate the checkpoint
 			}
 
-			// Process the DescriptorUpdates and identify all descriptor IDs that
-			// require translation.
+			// Process the SQLUpdates and identify all descriptor IDs that require
+			// translation. If the SQLUpdates includes ProtectedTimestampUpdates then
+			// instruct the translator to generate all records that apply to
+			// spanconfig.SystemTargets as well.
+			//
+			// TODO(adityamaru): Conditionally set the bool to true to generate system
+			// span configurations.
+			var generateSystemSpanConfigurations bool
 			var allIDs descpb.IDs
 			for _, update := range sqlUpdates {
 				if update.IsDescriptorUpdate() {
 					allIDs = append(allIDs, update.GetDescriptorUpdate().ID)
 				}
-				// TODO(adityamaru): Set the bool that tells the translator to emit
-				// SystemSpanConfigs.
 			}
 
 			// TODO(irfansharif): Would it be easier to just have the translator
@@ -390,7 +394,7 @@ func (r *incrementalReconciler) reconcile(
 				return err
 			}
 
-			entries, _, err := r.sqlTranslator.Translate(ctx, allIDs)
+			entries, _, err := r.sqlTranslator.Translate(ctx, allIDs, generateSystemSpanConfigurations)
 			if err != nil {
 				return err
 			}