From ba6ee420261b546e0d92e444f2c8ccbbd6dc4a4d Mon Sep 17 00:00:00 2001 From: arulajmani Date: Tue, 8 Feb 2022 01:02:52 -0500 Subject: [PATCH] spanconfig: introduce SystemTarget and encoding/decoding methods This patch introduces a new spanconfig.SystemTarget type. When used in conjunction with a span configuration, tenants (and the system tenant) can use this type to target multiple keyspans with the associated span configuration. We use this type to expand the a spanconfig.Target to optionally embedd a SystemTarget (in additon to a span target like it did before). We also add special encoding/decoding methods for system targets, which we will make use of when persisting them in `system.span_configurations`. We do so by reserving a special SystemSpanConfigSpan at the end of the System range. This allows us to then assign special meaning to key prefixes carved out of this range and the corresponding spans stored in `system.span_configurations`. We must take special care that the host tenant doesn't install span configurations to these keyspans directly, and as such, we make exceptions in the translator and reconciler. Release note: None --- pkg/BUILD.bazel | 1 + .../seed_tenant_span_configs_external_test.go | 14 +- .../datadriven_test.go | 2 +- .../spanconfigreconcilerccl/testdata/basic | 2 +- .../testdata/named_zones | 22 +-- .../datadriven_test.go | 4 +- .../testdata/full_translate | 2 +- .../full_translate_named_zones_deleted | 2 +- .../testdata/named_zones | 2 +- .../testdata/system_database | 2 +- pkg/keys/constants.go | 24 +++ pkg/keys/doc.go | 23 +-- pkg/keys/printer.go | 4 + pkg/keys/spans.go | 6 + pkg/kv/kvserver/client_spanconfigs_test.go | 2 +- .../migrations/migrate_span_configs_test.go | 10 +- .../migrations/seed_tenant_span_configs.go | 4 +- pkg/spanconfig/BUILD.bazel | 17 +- pkg/spanconfig/spanconfig.go | 2 +- .../spanconfigkvaccessor/kvaccessor.go | 9 +- .../spanconfigkvaccessor/validation_test.go | 22 +-- .../spanconfigkvsubscriber/kvsubscriber.go | 6 +- .../span_config_decoder_test.go | 4 +- .../spanconfigreconciler/reconciler.go | 30 ++-- .../spanconfigsqltranslator/sqltranslator.go | 22 ++- .../spanconfigstore/spanconfigstore.go | 22 +-- .../spanconfigstore/spanconfigstore_test.go | 12 +- pkg/spanconfig/spanconfigstore/store.go | 8 +- pkg/spanconfig/spanconfigstore/store_test.go | 8 +- pkg/spanconfig/spanconfigtestutils/utils.go | 8 +- pkg/spanconfig/systemtarget.go | 141 +++++++++++++++ pkg/spanconfig/target.go | 152 +++++++++++++--- pkg/spanconfig/target_test.go | 163 ++++++++++++++++++ pkg/sql/tenant.go | 4 +- 34 files changed, 610 insertions(+), 146 deletions(-) create mode 100644 pkg/spanconfig/systemtarget.go create mode 100644 pkg/spanconfig/target_test.go diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 2d06a5fd5e46..759c80c34d07 100644 --- 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/catprivilege:catprivilege_test", 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 index 71e53b452831..42e477bb598f 100644 --- 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 index 0e85dc64e406..48f211dc7519 100644 --- 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 index 7024b4e46f10..76625c86f346 100644 --- 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/named_zones b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/named_zones index bf7a0063c39f..7b379fe79757 100644 --- 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,8 +48,8 @@ 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 ---- @@ -57,7 +57,7 @@ state limit=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,8 +152,8 @@ 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 ---- @@ -161,5 +161,5 @@ state limit=5 /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/spanconfigsqltranslatorccl/datadriven_test.go b/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/datadriven_test.go index d18d470cdf65..84d106f5ca13 100644 --- 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 index 5d9ba9080aea..5c37ec3919e0 100644 --- 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 index a14690875db6..1fd076b7e38e 100644 --- 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 index 6bd36c51c3a3..cd3ab2fb49f4 100644 --- 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 index f0b296965327..711f0d116437 100644 --- 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 index fd582b1a7ff3..c26a935135b8 100644 --- 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 index 47f5fc5dfc58..2bda3c4b54c9 100644 --- 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 index 2125424da120..f2e589e2848a 100644 --- 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 index 02e513ada545..01964926faee 100644 --- 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 index e673df55d1ab..c9bfd59838fd 100644 --- 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) diff --git a/pkg/migration/migrations/migrate_span_configs_test.go b/pkg/migration/migrations/migrate_span_configs_test.go index 389bed931f57..9d1b4368d3a0 100644 --- a/pkg/migration/migrations/migrate_span_configs_test.go +++ b/pkg/migration/migrations/migrate_span_configs_test.go @@ -69,7 +69,7 @@ func TestEnsureSpanConfigReconciliation(t *testing.T) { { // Ensure that no span config entries are found. records, err := scKVAccessor.GetSpanConfigRecords(ctx, []spanconfig.Target{ - spanconfig.MakeSpanTarget(keys.EverythingSpan), + spanconfig.MakeTargetFromSpan(keys.EverythingSpan), }) require.NoError(t, err) require.Empty(t, records) @@ -91,7 +91,7 @@ func TestEnsureSpanConfigReconciliation(t *testing.T) { { // Ensure that the host tenant's span configs are installed. records, err := scKVAccessor.GetSpanConfigRecords(ctx, []spanconfig.Target{ - spanconfig.MakeSpanTarget(keys.EverythingSpan), + spanconfig.MakeTargetFromSpan(keys.EverythingSpan), }) require.NoError(t, err) require.NotEmpty(t, records) @@ -154,7 +154,7 @@ func TestEnsureSpanConfigReconciliationMultiNode(t *testing.T) { { // Ensure that no span config entries are to be found. records, err := scKVAccessor.GetSpanConfigRecords(ctx, []spanconfig.Target{ - spanconfig.MakeSpanTarget(keys.EverythingSpan), + spanconfig.MakeTargetFromSpan(keys.EverythingSpan), }) require.NoError(t, err) require.Empty(t, records) @@ -176,7 +176,7 @@ func TestEnsureSpanConfigReconciliationMultiNode(t *testing.T) { { // Ensure that the host tenant's span configs are installed. records, err := scKVAccessor.GetSpanConfigRecords(ctx, []spanconfig.Target{ - spanconfig.MakeSpanTarget(keys.EverythingSpan), + spanconfig.MakeTargetFromSpan(keys.EverythingSpan), }) require.NoError(t, err) require.NotEmpty(t, records) @@ -220,7 +220,7 @@ func TestEnsureSpanConfigSubscription(t *testing.T) { testutils.SucceedsSoon(t, func() error { records, err := scKVAccessor.GetSpanConfigRecords(ctx, []spanconfig.Target{ - spanconfig.MakeSpanTarget(keys.EverythingSpan), + spanconfig.MakeTargetFromSpan(keys.EverythingSpan), }) require.NoError(t, err) if len(records) == 0 { diff --git a/pkg/migration/migrations/seed_tenant_span_configs.go b/pkg/migration/migrations/seed_tenant_span_configs.go index c28601bf732a..c0b903c7cd9e 100644 --- a/pkg/migration/migrations/seed_tenant_span_configs.go +++ b/pkg/migration/migrations/seed_tenant_span_configs.go @@ -59,7 +59,7 @@ func seedTenantSpanConfigsMigration( // boundary. Look towards CreateTenantRecord for more details. tenantSpanConfig := d.SpanConfig.Default tenantPrefix := keys.MakeTenantPrefix(tenantID) - tenantTarget := spanconfig.MakeSpanTarget(roachpb.Span{ + tenantTarget := spanconfig.MakeTargetFromSpan(roachpb.Span{ Key: tenantPrefix, EndKey: tenantPrefix.PrefixEnd(), }) @@ -69,7 +69,7 @@ func seedTenantSpanConfigsMigration( } toUpsert := []spanconfig.Record{ { - Target: spanconfig.MakeSpanTarget(tenantSeedSpan), + Target: spanconfig.MakeTargetFromSpan(tenantSeedSpan), Config: tenantSpanConfig, }, } diff --git a/pkg/spanconfig/BUILD.bazel b/pkg/spanconfig/BUILD.bazel index 6373f91a57ff..2b1045833537 100644 --- a/pkg/spanconfig/BUILD.bazel +++ b/pkg/spanconfig/BUILD.bazel @@ -1,9 +1,10 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "spanconfig", srcs = [ "spanconfig.go", + "systemtarget.go", "target.go", "testing_knobs.go", ], @@ -16,6 +17,20 @@ go_library( "//pkg/roachpb", "//pkg/sql/catalog", "//pkg/sql/catalog/descpb", + "//pkg/util/encoding", "//pkg/util/hlc", + "@com_github_cockroachdb_errors//:errors", + ], +) + +go_test( + name = "spanconfig_test", + srcs = ["target_test.go"], + embed = [":spanconfig"], + deps = [ + "//pkg/keys", + "//pkg/roachpb", + "//pkg/testutils", + "@com_github_stretchr_testify//require", ], ) diff --git a/pkg/spanconfig/spanconfig.go b/pkg/spanconfig/spanconfig.go index c732da467353..6ed661b6e6a5 100644 --- a/pkg/spanconfig/spanconfig.go +++ b/pkg/spanconfig/spanconfig.go @@ -244,7 +244,7 @@ type Record struct { // IsEmpty returns true if the receiver is an empty Record. func (r *Record) IsEmpty() bool { - return r.Target.IsEmpty() && r.Config.IsEmpty() + return r.Target.isEmpty() && r.Config.IsEmpty() } // SQLUpdate captures either a descriptor or a protected timestamp update. diff --git a/pkg/spanconfig/spanconfigkvaccessor/kvaccessor.go b/pkg/spanconfig/spanconfigkvaccessor/kvaccessor.go index bb24b8722480..885387fc7c12 100644 --- a/pkg/spanconfig/spanconfigkvaccessor/kvaccessor.go +++ b/pkg/spanconfig/spanconfigkvaccessor/kvaccessor.go @@ -433,8 +433,8 @@ func validateUpdateArgs(toDelete []spanconfig.Target, toUpsert []spanconfig.Reco continue } - curSpan := *targets[i].GetSpan() - prevSpan := *targets[i-1].GetSpan() + curSpan := targets[i].GetSpan() + prevSpan := targets[i-1].GetSpan() if curSpan.Overlaps(prevSpan) { return errors.AssertionFailedf("overlapping spans %s and %s in same list", @@ -450,12 +450,11 @@ func validateUpdateArgs(toDelete []spanconfig.Target, toUpsert []spanconfig.Reco // are invalid or have an empty end key. func validateSpanTargets(targets []spanconfig.Target) error { for _, target := range targets { - sp := target.GetSpan() - if sp == nil { + if !target.IsSpanTarget() { // Nothing to do. continue } - if err := validateSpans(*sp); err != nil { + if err := validateSpans(target.GetSpan()); err != nil { return err } } diff --git a/pkg/spanconfig/spanconfigkvaccessor/validation_test.go b/pkg/spanconfig/spanconfigkvaccessor/validation_test.go index 5ce037c62739..8bdf9d26fd5f 100644 --- a/pkg/spanconfig/spanconfigkvaccessor/validation_test.go +++ b/pkg/spanconfig/spanconfigkvaccessor/validation_test.go @@ -36,7 +36,7 @@ func TestValidateUpdateArgs(t *testing.T) { }, { toDelete: []spanconfig.Target{ - spanconfig.MakeSpanTarget( + spanconfig.MakeTargetFromSpan( roachpb.Span{Key: roachpb.Key("a")}, // empty end key in delete list ), }, @@ -45,7 +45,7 @@ func TestValidateUpdateArgs(t *testing.T) { { toUpsert: []spanconfig.Record{ { - Target: spanconfig.MakeSpanTarget( + Target: spanconfig.MakeTargetFromSpan( roachpb.Span{Key: roachpb.Key("a")}, // empty end key in update list ), }, @@ -55,7 +55,7 @@ func TestValidateUpdateArgs(t *testing.T) { { toUpsert: []spanconfig.Record{ { - Target: spanconfig.MakeSpanTarget( + Target: spanconfig.MakeTargetFromSpan( roachpb.Span{Key: roachpb.Key("b"), EndKey: roachpb.Key("a")}, // invalid span; end < start ), }, @@ -64,7 +64,7 @@ func TestValidateUpdateArgs(t *testing.T) { }, { toDelete: []spanconfig.Target{ - spanconfig.MakeSpanTarget( + spanconfig.MakeTargetFromSpan( roachpb.Span{Key: roachpb.Key("b"), EndKey: roachpb.Key("a")}, // invalid span; end < start ), }, @@ -73,20 +73,20 @@ func TestValidateUpdateArgs(t *testing.T) { { toDelete: []spanconfig.Target{ // overlapping spans in the same list. - spanconfig.MakeSpanTarget(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("c")}), - spanconfig.MakeSpanTarget(roachpb.Span{Key: roachpb.Key("b"), EndKey: roachpb.Key("c")}), + spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("c")}), + spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("b"), EndKey: roachpb.Key("c")}), }, expErr: "overlapping spans {a-c} and {b-c} in same list", }, { toUpsert: []spanconfig.Record{ // overlapping spans in the same list { - Target: spanconfig.MakeSpanTarget( + Target: spanconfig.MakeTargetFromSpan( roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("c")}, ), }, { - Target: spanconfig.MakeSpanTarget( + Target: spanconfig.MakeTargetFromSpan( roachpb.Span{Key: roachpb.Key("b"), EndKey: roachpb.Key("c")}, ), }, @@ -97,16 +97,16 @@ func TestValidateUpdateArgs(t *testing.T) { // Overlapping spans in different lists. toDelete: []spanconfig.Target{ // Overlapping spans in the same list. - spanconfig.MakeSpanTarget(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("c")}), + spanconfig.MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("c")}), }, toUpsert: []spanconfig.Record{ { - Target: spanconfig.MakeSpanTarget( + Target: spanconfig.MakeTargetFromSpan( roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("b")}, ), }, { - Target: spanconfig.MakeSpanTarget( + Target: spanconfig.MakeTargetFromSpan( roachpb.Span{Key: roachpb.Key("b"), EndKey: roachpb.Key("c")}, ), }, diff --git a/pkg/spanconfig/spanconfigkvsubscriber/kvsubscriber.go b/pkg/spanconfig/spanconfigkvsubscriber/kvsubscriber.go index e8c5292e3bad..d08ba9c8638c 100644 --- a/pkg/spanconfig/spanconfigkvsubscriber/kvsubscriber.go +++ b/pkg/spanconfig/spanconfigkvsubscriber/kvsubscriber.go @@ -253,9 +253,9 @@ func (s *KVSubscriber) handlePartialUpdate( // TODO(arul): In the future, once we start reacting to system span // configurations, we'll want to invoke handlers with the correct span // here as well. - sp := ev.(*bufferEvent).Update.Target.GetSpan() - if sp != nil { - h.invoke(ctx, *sp) + target := ev.(*bufferEvent).Update.Target + if target.IsSpanTarget() { + h.invoke(ctx, target.GetSpan()) } } } diff --git a/pkg/spanconfig/spanconfigkvsubscriber/span_config_decoder_test.go b/pkg/spanconfig/spanconfigkvsubscriber/span_config_decoder_test.go index b8e4169ad080..8fd161968c9b 100644 --- a/pkg/spanconfig/spanconfigkvsubscriber/span_config_decoder_test.go +++ b/pkg/spanconfig/spanconfigkvsubscriber/span_config_decoder_test.go @@ -80,8 +80,8 @@ func TestSpanConfigDecoder(t *testing.T) { }, ) require.NoError(t, err) - require.Truef(t, span.Equal(*got.Target.GetSpan()), - "expected span=%s, got span=%s", span, *got.Target.GetSpan()) + require.Truef(t, span.Equal(got.Target.GetSpan()), + "expected span=%s, got span=%s", span, got.Target.GetSpan()) require.Truef(t, conf.Equal(got.Config), "expected config=%s, got config=%s", conf, got.Config) } diff --git a/pkg/spanconfig/spanconfigreconciler/reconciler.go b/pkg/spanconfig/spanconfigreconciler/reconciler.go index 0ada0176549f..ac53c2975924 100644 --- a/pkg/spanconfig/spanconfigreconciler/reconciler.go +++ b/pkg/spanconfig/spanconfigreconciler/reconciler.go @@ -270,7 +270,7 @@ func (f *fullReconciler) reconcile( func (f *fullReconciler) fetchExistingSpanConfigs( ctx context.Context, ) (*spanconfigstore.Store, error) { - var target spanconfig.Target + var targets []spanconfig.Target if f.codec.ForSystemTenant() { // The system tenant governs all system keys (meta, liveness, timeseries // ranges, etc.) and system tenant tables. @@ -278,28 +278,36 @@ func (f *fullReconciler) fetchExistingSpanConfigs( // TODO(irfansharif): Should we include the scratch range here? Some // tests make use of it; we may want to declare configs over it and have // it considered all the same. - target = spanconfig.MakeSpanTarget(roachpb.Span{ + // + // We don't want to request configs that are part of the + // SystemSpanConfigSpan, as the host tenant reserves that part of the + // keyspace to translate and persist SystemSpanConfigs. At the level of the + // reconciler we shouldn't be requesting these configs directly, instead, + // they should be targeted through SystemSpanConfigTargets instead. + targets = append(targets, spanconfig.MakeTargetFromSpan(roachpb.Span{ Key: keys.EverythingSpan.Key, + EndKey: keys.SystemSpanConfigSpan.Key, + })) + targets = append(targets, spanconfig.MakeTargetFromSpan(roachpb.Span{ + Key: keys.TableDataMin, EndKey: keys.TableDataMax, - }) + })) if f.knobs.ConfigureScratchRange { - target.GetSpan().EndKey = keys.ScratchRangeMax + sp := targets[1].GetSpan() + targets[1] = spanconfig.MakeTargetFromSpan(roachpb.Span{Key: sp.Key, EndKey: keys.ScratchRangeMax}) } } else { // Secondary tenants govern everything prefixed by their tenant ID. tenPrefix := keys.MakeTenantPrefix(f.tenID) - target = spanconfig.MakeSpanTarget(roachpb.Span{ + targets = append(targets, spanconfig.MakeTargetFromSpan(roachpb.Span{ Key: tenPrefix, EndKey: tenPrefix.PrefixEnd(), - }) + })) } - store := spanconfigstore.New(roachpb.SpanConfig{}) { // Fully populate the store with KVAccessor contents. - records, err := f.kvAccessor.GetSpanConfigRecords(ctx, []spanconfig.Target{ - target, - }) + records, err := f.kvAccessor.GetSpanConfigRecords(ctx, targets) if err != nil { return nil, err } @@ -398,7 +406,7 @@ func (r *incrementalReconciler) reconcile( Key: r.codec.TablePrefix(uint32(missingID)), EndKey: r.codec.TablePrefix(uint32(missingID)).PrefixEnd(), } - updates = append(updates, spanconfig.Deletion(spanconfig.MakeSpanTarget(tableSpan))) + updates = append(updates, spanconfig.Deletion(spanconfig.MakeTargetFromSpan(tableSpan))) } toDelete, toUpsert := r.storeWithKVContents.Apply(ctx, false /* dryrun */, updates...) diff --git a/pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go b/pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go index 70943d8704d6..4ea06cbad023 100644 --- a/pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go +++ b/pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go @@ -209,6 +209,10 @@ func (s *SQLTranslator) generateSpanConfigurationsForNamedZone( // Add spans for the system range without the timeseries and // liveness ranges, which are individually captured above. // + // We also don't apply configurations over the SystemSpanConfigSpan; spans + // carved from this range have no data, instead, associated configurations + // for these spans have special meaning in `system.span_configurations`. + // // Note that the NodeLivenessSpan sorts before the rest of the system // keyspace, so the first span here starts at the end of the // NodeLivenessSpan. @@ -218,7 +222,7 @@ func (s *SQLTranslator) generateSpanConfigurationsForNamedZone( }) spans = append(spans, roachpb.Span{ Key: keys.TimeseriesSpan.EndKey, - EndKey: keys.SystemMax, + EndKey: keys.SystemSpanConfigSpan.Key, }) case zonepb.TenantsZoneName: // nothing to do. default: @@ -233,7 +237,7 @@ func (s *SQLTranslator) generateSpanConfigurationsForNamedZone( var records []spanconfig.Record for _, span := range spans { records = append(records, spanconfig.Record{ - Target: spanconfig.MakeSpanTarget(span), + Target: spanconfig.MakeTargetFromSpan(span), Config: spanConfig, }) } @@ -290,7 +294,7 @@ func (s *SQLTranslator) generateSpanConfigurationsForTable( // but looking at range boundaries, it's slightly less confusing // this way. records = append(records, spanconfig.Record{ - Target: spanconfig.MakeSpanTarget(roachpb.Span{ + Target: spanconfig.MakeTargetFromSpan(roachpb.Span{ Key: s.codec.TenantPrefix(), EndKey: tableEndKey, }), @@ -307,7 +311,7 @@ func (s *SQLTranslator) generateSpanConfigurationsForTable( // (tiny) re-splitting costs when switching between the two // subsystems. records = append(records, spanconfig.Record{ - Target: spanconfig.MakeSpanTarget(roachpb.Span{ + Target: spanconfig.MakeTargetFromSpan(roachpb.Span{ Key: keys.SystemConfigSpan.Key, EndKey: tableEndKey, }), @@ -371,7 +375,7 @@ func (s *SQLTranslator) generateSpanConfigurationsForTable( if !prevEndKey.Equal(span.Key) { records = append(records, spanconfig.Record{ - Target: spanconfig.MakeSpanTarget(roachpb.Span{Key: prevEndKey, EndKey: span.Key}), + Target: spanconfig.MakeTargetFromSpan(roachpb.Span{Key: prevEndKey, EndKey: span.Key}), Config: tableSpanConfig, }, ) @@ -388,7 +392,7 @@ func (s *SQLTranslator) generateSpanConfigurationsForTable( } records = append(records, spanconfig.Record{ - Target: spanconfig.MakeSpanTarget(roachpb.Span{Key: span.Key, EndKey: span.EndKey}), + Target: spanconfig.MakeTargetFromSpan(roachpb.Span{Key: span.Key, EndKey: span.EndKey}), Config: subzoneSpanConfig, }, ) @@ -401,7 +405,7 @@ func (s *SQLTranslator) generateSpanConfigurationsForTable( if !prevEndKey.Equal(tableEndKey) { records = append(records, spanconfig.Record{ - Target: spanconfig.MakeSpanTarget(roachpb.Span{Key: prevEndKey, EndKey: tableEndKey}), + Target: spanconfig.MakeTargetFromSpan(roachpb.Span{Key: prevEndKey, EndKey: tableEndKey}), Config: tableSpanConfig, }, ) @@ -572,7 +576,7 @@ func (s *SQLTranslator) maybeGeneratePseudoTableRecords( tableStartKey := s.codec.TablePrefix(pseudoTableID) tableEndKey := tableStartKey.PrefixEnd() records = append(records, spanconfig.Record{ - Target: spanconfig.MakeSpanTarget(roachpb.Span{ + Target: spanconfig.MakeTargetFromSpan(roachpb.Span{ Key: tableStartKey, EndKey: tableEndKey, }), @@ -604,7 +608,7 @@ func (s *SQLTranslator) maybeGenerateScratchRangeRecord( } return spanconfig.Record{ - Target: spanconfig.MakeSpanTarget(roachpb.Span{ + Target: spanconfig.MakeTargetFromSpan(roachpb.Span{ Key: keys.ScratchRangeMin, EndKey: keys.ScratchRangeMax, }), diff --git a/pkg/spanconfig/spanconfigstore/spanconfigstore.go b/pkg/spanconfig/spanconfigstore/spanconfigstore.go index af65401cfb1e..2e3af504697f 100644 --- a/pkg/spanconfig/spanconfigstore/spanconfigstore.go +++ b/pkg/spanconfig/spanconfigstore/spanconfigstore.go @@ -44,7 +44,7 @@ func (s *spanConfigStore) copy(ctx context.Context) *spanConfigStore { clone := newSpanConfigStore() _ = s.forEachOverlapping(keys.EverythingSpan, func(entry spanConfigEntry) error { _, _, err := clone.apply(false /* dryrun */, spanconfig.Update{ - Target: spanconfig.MakeSpanTarget(entry.span), + Target: spanconfig.MakeTargetFromSpan(entry.span), Config: entry.config, }) if err != nil { @@ -298,15 +298,15 @@ func (s *spanConfigStore) accumulateOpsFor( } var ( - union = existing.span.Combine(*update.Target.GetSpan()) - inter = existing.span.Intersect(*update.Target.GetSpan()) + union = existing.span.Combine(update.Target.GetSpan()) + inter = existing.span.Intersect(update.Target.GetSpan()) pre = roachpb.Span{Key: union.Key, EndKey: inter.Key} post = roachpb.Span{Key: inter.EndKey, EndKey: union.EndKey} ) if update.Addition() { - if existing.span.Equal(*update.Target.GetSpan()) && existing.config.Equal(update.Config) { + if existing.span.Equal(update.Target.GetSpan()) && existing.config.Equal(update.Config) { skipAddingSelf = true break // no-op; peep-hole optimization } @@ -349,7 +349,7 @@ func (s *spanConfigStore) accumulateOpsFor( if update.Addition() && !skipAddingSelf { // Add the update itself. - toAdd = append(toAdd, s.makeEntry(*update.Target.GetSpan(), update.Config)) + toAdd = append(toAdd, s.makeEntry(update.Target.GetSpan(), update.Config)) // TODO(irfansharif): If we're adding an entry, we could inspect the // entries before and after and check whether either of them have @@ -385,11 +385,11 @@ func (s *spanConfigStore) makeEntry(sp roachpb.Span, conf roachpb.SpanConfig) sp // spans, those spans be valid, and non-overlapping. func validateApplyArgs(updates ...spanconfig.Update) error { for i := range updates { - sp := updates[i].Target.GetSpan() - - if sp == nil { + if !updates[i].Target.IsSpanTarget() { return errors.New("expected update to target a span") } + + sp := updates[i].Target.GetSpan() if !sp.Valid() || len(sp.EndKey) == 0 { return errors.New("invalid span") } @@ -406,11 +406,11 @@ func validateApplyArgs(updates ...spanconfig.Update) error { if i == 0 { continue } - if updates[i].Target.GetSpan().Overlaps(*updates[i-1].Target.GetSpan()) { + if updates[i].Target.GetSpan().Overlaps(updates[i-1].Target.GetSpan()) { return errors.Newf( "found overlapping updates %s and %s", - *updates[i-1].Target.GetSpan(), - *updates[i].Target.GetSpan(), + updates[i-1].Target.GetSpan(), + updates[i].Target.GetSpan(), ) } } diff --git a/pkg/spanconfig/spanconfigstore/spanconfigstore_test.go b/pkg/spanconfig/spanconfigstore/spanconfigstore_test.go index 1cc15adaf516..0c285477c60f 100644 --- a/pkg/spanconfig/spanconfigstore/spanconfigstore_test.go +++ b/pkg/spanconfig/spanconfigstore/spanconfigstore_test.go @@ -65,9 +65,9 @@ func TestRandomized(t *testing.T) { sp, conf, op := genRandomSpan(), getRandomConf(), getRandomOp() switch op { case "set": - return spanconfig.Addition(spanconfig.MakeSpanTarget(sp), conf) + return spanconfig.Addition(spanconfig.MakeTargetFromSpan(sp), conf) case "del": - return spanconfig.Deletion(spanconfig.MakeSpanTarget(sp)) + return spanconfig.Deletion(spanconfig.MakeTargetFromSpan(sp)) default: } t.Fatalf("unexpected op: %s", op) @@ -86,7 +86,7 @@ func TestRandomized(t *testing.T) { }) invalid := false for i := 1; i < numUpdates; i++ { - if updates[i].Target.GetSpan().Overlaps(*updates[i-1].Target.GetSpan()) { + if updates[i].Target.GetSpan().Overlaps(updates[i-1].Target.GetSpan()) { invalid = true } } @@ -113,7 +113,7 @@ func TestRandomized(t *testing.T) { _, _, err := store.apply(false /* dryrun */, updates...) require.NoError(t, err) for _, update := range updates { - if testSpan.Overlaps(*update.Target.GetSpan()) { + if testSpan.Overlaps(update.Target.GetSpan()) { if update.Addition() { expConfig, expFound = update.Config, true } else { @@ -128,7 +128,7 @@ func TestRandomized(t *testing.T) { func(entry spanConfigEntry) error { t.Fatalf("found unexpected entry: %s", spanconfigtestutils.PrintSpanConfigRecord(spanconfig.Record{ - Target: spanconfig.MakeSpanTarget(entry.span), + Target: spanconfig.MakeTargetFromSpan(entry.span), Config: entry.config, })) return nil @@ -141,7 +141,7 @@ func TestRandomized(t *testing.T) { if !foundEntry.isEmpty() { t.Fatalf("expected single overlapping entry, found second: %s", spanconfigtestutils.PrintSpanConfigRecord(spanconfig.Record{ - Target: spanconfig.MakeSpanTarget(entry.span), + Target: spanconfig.MakeTargetFromSpan(entry.span), Config: entry.config, })) } diff --git a/pkg/spanconfig/spanconfigstore/store.go b/pkg/spanconfig/spanconfigstore/store.go index c3e803bb2b46..41004fb5a0e7 100644 --- a/pkg/spanconfig/spanconfigstore/store.go +++ b/pkg/spanconfig/spanconfigstore/store.go @@ -129,7 +129,7 @@ func (s *Store) applyInternal( spanStoreUpdates := make([]spanconfig.Update, 0, len(updates)) for _, update := range updates { // TODO(arul): We'll hijack system span configurations here. - if update.Target.GetSpan() != nil { + if update.Target.IsSpanTarget() { spanStoreUpdates = append(spanStoreUpdates, update) } } @@ -139,12 +139,12 @@ func (s *Store) applyInternal( } for _, sp := range deletedSpans { - deleted = append(deleted, spanconfig.MakeSpanTarget(sp)) + deleted = append(deleted, spanconfig.MakeTargetFromSpan(sp)) } for _, entry := range addedEntries { added = append(added, spanconfig.Record{ - Target: spanconfig.MakeSpanTarget(entry.span), + Target: spanconfig.MakeTargetFromSpan(entry.span), Config: entry.config, }) } @@ -159,7 +159,7 @@ func (s *Store) Iterate(f func(spanconfig.Record) error) error { keys.EverythingSpan, func(s spanConfigEntry) error { return f(spanconfig.Record{ - Target: spanconfig.MakeSpanTarget(s.span), + Target: spanconfig.MakeTargetFromSpan(s.span), Config: s.config, }) }) diff --git a/pkg/spanconfig/spanconfigstore/store_test.go b/pkg/spanconfig/spanconfigstore/store_test.go index b083ba49823f..5b11e22d2629 100644 --- a/pkg/spanconfig/spanconfigstore/store_test.go +++ b/pkg/spanconfig/spanconfigstore/store_test.go @@ -134,7 +134,7 @@ func TestDataDriven(t *testing.T) { func(entry spanConfigEntry) error { results = append(results, spanconfigtestutils.PrintSpanConfigRecord(spanconfig.Record{ - Target: spanconfig.MakeSpanTarget(entry.span), + Target: spanconfig.MakeTargetFromSpan(entry.span), Config: entry.config, }), ) @@ -161,15 +161,15 @@ func TestStoreClone(t *testing.T) { updates := []spanconfig.Update{ spanconfig.Addition( - spanconfig.MakeSpanTarget(spanconfigtestutils.ParseSpan(t, "[a, b)")), + spanconfig.MakeTargetFromSpan(spanconfigtestutils.ParseSpan(t, "[a, b)")), spanconfigtestutils.ParseConfig(t, "A"), ), spanconfig.Addition( - spanconfig.MakeSpanTarget(spanconfigtestutils.ParseSpan(t, "[c, d)")), + spanconfig.MakeTargetFromSpan(spanconfigtestutils.ParseSpan(t, "[c, d)")), spanconfigtestutils.ParseConfig(t, "C"), ), spanconfig.Addition( - spanconfig.MakeSpanTarget(spanconfigtestutils.ParseSpan(t, "[e, f)")), + spanconfig.MakeTargetFromSpan(spanconfigtestutils.ParseSpan(t, "[e, f)")), spanconfigtestutils.ParseConfig(t, "E"), ), } diff --git a/pkg/spanconfig/spanconfigtestutils/utils.go b/pkg/spanconfig/spanconfigtestutils/utils.go index 2aa79126575d..2345411d8278 100644 --- a/pkg/spanconfig/spanconfigtestutils/utils.go +++ b/pkg/spanconfig/spanconfigtestutils/utils.go @@ -56,7 +56,7 @@ func ParseSpan(t *testing.T, sp string) roachpb.Span { // TODO(arul): Once we have system targets, we'll want to parse them here too // instead of just calling ParseSpan here. func ParseTarget(t *testing.T, target string) spanconfig.Target { - return spanconfig.MakeSpanTarget(ParseSpan(t, target)) + return spanconfig.MakeTargetFromSpan(ParseSpan(t, target)) } // ParseConfig is helper function that constructs a roachpb.SpanConfig that's @@ -194,10 +194,10 @@ func PrintSpan(sp roachpb.Span) string { // PrintTarget is a helper function that prints a spanconfig.Target. func PrintTarget(target spanconfig.Target) string { - if target.GetSpan() != nil { - return PrintSpan(*target.GetSpan()) + if target.IsSpanTarget() { + return PrintSpan(target.GetSpan()) } - panic("targets other than span targets are unsupported") + panic("targets other than span targets are unsupported for now") } // PrintSpanConfig is a helper function that transforms roachpb.SpanConfig into diff --git a/pkg/spanconfig/systemtarget.go b/pkg/spanconfig/systemtarget.go new file mode 100644 index 000000000000..44bcbdcdee4a --- /dev/null +++ b/pkg/spanconfig/systemtarget.go @@ -0,0 +1,141 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package spanconfig + +import ( + "bytes" + "fmt" + + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/util/encoding" + "github.com/cockroachdb/errors" +) + +// SystemTarget specifies the target of a system span configuration. +type SystemTarget struct { + // SourceTenantID is the ID of the tenant that specified the system span + // configuration. + // SourceTenantID is the ID of the tenant that specified the system span + // configuration. + SourceTenantID roachpb.TenantID + + // TargetTenantID is the ID of the tenant that the associated system span + // configuration applies to. + // + // If the host tenant is the source and targeting itself then the target + // applies over all ranges in the system (including those belonging to + // secondary tenants). + // + // Secondary tenants are only allowed to target themselves. The host tenant + // may use this field to target a specific secondary tenant. We validate this + // when constructing new system targets. + TargetTenantID roachpb.TenantID +} + +// MakeSystemTarget constructs, validates, and returns a new SystemTarget. +func MakeSystemTarget( + sourceTenantID roachpb.TenantID, targetTenantID roachpb.TenantID, +) (SystemTarget, error) { + t := SystemTarget{ + SourceTenantID: sourceTenantID, + TargetTenantID: targetTenantID, + } + return t, t.validate() +} + +// encode returns an encoded span associated with the receiver which is suitable +// for persistence in system.span_configurations. +func (st SystemTarget) encode() roachpb.Span { + var k roachpb.Key + + if st.SourceTenantID == roachpb.SystemTenantID && + st.TargetTenantID == roachpb.SystemTenantID { + k = keys.SystemSpanConfigEntireKeyspace + } else if st.SourceTenantID == roachpb.SystemTenantID { + k = encoding.EncodeUvarintAscending( + keys.SystemSpanConfigHostOnTenantKeyspace, st.TargetTenantID.InternalValue, + ) + } else { + k = encoding.EncodeUvarintAscending( + keys.SystemSpanConfigSecondaryTenantOnEntireKeyspace, st.SourceTenantID.InternalValue, + ) + } + return roachpb.Span{Key: k, EndKey: k.PrefixEnd()} +} + +// validate ensures that the receiver is well-formed. +func (st SystemTarget) validate() error { + if st.SourceTenantID != roachpb.SystemTenantID && st.SourceTenantID != st.TargetTenantID { + return errors.AssertionFailedf( + "secondary tenant %s cannot target another tenant with ID %s", + st.SourceTenantID, + st.TargetTenantID, + ) + } + return nil +} + +// isEmpty returns true if the receiver is empty. +func (st SystemTarget) isEmpty() bool { + return st.TargetTenantID.Equal(roachpb.TenantID{}) && + st.SourceTenantID.Equal(roachpb.TenantID{}) +} + +// String returns a pretty printed version of a system target. +func (st SystemTarget) String() string { + return fmt.Sprintf("{system target source: %s target: %s}", st.SourceTenantID, st.TargetTenantID) +} + +// decodeSystemTarget converts the given span into a SystemTarget. An error is +// returned if the supplied span does not conform to a system target's +// encoding. +func decodeSystemTarget(span roachpb.Span) (SystemTarget, error) { + // Validate the end key is well-formed. + if !span.EndKey.Equal(span.Key.PrefixEnd()) { + return SystemTarget{}, errors.AssertionFailedf("invalid end key in span %s", span) + } + switch { + case bytes.Equal(span.Key, keys.SystemSpanConfigEntireKeyspace): + return MakeSystemTarget(roachpb.SystemTenantID, roachpb.SystemTenantID) + case bytes.HasPrefix(span.Key, keys.SystemSpanConfigHostOnTenantKeyspace): + // System span config was applied by the host tenant over a secondary + // tenant's entire keyspace. + tenIDBytes := span.Key[len(keys.SystemSpanConfigHostOnTenantKeyspace):] + _, tenIDRaw, err := encoding.DecodeUvarintAscending(tenIDBytes) + if err != nil { + return SystemTarget{}, err + } + tenID := roachpb.MakeTenantID(tenIDRaw) + return MakeSystemTarget(roachpb.SystemTenantID, tenID) + case bytes.HasPrefix(span.Key, keys.SystemSpanConfigSecondaryTenantOnEntireKeyspace): + // System span config was applied by a secondary tenant over its entire + // keyspace. + tenIDBytes := span.Key[len(keys.SystemSpanConfigSecondaryTenantOnEntireKeyspace):] + _, tenIDRaw, err := encoding.DecodeUvarintAscending(tenIDBytes) + if err != nil { + return SystemTarget{}, err + } + tenID := roachpb.MakeTenantID(tenIDRaw) + return MakeSystemTarget(tenID, tenID) + default: + return SystemTarget{}, + errors.AssertionFailedf("span %s did not conform to SystemTarget encoding", span) + } +} + +// spanStartKeyConformsToSystemTargetEncoding returns true if the given span's +// start key conforms to the key encoding of system span configurations. +func spanStartKeyConformsToSystemTargetEncoding(span roachpb.Span) bool { + return bytes.Equal(span.Key, keys.SystemSpanConfigEntireKeyspace) || + bytes.HasPrefix(span.Key, keys.SystemSpanConfigHostOnTenantKeyspace) || + bytes.HasPrefix(span.Key, keys.SystemSpanConfigSecondaryTenantOnEntireKeyspace) +} diff --git a/pkg/spanconfig/target.go b/pkg/spanconfig/target.go index 61878807471a..9de180ea2342 100644 --- a/pkg/spanconfig/target.go +++ b/pkg/spanconfig/target.go @@ -13,81 +13,179 @@ package spanconfig import "github.com/cockroachdb/cockroach/pkg/roachpb" // Target specifies the target of an associated span configuration. -// -// TODO(arul): In the future, we will expand this to include system targets. type Target struct { span roachpb.Span + + systemTarget SystemTarget } // MakeTarget returns a new Target. func MakeTarget(t roachpb.SpanConfigTarget) Target { switch t.Union.(type) { case *roachpb.SpanConfigTarget_Span: - return MakeSpanTarget(*t.GetSpan()) + return MakeTargetFromSpan(*t.GetSpan()) + // TODO(arul): Add a case here for SpanConfigTarget_SystemTarget once we've + // taught and tested the KVAccessor to work with system targets. default: panic("cannot handle target") } } -// MakeSpanTarget constructs and returns a span target. -func MakeSpanTarget(span roachpb.Span) Target { +// MakeTargetFromSpan constructs and returns a span target. +func MakeTargetFromSpan(span roachpb.Span) Target { return Target{span: span} } -// GetSpan returns the underlying roachpb.Span if the target is a span target -// and nil otherwise. -func (t *Target) GetSpan() *roachpb.Span { - if t.span.Equal(roachpb.Span{}) { - return nil +// MakeTargetFromSystemTarget returns a Target which wraps a system target. +func MakeTargetFromSystemTarget(systemTarget SystemTarget) Target { + return Target{systemTarget: systemTarget} +} + +// IsSpanTarget returns true if the target is a span target. +func (t Target) IsSpanTarget() bool { + return !t.span.Equal(roachpb.Span{}) +} + +// GetSpan returns the underlying roachpb.Span if the target is a span +// target; panics if that isn't he case. +func (t Target) GetSpan() roachpb.Span { + if !t.IsSpanTarget() { + panic("target is not a span target") } - return &t.span + return t.span +} + +// IsSystemTarget returns true if the underlying target is a system target. +func (t Target) IsSystemTarget() bool { + return !t.systemTarget.isEmpty() +} + +// GetSystemTarget returns the underlying SystemTarget; it panics if that is not +// the case. +func (t Target) GetSystemTarget() SystemTarget { + if !t.IsSystemTarget() { + panic("target is not a system target") + } + return t.systemTarget } // Encode returns an encoded span suitable for persistence in // system.span_configurations. func (t Target) Encode() roachpb.Span { - if t.GetSpan() != nil { + switch { + case t.IsSpanTarget(): return t.span + case t.IsSystemTarget(): + return t.systemTarget.encode() + default: + panic("cannot handle any other type of target") } - panic("cannot handle any other type of target") } -// Less returns true if the receiver is less than the supplied target. -func (t *Target) Less(o Target) bool { - return t.Encode().Key.Compare(o.Encode().Key) < 0 +// Less returns true if the receiver is considered less than the supplied +// target. +func (t Target) Less(o Target) bool { + // We consider system targets to be less than span targets. + + // If we're dealing with both system targets, sort by: + // - host installed targets come first (ordered by target tenant ID). + // - secondary tenant installed targets come next, ordered by secondary tenant + // ID. + if t.IsSystemTarget() && o.IsSystemTarget() { + if t.GetSystemTarget().SourceTenantID == roachpb.SystemTenantID && + o.GetSystemTarget().SourceTenantID == roachpb.SystemTenantID { + return t.GetSystemTarget().TargetTenantID.InternalValue < + o.GetSystemTarget().TargetTenantID.InternalValue + } + + if t.GetSystemTarget().SourceTenantID == roachpb.SystemTenantID { + return true + } else if o.GetSystemTarget().SourceTenantID == roachpb.SystemTenantID { + return false + } + + return t.GetSystemTarget().SourceTenantID.InternalValue < + o.GetSystemTarget().SourceTenantID.InternalValue + } + + // Check if one of the targets is a system target and return accordingly. + if t.IsSystemTarget() { + return true + } else if o.IsSystemTarget() { + return false + } + + // We're dealing with 2 span targets; compare their start keys. + return t.GetSpan().Key.Compare(o.GetSpan().Key) < 0 } // Equal returns true iff the receiver is equal to the supplied target. -func (t *Target) Equal(o Target) bool { - return t.GetSpan().Equal(*o.GetSpan()) +func (t Target) Equal(o Target) bool { + if t.IsSpanTarget() && o.IsSpanTarget() { + return t.GetSpan().Equal(o.GetSpan()) + } + + if t.IsSystemTarget() && o.IsSystemTarget() { + return t.systemTarget.SourceTenantID == o.systemTarget.SourceTenantID && + t.systemTarget.TargetTenantID == o.systemTarget.TargetTenantID + } + + // We're dealing with one span target and one system target, so they're not + // equal. + return false } // String returns a formatted version of the traget suitable for printing. func (t Target) String() string { - return t.GetSpan().String() + if t.IsSpanTarget() { + return t.GetSpan().String() + } + return t.GetSystemTarget().String() } -// IsEmpty returns true if the receiver is an empty target. -func (t Target) IsEmpty() bool { - return t.GetSpan().Equal(roachpb.Span{}) - +// isEmpty returns true if the receiver is an empty target. +func (t Target) isEmpty() bool { + if t.systemTarget.isEmpty() && t.span.Equal(roachpb.Span{}) { + return true + } + return false } +// SpanConfigTargetProto returns a roachpb.SpanConfigTarget equivalent to the +// receiver. func (t Target) SpanConfigTargetProto() roachpb.SpanConfigTarget { - if t.GetSpan() != nil { + switch { + case t.IsSpanTarget(): + sp := t.GetSpan() return roachpb.SpanConfigTarget{ Union: &roachpb.SpanConfigTarget_Span{ - Span: t.GetSpan(), + Span: &sp, + }, + } + case t.IsSystemTarget(): + return roachpb.SpanConfigTarget{ + Union: &roachpb.SpanConfigTarget_SystemSpanConfigTarget{ + SystemSpanConfigTarget: &roachpb.SystemSpanConfigTarget{ + SourceTenantID: t.GetSystemTarget().SourceTenantID, + TargetTenantID: t.GetSystemTarget().TargetTenantID, + }, }, } + default: + panic("cannot handle any other type of target") } - - panic("cannot handle any other type of target") } // DecodeTarget takes a raw span and decodes it into a Target given its // encoding. It is the inverse of Encode. func DecodeTarget(span roachpb.Span) Target { + if spanStartKeyConformsToSystemTargetEncoding(span) { + systemTarget, err := decodeSystemTarget(span) + if err != nil { + panic(err) + } + return Target{systemTarget: systemTarget} + } return Target{span: span} } diff --git a/pkg/spanconfig/target_test.go b/pkg/spanconfig/target_test.go new file mode 100644 index 000000000000..cc8ad863a6cf --- /dev/null +++ b/pkg/spanconfig/target_test.go @@ -0,0 +1,163 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package spanconfig + +import ( + "math/rand" + "sort" + "testing" + + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/testutils" + "github.com/stretchr/testify/require" +) + +// TestEncodeDecodeSystemTarget ensures that encoding/decoding a SystemTarget +// is roundtripable. +func TestEncodeDecodeSystemTarget(t *testing.T) { + for _, testTarget := range []SystemTarget{ + // Tenant targeting its logical cluster. + makeSystemTargetOrFatal(t, roachpb.MakeTenantID(10), roachpb.MakeTenantID(10)), + // System tenant targeting its logical cluster. + makeSystemTargetOrFatal(t, roachpb.SystemTenantID, roachpb.SystemTenantID), + // System tenant targeting a secondary tenant. + makeSystemTargetOrFatal(t, roachpb.SystemTenantID, roachpb.MakeTenantID(10)), + } { + systemTarget, err := decodeSystemTarget(testTarget.encode()) + require.NoError(t, err) + require.Equal(t, testTarget, systemTarget) + + // Next, we encode/decode a spanconfig.Target that wraps a SystemTarget. + target := MakeTargetFromSystemTarget(systemTarget) + decodedTarget := DecodeTarget(target.Encode()) + require.Equal(t, target, decodedTarget) + } +} + +// TestDecodeInvalidSpanAsSystemTarget ensures that decoding an invalid span +// as a system target fails. +func TestDecodeInvalidSpanAsSystemTarget(t *testing.T) { + for _, tc := range []struct { + span roachpb.Span + expectedErr string + }{ + { + span: roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("b")}, + expectedErr: "span .* did not conform to SystemTarget encoding", + }, + { + // No end key. + span: roachpb.Span{Key: keys.SystemSpanConfigEntireKeyspace}, + expectedErr: "invalid end key in span", + }, + { + // Invalid end key. + span: roachpb.Span{ + Key: keys.SystemSpanConfigEntireKeyspace, + EndKey: append(keys.SystemSpanConfigEntireKeyspace, byte('a')).PrefixEnd(), + }, + expectedErr: "invalid end key in span", + }, + { + // Sentinel key for SystemSpanConfigEntireKeyspace should not have a + // suffix. + span: roachpb.Span{ + Key: append(keys.SystemSpanConfigEntireKeyspace, byte('a')), + EndKey: append(keys.SystemSpanConfigEntireKeyspace, byte('a')).PrefixEnd(), + }, + expectedErr: "span .* did not conform to SystemTarget encoding", + }, + } { + _, err := decodeSystemTarget(tc.span) + require.Error(t, err) + require.True(t, testutils.IsError(err, tc.expectedErr)) + } +} + +// TestSystemTargetValidation ensures target.validate() works as expected. +func TestSystemTargetValidation(t *testing.T) { + for _, tc := range []struct { + sourceTenantID roachpb.TenantID + targetTenantID roachpb.TenantID + expErr string + }{ + { + // Secondary tenants cannot target the system tenant. + sourceTenantID: roachpb.MakeTenantID(10), + targetTenantID: roachpb.SystemTenantID, + expErr: "secondary tenant 10 cannot target another tenant with ID", + }, + { + // Secondary tenants cannot target other secondary tenants. + sourceTenantID: roachpb.MakeTenantID(10), + targetTenantID: roachpb.MakeTenantID(20), + expErr: "secondary tenant 10 cannot target another tenant with ID", + }, + // Test some valid targets. + { + // System tenant targeting secondary tenant is allowed. + sourceTenantID: roachpb.SystemTenantID, + targetTenantID: roachpb.MakeTenantID(20), + }, + { + // System tenant targeting itself is allowed. + sourceTenantID: roachpb.SystemTenantID, + targetTenantID: roachpb.SystemTenantID, + }, + { + // Secondary tenant targeting itself is allowed. + sourceTenantID: roachpb.MakeTenantID(10), + targetTenantID: roachpb.MakeTenantID(10), + }, + } { + target := SystemTarget{ + SourceTenantID: tc.sourceTenantID, + TargetTenantID: tc.targetTenantID, + } + require.True(t, testutils.IsError(target.validate(), tc.expErr)) + } +} + +// TestTargetSortingRandomized ensures we sort targets correctly. +func TestTargetSortingRandomized(t *testing.T) { + // Construct a set of sorted targets. + sortedTargets := Targets{ + MakeTargetFromSystemTarget(makeSystemTargetOrFatal(t, roachpb.SystemTenantID, roachpb.SystemTenantID)), + MakeTargetFromSystemTarget(makeSystemTargetOrFatal(t, roachpb.SystemTenantID, roachpb.MakeTenantID(10))), + MakeTargetFromSystemTarget(makeSystemTargetOrFatal(t, roachpb.SystemTenantID, roachpb.MakeTenantID(20))), + MakeTargetFromSystemTarget(makeSystemTargetOrFatal(t, roachpb.MakeTenantID(5), roachpb.MakeTenantID(5))), + MakeTargetFromSystemTarget(makeSystemTargetOrFatal(t, roachpb.MakeTenantID(10), roachpb.MakeTenantID(10))), + MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("a"), EndKey: roachpb.Key("b")}), + MakeTargetFromSpan(roachpb.Span{Key: roachpb.Key("y"), EndKey: roachpb.Key("z")}), + } + + const numOps = 20 + for i := 0; i < numOps; i++ { + tc := make(Targets, len(sortedTargets)) + copy(tc, sortedTargets) + + rand.Shuffle(len(tc), func(i, j int) { + tc[i], tc[j] = tc[j], tc[i] + }) + + sort.Sort(tc) + require.Equal(t, sortedTargets, tc) + } +} + +func makeSystemTargetOrFatal( + t *testing.T, sourceID roachpb.TenantID, targetID roachpb.TenantID, +) SystemTarget { + target, err := MakeSystemTarget(sourceID, targetID) + require.NoError(t, err) + return target +} diff --git a/pkg/sql/tenant.go b/pkg/sql/tenant.go index 6e4579f96453..1a14e4df1b5f 100644 --- a/pkg/sql/tenant.go +++ b/pkg/sql/tenant.go @@ -152,7 +152,7 @@ func CreateTenantRecord( tenantPrefix := keys.MakeTenantPrefix(roachpb.MakeTenantID(tenID)) toUpsert := []spanconfig.Record{ { - Target: spanconfig.MakeSpanTarget(roachpb.Span{ + Target: spanconfig.MakeTargetFromSpan(roachpb.Span{ Key: tenantPrefix, EndKey: tenantPrefix.Next(), }), @@ -450,7 +450,7 @@ func GCTenantSync(ctx context.Context, execCfg *ExecutorConfig, info *descpb.Ten scKVAccessor := execCfg.SpanConfigKVAccessor.WithTxn(ctx, txn) records, err := scKVAccessor.GetSpanConfigRecords( - ctx, []spanconfig.Target{spanconfig.MakeSpanTarget(tenantSpan)}, + ctx, []spanconfig.Target{spanconfig.MakeTargetFromSpan(tenantSpan)}, ) if err != nil { return err