Merge #76219

76219: spanconfig: modify existing RPCs to work with system span configurations r=arulajmani a=arulajmani See individual commits for details. Co-authored-by: arulajmani <[email protected]>
cockroachdb · Feb 15, 2022 · 4f4f2ac · 4f4f2ac
2 parents 95943aa + c7b7c26
commit 4f4f2ac
Show file tree

Hide file tree

Showing 44 changed files with 1,036 additions and 267 deletions.
diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel
@@ -201,6 +201,7 @@ ALL_TESTS = [
     "//pkg/spanconfig/spanconfigsqlwatcher:spanconfigsqlwatcher_test",
     "//pkg/spanconfig/spanconfigstore:spanconfigstore_test",
     "//pkg/spanconfig/spanconfigtestutils:spanconfigtestutils_test",
+    "//pkg/spanconfig:spanconfig_test",
     "//pkg/sql/catalog/catalogkeys:catalogkeys_test",
     "//pkg/sql/catalog/catformat:catformat_test",
     "//pkg/sql/catalog/catpb:catpb_test",

diff --git a/pkg/ccl/kvccl/kvtenantccl/connector.go b/pkg/ccl/kvccl/kvtenantccl/connector.go
@@ -461,18 +461,17 @@ func (c *Connector) GetSpanConfigRecords(
 	ctx context.Context, targets []spanconfig.Target,
 ) (records []spanconfig.Record, _ error) {
 	if err := c.withClient(ctx, func(ctx context.Context, c *client) error {
-		spans := make([]roachpb.Span, 0, len(targets))
-		for _, target := range targets {
-			spans = append(spans, *target.GetSpan())
-		}
 		resp, err := c.GetSpanConfigs(ctx, &roachpb.GetSpanConfigsRequest{
-			Spans: spans,
+			Targets: spanconfig.TargetsToProtos(targets),
 		})
 		if err != nil {
 			return err
 		}
 
-		records = spanconfig.EntriesToRecords(resp.SpanConfigEntries)
+		records, err = spanconfig.EntriesToRecords(resp.SpanConfigEntries)
+		if err != nil {
+			return err
+		}
 		return nil
 	}); err != nil {
 		return nil, err
@@ -485,18 +484,10 @@ func (c *Connector) GetSpanConfigRecords(
 func (c *Connector) UpdateSpanConfigRecords(
 	ctx context.Context, toDelete []spanconfig.Target, toUpsert []spanconfig.Record,
 ) error {
-	spansToDelete := make([]roachpb.Span, 0, len(toDelete))
-	for _, toDel := range toDelete {
-		spansToDelete = append(spansToDelete, roachpb.Span(toDel))
-	}
-
-	entriesToUpsert := spanconfig.RecordsToSpanConfigEntries(toUpsert)
-
 	return c.withClient(ctx, func(ctx context.Context, c *client) error {
-
 		_, err := c.UpdateSpanConfigs(ctx, &roachpb.UpdateSpanConfigsRequest{
-			ToDelete: spansToDelete,
-			ToUpsert: entriesToUpsert,
+			ToDelete: spanconfig.TargetsToProtos(toDelete),
+			ToUpsert: spanconfig.RecordsToEntries(toUpsert),
 		})
 		return err
 	})

diff --git a/pkg/ccl/migrationccl/migrationsccl/seed_tenant_span_configs_external_test.go b/pkg/ccl/migrationccl/migrationsccl/seed_tenant_span_configs_external_test.go
@@ -71,11 +71,11 @@ func TestPreSeedSpanConfigsWrittenWhenActive(t *testing.T) {
 
 	{
 		records, err := scKVAccessor.GetSpanConfigRecords(ctx, []spanconfig.Target{
-			spanconfig.MakeSpanTarget(tenantSpan),
+			spanconfig.MakeTargetFromSpan(tenantSpan),
 		})
 		require.NoError(t, err)
 		require.Len(t, records, 1)
-		require.Equal(t, *records[0].Target.GetSpan(), tenantSeedSpan)
+		require.Equal(t, records[0].Target.GetSpan(), tenantSeedSpan)
 	}
 }
 
@@ -106,7 +106,7 @@ func TestSeedTenantSpanConfigs(t *testing.T) {
 
 	tenantID := roachpb.MakeTenantID(10)
 	tenantPrefix := keys.MakeTenantPrefix(tenantID)
-	tenantTarget := spanconfig.MakeSpanTarget(
+	tenantTarget := spanconfig.MakeTargetFromSpan(
 		roachpb.Span{Key: tenantPrefix, EndKey: tenantPrefix.PrefixEnd()},
 	)
 	tenantSeedSpan := roachpb.Span{Key: tenantPrefix, EndKey: tenantPrefix.Next()}
@@ -144,7 +144,7 @@ func TestSeedTenantSpanConfigs(t *testing.T) {
 		})
 		require.NoError(t, err)
 		require.Len(t, records, 1)
-		require.Equal(t, *records[0].Target.GetSpan(), tenantSeedSpan)
+		require.Equal(t, records[0].Target.GetSpan(), tenantSeedSpan)
 	}
 }
 
@@ -175,7 +175,7 @@ func TestSeedTenantSpanConfigsWithExistingEntry(t *testing.T) {
 
 	tenantID := roachpb.MakeTenantID(10)
 	tenantPrefix := keys.MakeTenantPrefix(tenantID)
-	tenantTarget := spanconfig.MakeSpanTarget(
+	tenantTarget := spanconfig.MakeTargetFromSpan(
 		roachpb.Span{Key: tenantPrefix, EndKey: tenantPrefix.PrefixEnd()},
 	)
 	tenantSeedSpan := roachpb.Span{Key: tenantPrefix, EndKey: tenantPrefix.Next()}
@@ -200,7 +200,7 @@ func TestSeedTenantSpanConfigsWithExistingEntry(t *testing.T) {
 		})
 		require.NoError(t, err)
 		require.Len(t, records, 1)
-		require.Equal(t, *records[0].Target.GetSpan(), tenantSeedSpan)
+		require.Equal(t, records[0].Target.GetSpan(), tenantSeedSpan)
 	}
 
 	// Ensure the cluster version bump goes through successfully.
@@ -215,6 +215,6 @@ func TestSeedTenantSpanConfigsWithExistingEntry(t *testing.T) {
 		})
 		require.NoError(t, err)
 		require.Len(t, records, 1)
-		require.Equal(t, *records[0].Target.GetSpan(), tenantSeedSpan)
+		require.Equal(t, records[0].Target.GetSpan(), tenantSeedSpan)
 	}
 }
diff --git a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/datadriven_test.go b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/datadriven_test.go
@@ -192,7 +192,7 @@ func TestDataDriven(t *testing.T) {
 					return nil
 				})
 				records, err := kvAccessor.GetSpanConfigRecords(
-					ctx, []spanconfig.Target{spanconfig.MakeSpanTarget(keys.EverythingSpan)},
+					ctx, []spanconfig.Target{spanconfig.MakeTargetFromSpan(keys.EverythingSpan)},
 				)
 				require.NoError(t, err)
 				sort.Slice(records, func(i, j int) bool {

diff --git a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/basic b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/basic
@@ -11,7 +11,7 @@ upsert /{Min-System/NodeLiveness}          ttl_seconds=3600 num_replicas=5
 upsert /System/NodeLiveness{-Max}          ttl_seconds=600 num_replicas=5
 upsert /System/{NodeLivenessMax-tsd}       range system
 upsert /System{/tsd-tse}                   range default
-upsert /System{tse-/Max}                   range system
+upsert /System{tse-/SystemSpanConfigKeys}  range system
 upsert /Table/{SystemConfigSpan/Start-4}   database system (host)
 upsert /Table/{4-5}                        database system (host)
 upsert /Table/{5-6}                        database system (host)

diff --git a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/indexes b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/indexes
@@ -35,8 +35,8 @@ ALTER INDEX db.t@idx CONFIGURE ZONE USING num_voters = 5;
 # - any future indexes that may be added to this table (table's config)
 mutations
 ----
-delete /Table/10{6-7}
 upsert /Table/106{-/2}                     num_replicas=7
+delete /Table/10{6-7}
 upsert /Table/106/{2-3}                    num_replicas=7 num_voters=5
 upsert /Table/10{6/3-7}                    num_replicas=7
 

diff --git a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/named_zones b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/named_zones
@@ -13,7 +13,7 @@ state limit=5
 /System/NodeLiveness{-Max}                 ttl_seconds=600 num_replicas=5
 /System/{NodeLivenessMax-tsd}              range system
 /System{/tsd-tse}                          range default
-/System{tse-/Max}                          range system
+/System{tse-/SystemSpanConfigKeys}         range system
 ...
 
 # Adding an explicit zone configuration for the timeseries range should work
@@ -48,16 +48,16 @@ mutations
 ----
 delete /System/{NodeLivenessMax-tsd}
 upsert /System/{NodeLivenessMax-tsd}       range default
-delete /System{tse-/Max}
-upsert /System{tse-/Max}                   range default
+delete /System{tse-/SystemSpanConfigKeys}
+upsert /System{tse-/SystemSpanConfigKeys}  range default
 
 state limit=5
 ----
 /{Min-System/NodeLiveness}                 ttl_seconds=3600 num_replicas=5
 /System/NodeLiveness{-Max}                 ttl_seconds=600 num_replicas=7
 /System/{NodeLivenessMax-tsd}              range default
 /System{/tsd-tse}                          ttl_seconds=42
-/System{tse-/Max}                          range default
+/System{tse-/SystemSpanConfigKeys}         range default
 ...
 
 # Ensure that discarding other named zones behave as expected (reparenting them
@@ -80,7 +80,7 @@ state limit=5
 /System/NodeLiveness{-Max}                 ttl_seconds=600 num_replicas=7
 /System/{NodeLivenessMax-tsd}              range default
 /System{/tsd-tse}                          range default
-/System{tse-/Max}                          range default
+/System{tse-/SystemSpanConfigKeys}         range default
 ...
 
 
@@ -106,8 +106,8 @@ delete /System/{NodeLivenessMax-tsd}
 upsert /System/{NodeLivenessMax-tsd}       ttl_seconds=50
 delete /System{/tsd-tse}
 upsert /System{/tsd-tse}                   ttl_seconds=50
-delete /System{tse-/Max}
-upsert /System{tse-/Max}                   ttl_seconds=50
+delete /System{tse-/SystemSpanConfigKeys}
+upsert /System{tse-/SystemSpanConfigKeys}  ttl_seconds=50
 delete /Table/10{6-7}
 upsert /Table/10{6-7}                      ttl_seconds=50
 
@@ -117,7 +117,7 @@ state limit=5
 /System/NodeLiveness{-Max}                 ttl_seconds=600 num_replicas=7
 /System/{NodeLivenessMax-tsd}              ttl_seconds=50
 /System{/tsd-tse}                          ttl_seconds=50
-/System{tse-/Max}                          ttl_seconds=50
+/System{tse-/SystemSpanConfigKeys}         ttl_seconds=50
 ...
 
 state offset=46
@@ -152,14 +152,14 @@ mutations
 ----
 delete /System/{NodeLivenessMax-tsd}
 upsert /System/{NodeLivenessMax-tsd}       ttl_seconds=100
-delete /System{tse-/Max}
-upsert /System{tse-/Max}                   ttl_seconds=100
+delete /System{tse-/SystemSpanConfigKeys}
+upsert /System{tse-/SystemSpanConfigKeys}  ttl_seconds=100
 
 state limit=5
 ----
 /{Min-System/NodeLiveness}                 ttl_seconds=50
 /System/NodeLiveness{-Max}                 ttl_seconds=600 num_replicas=7
 /System/{NodeLivenessMax-tsd}              ttl_seconds=100
 /System{/tsd-tse}                          ttl_seconds=50
-/System{tse-/Max}                          ttl_seconds=100
+/System{tse-/SystemSpanConfigKeys}         ttl_seconds=100
 ...
diff --git a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/partitions b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/partitions
@@ -63,8 +63,8 @@ ALTER PARTITION one_two OF TABLE db.t CONFIGURE ZONE USING global_reads = true
 
 mutations
 ----
-delete /Table/10{6-7}
 upsert /Table/106{-/1/1}                   num_replicas=7 num_voters=5
+delete /Table/10{6-7}
 upsert /Table/106/1/{1-2}                  global_reads=true num_replicas=7 num_voters=5
 upsert /Table/106/1/{2-3}                  global_reads=true num_replicas=7 num_voters=5
 upsert /Table/10{6/1/3-7}                  num_replicas=7 num_voters=5
@@ -86,8 +86,8 @@ ALTER PARTITION three_four OF TABLE db.t CONFIGURE ZONE USING gc.ttlseconds = 5
 
 mutations
 ----
-delete /Table/10{6/1/3-7}
 upsert /Table/106/1/{3-4}                  ttl_seconds=5 num_replicas=7 num_voters=5
+delete /Table/10{6/1/3-7}
 upsert /Table/106/1/{4-5}                  ttl_seconds=5 num_replicas=7 num_voters=5
 upsert /Table/10{6/1/5-7}                  num_replicas=7 num_voters=5
 
@@ -120,11 +120,11 @@ ALTER PARTITION default OF TABLE db.t CONFIGURE ZONE USING num_voters = 6
 
 mutations
 ----
-delete /Table/106{-/1/1}
 upsert /Table/106{-/1}                     num_replicas=7 num_voters=5
+delete /Table/106{-/1/1}
 upsert /Table/106/1{-/1}                   num_replicas=7 num_voters=6
-delete /Table/10{6/1/5-7}
 upsert /Table/106/{1/5-2}                  num_replicas=7 num_voters=6
+delete /Table/10{6/1/5-7}
 upsert /Table/10{6/2-7}                    num_replicas=7 num_voters=5
 
 state offset=47

diff --git a/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/datadriven_test.go b/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/datadriven_test.go
@@ -167,7 +167,7 @@ func TestDataDriven(t *testing.T) {
 
 				var output strings.Builder
 				for _, record := range records {
-					output.WriteString(fmt.Sprintf("%-42s %s\n", *record.Target.GetSpan(),
+					output.WriteString(fmt.Sprintf("%-42s %s\n", record.Target.GetSpan(),
 						spanconfigtestutils.PrintSpanConfigDiffedAgainstDefaults(record.Config)))
 				}
 				return output.String()
@@ -182,7 +182,7 @@ func TestDataDriven(t *testing.T) {
 				})
 				var output strings.Builder
 				for _, record := range records {
-					output.WriteString(fmt.Sprintf("%-42s %s\n", *record.Target.GetSpan(),
+					output.WriteString(fmt.Sprintf("%-42s %s\n", record.Target.GetSpan(),
 						spanconfigtestutils.PrintSpanConfigDiffedAgainstDefaults(record.Config)))
 				}
 				return output.String()

diff --git a/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/full_translate b/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/full_translate
@@ -24,7 +24,7 @@ full-translate
 /System/NodeLiveness{-Max}                 ttl_seconds=600 num_replicas=5
 /System/{NodeLivenessMax-tsd}              range system
 /System{/tsd-tse}                          range default
-/System{tse-/Max}                          range system
+/System{tse-/SystemSpanConfigKeys}         range system
 /Table/{SystemConfigSpan/Start-4}          database system (host)
 /Table/{4-5}                               database system (host)
 /Table/{5-6}                               database system (host)

diff --git a/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/full_translate_named_zones_deleted b/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/full_translate_named_zones_deleted
@@ -40,7 +40,7 @@ full-translate
 /System/NodeLiveness{-Max}                 range default
 /System/{NodeLivenessMax-tsd}              range default
 /System{/tsd-tse}                          range default
-/System{tse-/Max}                          range default
+/System{tse-/SystemSpanConfigKeys}         range default
 /Table/{SystemConfigSpan/Start-4}          database system (host)
 /Table/{4-5}                               database system (host)
 /Table/{5-6}                               database system (host)

diff --git a/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/named_zones b/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/named_zones
@@ -47,4 +47,4 @@ translate named-zone=liveness
 translate named-zone=system
 ----
 /System/{NodeLivenessMax-tsd}              range system
-/System{tse-/Max}                          range system
+/System{tse-/SystemSpanConfigKeys}         range system
diff --git a/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/system_database b/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/system_database
@@ -111,7 +111,7 @@ full-translate
 /System/NodeLiveness{-Max}                 ttl_seconds=600 num_replicas=5
 /System/{NodeLivenessMax-tsd}              range system
 /System{/tsd-tse}                          range default
-/System{tse-/Max}                          range system
+/System{tse-/SystemSpanConfigKeys}         range system
 /Table/{SystemConfigSpan/Start-4}          ignore_strict_gc=true num_replicas=7 rangefeed_enabled=true
 /Table/{4-5}                               ignore_strict_gc=true num_replicas=7 rangefeed_enabled=true
 /Table/{5-6}                               ignore_strict_gc=true num_replicas=7 rangefeed_enabled=true

diff --git a/pkg/keys/constants.go b/pkg/keys/constants.go
@@ -301,6 +301,30 @@ var (
 	TimeseriesPrefix = roachpb.Key(makeKey(SystemPrefix, roachpb.RKey("tsd")))
 	// TimeseriesKeyMax is the maximum value for any timeseries data.
 	TimeseriesKeyMax = TimeseriesPrefix.PrefixEnd()
+	//
+	// SystemSpanConfigPrefix is the key prefix for all system span config data.
+	//
+	// We sort this at the end of the system keyspace to easily be able to exclude
+	// it from the span configuration that applies over the system keyspace. This
+	// is important because spans carved out from this range are used to store
+	// system span configurations in the `system.span_configurations` table, and
+	// as such, have special meaning associated with them; nothing is stored in
+	// the range itself.
+	SystemSpanConfigPrefix = roachpb.Key(makeKey(SystemPrefix, roachpb.RKey("\xffsys-scfg")))
+	// SystemSpanConfigEntireKeyspace is the key prefix used to denote that the
+	// associated system span configuration applies over the entire keyspace
+	// (including all secondary tenants).
+	SystemSpanConfigEntireKeyspace = roachpb.Key(makeKey(SystemSpanConfigPrefix, roachpb.RKey("host/all")))
+	// SystemSpanConfigHostOnTenantKeyspace is the key prefix used to denote that
+	// the associated system span configuration was applied by the host tenant
+	// over the keyspace of a secondary tenant.
+	SystemSpanConfigHostOnTenantKeyspace = roachpb.Key(makeKey(SystemSpanConfigPrefix, roachpb.RKey("host/ten/")))
+	// SystemSpanConfigSecondaryTenantOnEntireKeyspace is the key prefix used to
+	// denote that the associated system span configuration was applied by a
+	// secondary tenant over its entire keyspace.
+	SystemSpanConfigSecondaryTenantOnEntireKeyspace = roachpb.Key(makeKey(SystemSpanConfigPrefix, roachpb.RKey("ten/")))
+	// SystemSpanConfigKeyMax is the maximum value for any system span config key.
+	SystemSpanConfigKeyMax = SystemSpanConfigPrefix.PrefixEnd()
 
 	// 3. System tenant SQL keys
 	//

diff --git a/pkg/keys/doc.go b/pkg/keys/doc.go
@@ -246,17 +246,18 @@ var _ = [...]interface{}{
 	// 	2. System keys: This is where we store global, system data which is
 	// 	replicated across the cluster.
 	SystemPrefix,
-	NodeLivenessPrefix,  // "\x00liveness-"
-	BootstrapVersionKey, // "bootstrap-version"
-	descIDGenerator,     // "desc-idgen"
-	NodeIDGenerator,     // "node-idgen"
-	RangeIDGenerator,    // "range-idgen"
-	StatusPrefix,        // "status-"
-	StatusNodePrefix,    // "status-node-"
-	StoreIDGenerator,    // "store-idgen"
-	MigrationPrefix,     // "system-version/"
-	MigrationLease,      // "system-version/lease"
-	TimeseriesPrefix,    // "tsd"
+	NodeLivenessPrefix,     // "\x00liveness-"
+	BootstrapVersionKey,    // "bootstrap-version"
+	descIDGenerator,        // "desc-idgen"
+	NodeIDGenerator,        // "node-idgen"
+	RangeIDGenerator,       // "range-idgen"
+	StatusPrefix,           // "status-"
+	StatusNodePrefix,       // "status-node-"
+	StoreIDGenerator,       // "store-idgen"
+	MigrationPrefix,        // "system-version/"
+	MigrationLease,         // "system-version/lease"
+	TimeseriesPrefix,       // "tsd"
+	SystemSpanConfigPrefix, // "xffsys-scfg"
 	SystemMax,
 
 	// 	3. System tenant SQL keys: This is where we store all system-tenant

diff --git a/pkg/keys/printer.go b/pkg/keys/printer.go
@@ -128,6 +128,10 @@ var (
 				ppFunc: timeseriesKeyPrint,
 				PSFunc: parseUnsupported,
 			},
+			{Name: "/SystemSpanConfigKeys", prefix: SystemSpanConfigPrefix,
+				ppFunc: decodeKeyPrint,
+				PSFunc: parseUnsupported,
+			},
 		}},
 		{Name: "/NamespaceTable", start: NamespaceTableMin, end: NamespaceTableMax, Entries: []DictEntry{
 			{Name: "", prefix: nil, ppFunc: decodeKeyPrint, PSFunc: parseUnsupported},

diff --git a/pkg/keys/spans.go b/pkg/keys/spans.go
@@ -37,6 +37,12 @@ var (
 	// TimeseriesSpan holds all the timeseries data in the cluster.
 	TimeseriesSpan = roachpb.Span{Key: TimeseriesPrefix, EndKey: TimeseriesKeyMax}
 
+	// SystemSpanConfigSpan is part of the system keyspace that is used to carve
+	// out spans for system span configurations. No data is stored in these spans,
+	// instead, special meaning is assigned to them when stored in
+	// `system.span_configurations`.
+	SystemSpanConfigSpan = roachpb.Span{Key: SystemSpanConfigPrefix, EndKey: SystemSpanConfigKeyMax}
+
 	// SystemConfigSpan is the range of system objects which will be gossiped.
 	SystemConfigSpan = roachpb.Span{Key: SystemConfigSplitKey, EndKey: SystemConfigTableDataMax}
 

diff --git a/pkg/kv/kvserver/client_spanconfigs_test.go b/pkg/kv/kvserver/client_spanconfigs_test.go
@@ -72,7 +72,7 @@ func TestSpanConfigUpdateAppliedToReplica(t *testing.T) {
 	deleted, added := spanConfigStore.Apply(
 		ctx,
 		false, /* dryrun */
-		spanconfig.Addition(spanconfig.MakeSpanTarget(span), conf),
+		spanconfig.Addition(spanconfig.MakeTargetFromSpan(span), conf),
 	)
 	require.Empty(t, deleted)
 	require.Len(t, added, 1)