Skip to content

Commit 69e48ba

Browse files
ajwernerajwerner
committed
*: fix system config for tenant
Follow-on from #76279. Fixes #75864. This commit adds a mechanism to combine the system config data of the tenant with the data provided over the GossipSubscription from the system tenant. It then plumbs a version into the zone config methods. In the mixed version state, the tenant uses the existing override from the system tenant. After the span config infrastructure has been activated, the tenant uses the overrides they've set. This affects, realistically, just the GC job, and, to a lesser extent, the optimizer. Release note: None
1 parent 5c1ae72 commit 69e48ba

33 files changed

Lines changed: 592 additions & 180 deletions

‎pkg/ccl/backupccl/backup_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1739,7 +1739,7 @@ func TestBackupRestoreControlJob(t *testing.T) {
17391739
if err != nil {
17401740
t.Fatal(err)
17411741
}
1742-
last := config.SystemTenantObjectID(v.ValueInt())
1742+
last := config.ObjectID(v.ValueInt())
17431743
zoneConfig := zonepb.DefaultZoneConfig()
17441744
zoneConfig.RangeMaxBytes = proto.Int64(5000)
17451745
config.TestingSetZoneConfig(last+1, zoneConfig)

‎pkg/ccl/kvccl/kvtenantccl/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ go_test(
5656
"//pkg/config",
5757
"//pkg/gossip",
5858
"//pkg/jobs",
59+
"//pkg/keys",
5960
"//pkg/kv/kvclient/kvtenant",
6061
"//pkg/kv/kvserver",
6162
"//pkg/kv/kvserver/kvserverbase",
@@ -86,6 +87,7 @@ go_test(
8687
"//pkg/util/stop",
8788
"//pkg/util/tracing",
8889
"//pkg/util/uuid",
90+
"@com_github_cockroachdb_errors//:errors",
8991
"@com_github_cockroachdb_redact//:redact",
9092
"@com_github_stretchr_testify//assert",
9193
"@com_github_stretchr_testify//require",

‎pkg/ccl/kvccl/kvtenantccl/tenant_upgrade_test.go‎

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,24 +17,29 @@ import (
1717

1818
"github.com/cockroachdb/cockroach/pkg/base"
1919
"github.com/cockroachdb/cockroach/pkg/clusterversion"
20+
"github.com/cockroachdb/cockroach/pkg/config"
2021
"github.com/cockroachdb/cockroach/pkg/jobs"
22+
"github.com/cockroachdb/cockroach/pkg/keys"
2123
"github.com/cockroachdb/cockroach/pkg/migration"
2224
"github.com/cockroachdb/cockroach/pkg/migration/migrations"
2325
"github.com/cockroachdb/cockroach/pkg/roachpb"
2426
"github.com/cockroachdb/cockroach/pkg/security"
2527
"github.com/cockroachdb/cockroach/pkg/server"
2628
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
2729
"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness/slinstance"
30+
"github.com/cockroachdb/cockroach/pkg/testutils"
31+
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
2832
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
2933
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
3034
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
3135
"github.com/cockroachdb/cockroach/pkg/util/stop"
36+
"github.com/cockroachdb/errors"
3237
"github.com/stretchr/testify/require"
3338
)
3439

3540
// TestTenantUpgrade exercises the case where a system tenant is in a
3641
// non-finalized version state and creates a tenant. The test ensures
37-
// that that newly created tenant begins in that same version.
42+
// that the newly created tenant begins in that same version.
3843
//
3944
// The first subtest creates the tenant in the mixed version state,
4045
// then upgrades the system tenant, then upgrades the secondary tenant,
@@ -358,3 +363,111 @@ func TestTenantUpgradeFailure(t *testing.T) {
358363
tenantInfo.v2onMigrationStopper.Stop(ctx)
359364
})
360365
}
366+
367+
// TestTenantSystemConfigUpgrade ensures that the tenant GC job uses the
368+
// appropriate view of the GC TTL.
369+
func TestTenantSystemConfigUpgrade(t *testing.T) {
370+
defer leaktest.AfterTest(t)()
371+
ctx := context.Background()
372+
settings := cluster.MakeTestingClusterSettingsWithVersions(
373+
clusterversion.TestingBinaryVersion,
374+
clusterversion.TestingBinaryMinSupportedVersion,
375+
false, // initializeVersion
376+
)
377+
// Initialize the version to the BinaryMinSupportedVersion.
378+
require.NoError(t, clusterversion.Initialize(ctx,
379+
clusterversion.TestingBinaryMinSupportedVersion, &settings.SV))
380+
tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{
381+
ServerArgs: base.TestServerArgs{
382+
Settings: settings,
383+
Knobs: base.TestingKnobs{
384+
Server: &server.TestingKnobs{
385+
DisableAutomaticVersionUpgrade: make(chan struct{}),
386+
BinaryVersionOverride: clusterversion.TestingBinaryMinSupportedVersion,
387+
},
388+
},
389+
},
390+
})
391+
hostDB := sqlutils.MakeSQLRunner(tc.ServerConn(0))
392+
hostDB.Exec(t, `SET CLUSTER SETTING kv.closed_timestamp.target_duration = '20ms'`)
393+
hostDB.Exec(t, `SET CLUSTER SETTING kv.closed_timestamp.side_transport_interval = '20ms'`)
394+
defer tc.Stopper().Stop(ctx)
395+
connectToTenant := func(t *testing.T, addr string) (_ *gosql.DB, cleanup func()) {
396+
pgURL, cleanupPGUrl := sqlutils.PGUrl(t, addr, "Tenant", url.User(security.RootUser))
397+
tenantDB, err := gosql.Open("postgres", pgURL.String())
398+
require.NoError(t, err)
399+
return tenantDB, func() {
400+
tenantDB.Close()
401+
cleanupPGUrl()
402+
}
403+
}
404+
mkTenant := func(t *testing.T, id uint64) (
405+
tenant serverutils.TestTenantInterface,
406+
) {
407+
settings := cluster.MakeTestingClusterSettingsWithVersions(
408+
clusterversion.TestingBinaryVersion,
409+
clusterversion.TestingBinaryMinSupportedVersion,
410+
false, // initializeVersion
411+
)
412+
// Initialize the version to the minimum it could be.
413+
require.NoError(t, clusterversion.Initialize(ctx,
414+
clusterversion.TestingBinaryMinSupportedVersion, &settings.SV))
415+
tenantArgs := base.TestTenantArgs{
416+
TenantID: roachpb.MakeTenantID(id),
417+
TestingKnobs: base.TestingKnobs{},
418+
Settings: settings,
419+
}
420+
tenant, err := tc.Server(0).StartTenant(ctx, tenantArgs)
421+
require.NoError(t, err)
422+
return tenant
423+
}
424+
const tenantID = 10
425+
codec := keys.MakeSQLCodec(roachpb.MakeTenantID(tenantID))
426+
tenant := mkTenant(t, tenantID)
427+
tenantSQL, cleanup := connectToTenant(t, tenant.SQLAddr())
428+
defer cleanup()
429+
tenantDB := sqlutils.MakeSQLRunner(tenantSQL)
430+
tenantDB.CheckQueryResults(t, "SHOW CLUSTER SETTING version", [][]string{{"21.2"}})
431+
tenantDB.Exec(t, "CREATE TABLE foo ()")
432+
fooID := sqlutils.QueryTableID(t, tenantSQL, "defaultdb", "public", "foo")
433+
tenantP := tenant.SystemConfigProvider()
434+
ch, _ := tenantP.RegisterSystemConfigChannel()
435+
436+
hostDB.Exec(t, "SET CLUSTER SETTING version = crdb_internal.node_executable_version()")
437+
hostDB.Exec(t, "ALTER RANGE tenants CONFIGURE ZONE USING gc.ttlseconds = 111")
438+
hostDB.Exec(t,
439+
"ALTER TENANT $1 SET CLUSTER SETTING sql.zone_configs.allow_for_secondary_tenant.enabled = true;",
440+
tenantID)
441+
tenantDB.CheckQueryResultsRetry(
442+
t, "SHOW CLUSTER SETTING sql.zone_configs.allow_for_secondary_tenant.enabled",
443+
[][]string{{"true"}},
444+
)
445+
tenantVersion := func() clusterversion.ClusterVersion {
446+
return tenant.ClusterSettings().Version.ActiveVersionOrEmpty(ctx)
447+
}
448+
checkConfigEqual := func(t *testing.T, exp int32) {
449+
testutils.SucceedsSoon(t, func() error {
450+
cfg := tenantP.GetSystemConfig()
451+
if cfg == nil {
452+
return errors.New("no config")
453+
}
454+
conf, err := tenantP.GetSystemConfig().GetZoneConfigForObject(codec, tenantVersion(), config.ObjectID(fooID))
455+
if err != nil {
456+
return err
457+
}
458+
if conf.GC.TTLSeconds != exp {
459+
return errors.Errorf("got %d, expected %d", conf.GC.TTLSeconds, exp)
460+
}
461+
return nil
462+
})
463+
}
464+
checkConfigEqual(t, 111)
465+
<-ch
466+
hostDB.Exec(t, "ALTER RANGE tenants CONFIGURE ZONE USING gc.ttlseconds = 112")
467+
<-ch
468+
checkConfigEqual(t, 112)
469+
tenantDB.Exec(t, "SET CLUSTER SETTING version = crdb_internal.node_executable_version()")
470+
tenantDB.Exec(t, "ALTER TABLE foo CONFIGURE ZONE USING gc.ttlseconds = 113")
471+
<-ch
472+
checkConfigEqual(t, 113)
473+
}

‎pkg/ccl/spanconfigccl/spanconfiglimiterccl/BUILD.bazel‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ go_test(
1515
"//pkg/ccl/utilccl",
1616
"//pkg/config",
1717
"//pkg/config/zonepb",
18-
"//pkg/keys",
1918
"//pkg/roachpb",
2019
"//pkg/security",
2120
"//pkg/security/securitytest",

‎pkg/ccl/spanconfigccl/spanconfiglimiterccl/drop_table_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"github.com/cockroachdb/cockroach/pkg/base"
1818
"github.com/cockroachdb/cockroach/pkg/config"
1919
"github.com/cockroachdb/cockroach/pkg/config/zonepb"
20-
"github.com/cockroachdb/cockroach/pkg/keys"
2120
"github.com/cockroachdb/cockroach/pkg/roachpb"
2221
"github.com/cockroachdb/cockroach/pkg/security"
2322
"github.com/cockroachdb/cockroach/pkg/sql/gcjob"
@@ -57,14 +56,15 @@ func TestDropTableLowersSpanCount(t *testing.T) {
5756
zoneConfig := zonepb.DefaultZoneConfig()
5857
zoneConfig.GC.TTLSeconds = 1
5958
config.TestingSetupZoneConfigHook(tc.Stopper())
60-
// TODO(irfansharif): Work around for #75864.
61-
config.TestingSetZoneConfig(keys.TenantsRangesID, zoneConfig)
6259

6360
require.NoError(t, err)
6461
defer func() { require.NoError(t, tenantSQLDB.Close()) }()
6562

6663
tenantDB := sqlutils.MakeSQLRunner(tenantSQLDB)
64+
6765
tenantDB.Exec(t, `CREATE TABLE t(k INT PRIMARY KEY)`)
66+
id := sqlutils.QueryTableID(t, tenantSQLDB, "defaultdb", "public", "t")
67+
config.TestingSetZoneConfig(config.ObjectID(id), zoneConfig)
6868

6969
var spanCount int
7070
tenantDB.QueryRow(t, `SELECT span_count FROM system.span_count LIMIT 1`).Scan(&spanCount)

‎pkg/config/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ go_library(
1515
importpath = "github.com/cockroachdb/cockroach/pkg/config",
1616
visibility = ["//visibility:public"],
1717
deps = [
18+
"//pkg/clusterversion",
1819
"//pkg/config/zonepb",
1920
"//pkg/keys",
2021
"//pkg/roachpb",

‎pkg/config/keys.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@ func MakeZoneKey(codec keys.SQLCodec, id descpb.ID) roachpb.Key {
2727
return codec.ZoneKey(uint32(id))
2828
}
2929

30-
// DecodeSystemTenantObjectID decodes the object ID for the system-tenant from
30+
// DecodeObjectID decodes the object ID for the system-tenant from
3131
// the front of key. It returns the decoded object ID, the remainder of the key,
3232
// and whether the result is valid (i.e., whether the key was within the system
3333
// tenant's structured key space).
34-
func DecodeSystemTenantObjectID(key roachpb.RKey) (SystemTenantObjectID, []byte, bool) {
35-
rem, id, err := keys.SystemSQLCodec.DecodeTablePrefix(key.AsRawKey())
36-
return SystemTenantObjectID(id), rem, err == nil
34+
func DecodeObjectID(codec keys.SQLCodec, key roachpb.RKey) (ObjectID, []byte, bool) {
35+
rem, id, err := codec.DecodeTablePrefix(key.AsRawKey())
36+
return ObjectID(id), rem, err == nil
3737
}

‎pkg/config/keys_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func TestDecodeSystemTenantObjectID(t *testing.T) {
2828
key roachpb.RKey
2929
keySuffix []byte
3030
success bool
31-
id config.SystemTenantObjectID
31+
id config.ObjectID
3232
}{
3333
// Before the structured span.
3434
{roachpb.RKeyMin, nil, false, 0},
@@ -43,7 +43,7 @@ func TestDecodeSystemTenantObjectID(t *testing.T) {
4343
}
4444

4545
for tcNum, tc := range testCases {
46-
id, keySuffix, success := config.DecodeSystemTenantObjectID(tc.key)
46+
id, keySuffix, success := config.DecodeObjectID(keys.SystemSQLCodec, tc.key)
4747
if success != tc.success {
4848
t.Errorf("#%d: expected success=%t", tcNum, tc.success)
4949
continue

0 commit comments

Comments
 (0)