spanconfig: teach the KVAccessor about system span configurations

pkg/spanconfig/spanconfigkvaccessor/kvaccessor.go

@@ -43,7 +43,7 @@ type KVAccessor struct {
 	settings    *cluster.Settings
 
 	// configurationsTableFQN is typically 'system.public.span_configurations',
-	// but left configurable ease-of-testing.
+	// but left configurable for ease-of-testing.
 	configurationsTableFQN string
 }
 
@@ -433,13 +433,22 @@ func validateUpdateArgs(toDelete []spanconfig.Target, toUpsert []spanconfig.Reco
 				continue
 			}
 
-			curSpan := targets[i].GetSpan()
-			prevSpan := targets[i-1].GetSpan()
+			curTarget := targets[i]
+			prevTarget := targets[i-1]
+			if curTarget.IsSpanTarget() && prevTarget.IsSpanTarget() {
+				if curTarget.GetSpan().Overlaps(prevTarget.GetSpan()) {
+					return errors.AssertionFailedf("overlapping spans %s and %s in same list",
+						prevTarget.GetSpan(), curTarget.GetSpan())
+				}
+			}
 
-			if curSpan.Overlaps(prevSpan) {
-				return errors.AssertionFailedf("overlapping spans %s and %s in same list",
-					prevSpan, curSpan)
+			if curTarget.IsSystemTarget() && prevTarget.IsSystemTarget() && curTarget.Equal(prevTarget) {
+				return errors.AssertionFailedf("duplicate system targets %s in the same list",
+					prevTarget.GetSystemTarget())
 			}
+
+			// We're dealing with different types of target; no
+			// duplication/overlapping is possible.
 		}
 	}
 

pkg/spanconfig/spanconfigkvaccessor/kvaccessor_test.go

@@ -34,20 +34,25 @@ import (
 // 		span [a,e)
 // 		span [a,b)
 // 		span [b,c)
+//		system-target {source=1,target=20}
+//		system-target {source=1,target=1}
+//		system-target {source=20,target=20}
 //      ----
 //
 // 		kvaccessor-update
 // 		delete [c,e)
 // 		upsert [c,d):C
 // 		upsert [d,e):D
+// 		delete {source=1,target=1}
+// 		upsert {source=1,target=1}:A
 //      ----
 //
 // They tie into GetSpanConfigRecords and UpdateSpanConfigRecords
-// respectively. For kvaccessor-get, each listed span is added to the set of
-// spans being read. For kvaccessor-update, the lines prefixed with "delete"
-// count towards the spans being deleted, and for "upsert" they correspond to
+// respectively. For kvaccessor-get, each listed target is added to the set of
+// targets being read. For kvaccessor-update, the lines prefixed with "delete"
+// count towards the targets being deleted, and for "upsert" they correspond to
 // the span config entries being upserted. See
-// spanconfigtestutils.Parse{Span,Config,SpanConfigEntry} for more details.
+// spanconfigtestutils.Parse{Span,Config,SpanConfigRecord} for more details.
 func TestDataDriven(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 
@@ -77,7 +82,9 @@ func TestDataDriven(t *testing.T) {
 
 				var output strings.Builder
 				for _, record := range records {
-					output.WriteString(fmt.Sprintf("%s\n", spanconfigtestutils.PrintSpanConfigRecord(record)))
+					output.WriteString(fmt.Sprintf(
+						"%s\n", spanconfigtestutils.PrintSpanConfigRecord(t, record),
+					))
 				}
 				return output.String()
 			case "kvaccessor-update":

pkg/spanconfig/spanconfigkvaccessor/testdata/system_span_configs

@@ -0,0 +1,97 @@
+# Walk through a full set of scenarios pertaining to system span configs.
+
+# Test with an empty slate.
+kvaccessor-get
+system-target {cluster}
+system-target {source=1,target=1}
+system-target {source=1,target=10}
+system-target {source=20,target=20}
+----
+
+# Try deleting a system span configuration that doesn't exist.
+kvaccessor-update
+delete {cluster}
+----
+err: expected to delete 1 row(s), deleted 0
+
+# Basic tests that set all possible combinations of system targets.
+kvaccessor-update
+upsert {cluster}:A
+upsert {source=1,target=1}:B
+upsert {source=1,target=10}:C
+upsert {source=20,target=20}:D
+----
+ok
+
+kvaccessor-get
+system-target {cluster}
+system-target {source=1,target=1}
+system-target {source=1,target=10}
+system-target {source=20,target=20}
+----
+{cluster}:A
+{source=1,target=1}:B
+{source=1,target=10}:C
+{source=20,target=20}:D
+
+# Update some of the span configurations that we added and ensure that works.
+kvaccessor-update
+upsert {cluster}:F
+upsert {source=1,target=1}:G
+upsert {source=1,target=10}:H
+upsert {source=20,target=20}:I
+----
+ok
+
+kvaccessor-get
+system-target {cluster}
+system-target {source=1,target=1}
+system-target {source=1,target=10}
+system-target {source=20,target=20}
+----
+{cluster}:F
+{source=1,target=1}:G
+{source=1,target=10}:H
+{source=20,target=20}:I
+
+# Delete all the system span configurations that we just added and ensure
+# they take effect.
+kvaccessor-update
+delete {cluster}
+delete {source=1,target=1}
+delete {source=1,target=10}
+delete {source=20,target=20}
+----
+ok
+
+kvaccessor-get
+system-target {cluster}
+system-target {source=1,target=1}
+system-target {source=1,target=10}
+system-target {source=20,target=20}
+----
+
+# Lastly, try adding multiple system targets set by the host tenant that apply
+# to distinct secondary tenants. We also add a system span configuration set by
+# one of these secondary tenant's on itself for kicks.
+kvaccessor-update
+upsert {cluster}:Z
+upsert {source=1,target=10}:A
+upsert {source=1,target=20}:B
+upsert {source=1,target=30}:C
+upsert {source=10,target=10}:G
+----
+ok
+
+kvaccessor-get
+system-target {cluster}
+system-target {source=1,target=10}
+system-target {source=1,target=20}
+system-target {source=1,target=30}
+system-target {source=10,target=10}
+----
+{cluster}:Z
+{source=1,target=10}:A
+{source=1,target=20}:B
+{source=1,target=30}:C
+{source=10,target=10}:G

pkg/spanconfig/spanconfigkvaccessor/validation_test.go

@@ -25,6 +25,17 @@ import (
 func TestValidateUpdateArgs(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 
+	clusterTarget := spanconfig.MakeTargetFromSystemTarget(spanconfig.SystemTarget{
+		SourceTenantID: roachpb.SystemTenantID,
+		TargetTenantID: nil,
+	})
+
+	makeTenantTarget := func(id uint64) spanconfig.Target {
+		target, err := spanconfig.MakeTenantTarget(roachpb.MakeTenantID(id), roachpb.MakeTenantID(id))
+		require.NoError(t, err)
+		return spanconfig.MakeTargetFromSystemTarget(target)
+	}
+
 	for _, tc := range []struct {
 		toDelete []spanconfig.Target
 		toUpsert []spanconfig.Record
@@ -113,6 +124,65 @@ func TestValidateUpdateArgs(t *testing.T) {
 			},
 			expErr: "",
 		},
+		// Tests for system span configurations.
+		{
+			// Duplicate in toDelete.
+			toDelete: []spanconfig.Target{makeTenantTarget(10), makeTenantTarget(10)},
+			expErr:   "duplicate system targets .* in the same list",
+		},
+		{
+			// Duplicate in toUpsert.
+			toUpsert: []spanconfig.Record{
+				{
+					Target: makeTenantTarget(10),
+				},
+				{
+					Target: makeTenantTarget(10)},
+			},
+			expErr: "duplicate system targets .* in the same list",
+		},
+		{
+			// Duplicate in toDelete with some span targets.
+			toDelete: []spanconfig.Target{
+				spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("c")}),
+				clusterTarget,
+				spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("e"), EndKey: roachpb.Key("f")}),
+				clusterTarget,
+				spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("g"), EndKey: roachpb.Key("h")}),
+			},
+			expErr: "duplicate system targets .* in the same list",
+		},
+		{
+			// Duplicate in toDelete with some span targets.
+			toDelete: []spanconfig.Target{
+				spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("c")}),
+				clusterTarget,
+				spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("e"), EndKey: roachpb.Key("f")}),
+				clusterTarget,
+				spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("g"), EndKey: roachpb.Key("h")}),
+			},
+			expErr: "duplicate system targets .* in the same list",
+		},
+		{
+			// Duplicate some span/system target entries across different lists;
+			// should work.
+			toDelete: []spanconfig.Target{
+				spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("c")}),
+				makeTenantTarget(20),
+				spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("e"), EndKey: roachpb.Key("f")}),
+				clusterTarget,
+				spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("g"), EndKey: roachpb.Key("h")}),
+			},
+			toUpsert: []spanconfig.Record{
+				{
+					Target: makeTenantTarget(20),
+				},
+				{
+					Target: spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("c")}),
+				},
+			},
+			expErr: "",
+		},
 	} {
 		require.True(t, testutils.IsError(validateUpdateArgs(tc.toDelete, tc.toUpsert), tc.expErr))
 	}

pkg/spanconfig/spanconfigkvsubscriber/datadriven_test.go

@@ -181,7 +181,7 @@ func TestDataDriven(t *testing.T) {
 
 				var output strings.Builder
 				for _, record := range records {
-					output.WriteString(fmt.Sprintf("%s\n", spanconfigtestutils.PrintSpanConfigRecord(record)))
+					output.WriteString(fmt.Sprintf("%s\n", spanconfigtestutils.PrintSpanConfigRecord(t, record)))
 				}
 				return output.String()
 

pkg/spanconfig/spanconfigstore/spanconfigstore_test.go

@@ -127,7 +127,7 @@ func TestRandomized(t *testing.T) {
 		_ = store.forEachOverlapping(testSpan,
 			func(entry spanConfigEntry) error {
 				t.Fatalf("found unexpected entry: %s",
-					spanconfigtestutils.PrintSpanConfigRecord(spanconfig.Record{
+					spanconfigtestutils.PrintSpanConfigRecord(t, spanconfig.Record{
 						Target: spanconfig.MakeTargetFromSpan(entry.span),
 						Config: entry.config,
 					}))
@@ -140,7 +140,7 @@ func TestRandomized(t *testing.T) {
 			func(entry spanConfigEntry) error {
 				if !foundEntry.isEmpty() {
 					t.Fatalf("expected single overlapping entry, found second: %s",
-						spanconfigtestutils.PrintSpanConfigRecord(spanconfig.Record{
+						spanconfigtestutils.PrintSpanConfigRecord(t, spanconfig.Record{
 							Target: spanconfig.MakeTargetFromSpan(entry.span),
 							Config: entry.config,
 						}))

pkg/spanconfig/spanconfigstore/store_test.go

@@ -98,10 +98,10 @@ func TestDataDriven(t *testing.T) {
 
 				var b strings.Builder
 				for _, target := range deleted {
-					b.WriteString(fmt.Sprintf("deleted %s\n", spanconfigtestutils.PrintTarget(target)))
+					b.WriteString(fmt.Sprintf("deleted %s\n", spanconfigtestutils.PrintTarget(t, target)))
 				}
 				for _, ent := range added {
-					b.WriteString(fmt.Sprintf("added %s\n", spanconfigtestutils.PrintSpanConfigRecord(ent)))
+					b.WriteString(fmt.Sprintf("added %s\n", spanconfigtestutils.PrintSpanConfigRecord(t, ent)))
 				}
 				return b.String()
 
@@ -133,7 +133,7 @@ func TestDataDriven(t *testing.T) {
 				_ = store.TestingSpanConfigStoreForEachOverlapping(span,
 					func(entry spanConfigEntry) error {
 						results = append(results,
-							spanconfigtestutils.PrintSpanConfigRecord(spanconfig.Record{
+							spanconfigtestutils.PrintSpanConfigRecord(t, spanconfig.Record{
 								Target: spanconfig.MakeTargetFromSpan(entry.span),
 								Config: entry.config,
 							}),