diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index e3097ef95ac3..420e28042054 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -193,4 +193,4 @@ trace.jaeger.agent string the address of a Jaeger agent to receive traces using trace.opentelemetry.collector string address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as :. If no port is specified, 4317 will be used. trace.span_registry.enabled boolean true if set, ongoing traces can be seen at https:///#/debug/tracez trace.zipkin.collector string the address of a Zipkin instance to receive traces, as :. If no port is specified, 9411 will be used. -version version 21.2-104 set the active cluster version in the format '.' +version version 21.2-110 set the active cluster version in the format '.' diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index d51d862aa25a..5b7578f36cc8 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -209,6 +209,6 @@ trace.opentelemetry.collectorstringaddress of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as :. If no port is specified, 4317 will be used. trace.span_registry.enabledbooleantrueif set, ongoing traces can be seen at https:///#/debug/tracez trace.zipkin.collectorstringthe address of a Zipkin instance to receive traces, as :. If no port is specified, 9411 will be used. -versionversion21.2-104set the active cluster version in the format '.' +versionversion21.2-110set the active cluster version in the format '.' diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index fda6ded3a19f..4e5b3a131c04 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -37,6 +37,7 @@ ALL_TESTS = [ "//pkg/ccl/serverccl/statusccl:statusccl_test", "//pkg/ccl/serverccl:serverccl_test", "//pkg/ccl/spanconfigccl/spanconfigcomparedccl:spanconfigcomparedccl_test", + "//pkg/ccl/spanconfigccl/spanconfiglimiterccl:spanconfiglimiterccl_test", "//pkg/ccl/spanconfigccl/spanconfigreconcilerccl:spanconfigreconcilerccl_test", "//pkg/ccl/spanconfigccl/spanconfigsplitterccl:spanconfigsplitterccl_test", "//pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl:spanconfigsqltranslatorccl_test", diff --git a/pkg/ccl/backupccl/system_schema.go b/pkg/ccl/backupccl/system_schema.go index d013bb4bc14d..639c42b923d1 100644 --- a/pkg/ccl/backupccl/system_schema.go +++ b/pkg/ccl/backupccl/system_schema.go @@ -361,6 +361,9 @@ var systemTableBackupConfiguration = map[string]systemBackupConfiguration{ systemschema.TenantSettingsTable.GetName(): { shouldIncludeInClusterBackup: optInToClusterBackup, }, + systemschema.SpanCountTable.GetName(): { + shouldIncludeInClusterBackup: optOutOfClusterBackup, + }, } // GetSystemTablesToIncludeInClusterBackup returns a set of system table names that diff --git a/pkg/ccl/migrationccl/migrationsccl/BUILD.bazel b/pkg/ccl/migrationccl/migrationsccl/BUILD.bazel index 0b721fbb3ed1..eeadabe3b3eb 100644 --- a/pkg/ccl/migrationccl/migrationsccl/BUILD.bazel +++ b/pkg/ccl/migrationccl/migrationsccl/BUILD.bazel @@ -4,6 +4,7 @@ go_test( name = "migrationsccl_test", srcs = [ "main_test.go", + "seed_span_counts_external_test.go", "seed_tenant_span_configs_external_test.go", ], deps = [ @@ -15,7 +16,9 @@ go_test( "//pkg/security", "//pkg/security/securitytest", "//pkg/server", + "//pkg/settings/cluster", "//pkg/spanconfig", + "//pkg/testutils", "//pkg/testutils/serverutils", "//pkg/testutils/sqlutils", "//pkg/testutils/testcluster", diff --git a/pkg/ccl/migrationccl/migrationsccl/seed_span_counts_external_test.go b/pkg/ccl/migrationccl/migrationsccl/seed_span_counts_external_test.go new file mode 100644 index 000000000000..7e6bea82140d --- /dev/null +++ b/pkg/ccl/migrationccl/migrationsccl/seed_span_counts_external_test.go @@ -0,0 +1,252 @@ +// Copyright 2022 The Cockroach Authors. +// +// Licensed as a CockroachDB Enterprise file under the Cockroach Community +// License (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt + +package migrationsccl_test + +import ( + "context" + gosql "database/sql" + "net/url" + "testing" + + "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/clusterversion" + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/security" + "github.com/cockroachdb/cockroach/pkg/server" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/spanconfig" + "github.com/cockroachdb/cockroach/pkg/testutils" + "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" + "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/stretchr/testify/require" +) + +// TestPreSeedSpanCountTable tests that incremental schema changes after +// PreSeedSpanCountTable is enabled get tracked as such. It also tests that once +// SeedSpanCountTable is reached, the span count is updated to capture the most +// up-to-date view of all schema objects. Specifically, we're not +// double-counting the incremental update we tracked in the +// PreSeedSpanCountTable state. +func TestPreSeedSpanCountTable(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + var ( + v0 = clusterversion.ByKey(clusterversion.SpanCountTable) + v1 = clusterversion.ByKey(clusterversion.PreSeedSpanCountTable) + v2 = clusterversion.ByKey(clusterversion.SeedSpanCountTable) + ) + + ctx := context.Background() + settings := cluster.MakeTestingClusterSettingsWithVersions(v2, v0, false /* initializeVersion */) + require.NoError(t, clusterversion.Initialize(ctx, v0, &settings.SV)) + + tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + Settings: settings, + Knobs: base.TestingKnobs{ + Server: &server.TestingKnobs{ + DisableAutomaticVersionUpgrade: make(chan struct{}), + BinaryVersionOverride: v0, + }, + }, + }, + }) + + defer tc.Stopper().Stop(ctx) + ts := tc.Server(0) + hostDB := sqlutils.MakeSQLRunner(tc.ServerConn(0)) + + tenantID := roachpb.MakeTenantID(10) + tenantSettings := cluster.MakeTestingClusterSettingsWithVersions(v2, v0, false /* initializeVersion */) + require.NoError(t, clusterversion.Initialize(ctx, v0, &tenantSettings.SV)) + tenant, err := ts.StartTenant(ctx, base.TestTenantArgs{ + TenantID: tenantID, + TestingKnobs: base.TestingKnobs{}, + Settings: tenantSettings, + }) + require.NoError(t, err) + + pgURL, cleanupPGUrl := sqlutils.PGUrl(t, tenant.SQLAddr(), "Tenant", url.User(security.RootUser)) + defer cleanupPGUrl() + + tenantSQLDB, err := gosql.Open("postgres", pgURL.String()) + require.NoError(t, err) + defer func() { require.NoError(t, tenantSQLDB.Close()) }() + + // Upgrade the host cluster all the way. + hostDB.Exec(t, "SET CLUSTER SETTING version = $1", v2.String()) + + var spanCount, numRows int + tenantDB := sqlutils.MakeSQLRunner(tenantSQLDB) + + tenantDB.CheckQueryResults(t, "SHOW CLUSTER SETTING version", [][]string{{v0.String()}}) + tenantDB.Exec(t, `CREATE TABLE t(k INT PRIMARY KEY)`) + tenantDB.QueryRow(t, `SELECT count(*) FROM system.span_count`).Scan(&numRows) + require.Equal(t, 0, numRows) + + tenantDB.Exec(t, "SET CLUSTER SETTING version = $1", v1.String()) + tenantDB.CheckQueryResults(t, "SHOW CLUSTER SETTING version", [][]string{{v1.String()}}) + tenantDB.Exec(t, `CREATE INDEX idx ON t (k)`) + tenantDB.QueryRow(t, `SELECT span_count FROM system.span_count LIMIT 1`).Scan(&spanCount) + require.Equal(t, 2, spanCount) + + tenantDB.Exec(t, "SET CLUSTER SETTING version = $1", v2.String()) + tenantDB.CheckQueryResults(t, "SHOW CLUSTER SETTING version", [][]string{{v2.String()}}) + tenantDB.QueryRow(t, `SELECT span_count FROM system.span_count LIMIT 1`).Scan(&spanCount) + require.Equal(t, 5, spanCount) +} + +// TestSeedSpanCountTable tests that the migration seeds system.span_count +// correctly for secondary tenants. +func TestSeedSpanCountTable(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + var ( + v0 = clusterversion.ByKey(clusterversion.SpanCountTable) + v2 = clusterversion.ByKey(clusterversion.SeedSpanCountTable) + ) + + ctx := context.Background() + settings := cluster.MakeTestingClusterSettingsWithVersions(v2, v0, false /* initializeVersion */) + require.NoError(t, clusterversion.Initialize(ctx, v0, &settings.SV)) + + tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + Settings: settings, + Knobs: base.TestingKnobs{ + Server: &server.TestingKnobs{ + DisableAutomaticVersionUpgrade: make(chan struct{}), + BinaryVersionOverride: v0, + }, + }, + }, + }) + + defer tc.Stopper().Stop(ctx) + ts := tc.Server(0) + hostDB := sqlutils.MakeSQLRunner(tc.ServerConn(0)) + + tenantID := roachpb.MakeTenantID(10) + tenantSettings := cluster.MakeTestingClusterSettingsWithVersions(v2, v0, false /* initializeVersion */) + require.NoError(t, clusterversion.Initialize(ctx, v0, &tenantSettings.SV)) + tenant, err := ts.StartTenant(ctx, base.TestTenantArgs{ + TenantID: tenantID, + TestingKnobs: base.TestingKnobs{}, + Settings: tenantSettings, + }) + require.NoError(t, err) + + pgURL, cleanupPGUrl := sqlutils.PGUrl(t, tenant.SQLAddr(), "Tenant", url.User(security.RootUser)) + defer cleanupPGUrl() + + tenantSQLDB, err := gosql.Open("postgres", pgURL.String()) + require.NoError(t, err) + defer func() { require.NoError(t, tenantSQLDB.Close()) }() + + tenantDB := sqlutils.MakeSQLRunner(tenantSQLDB) + tenantDB.CheckQueryResults(t, "SHOW CLUSTER SETTING version", [][]string{{v0.String()}}) + + // Upgrade the host cluster. + hostDB.Exec(t, "SET CLUSTER SETTING version = $1", v2.String()) + tenantDB.CheckQueryResults(t, "SHOW CLUSTER SETTING version", [][]string{{v0.String()}}) + + tenantDB.Exec(t, `CREATE TABLE t(k INT PRIMARY KEY)`) + + var spanCount, numRows int + tenantDB.QueryRow(t, `SELECT count(*) FROM system.span_count`).Scan(&numRows) + require.Equal(t, 0, numRows) + + tenantDB.Exec(t, "SET CLUSTER SETTING version = $1", v2.String()) + tenantDB.QueryRow(t, `SELECT span_count FROM system.span_count LIMIT 1`).Scan(&spanCount) + require.Equal(t, 3, spanCount) +} + +// TestSeedSpanCountTableOverLimit tests that the migration seeds +// system.span_count correctly for secondary tenants, even if over the +// proscribed limit. In these cases the tenant goes into debt -- all subsequent +// schema changes that add schema elements will be rejected. Attempts to free up +// spans however will be accepted. +func TestSeedSpanCountTableOverLimit(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + var ( + v0 = clusterversion.ByKey(clusterversion.SpanCountTable) + v2 = clusterversion.ByKey(clusterversion.SeedSpanCountTable) + ) + + ctx := context.Background() + settings := cluster.MakeTestingClusterSettingsWithVersions(v2, v0, false /* initializeVersion */) + require.NoError(t, clusterversion.Initialize(ctx, v0, &settings.SV)) + + tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + Settings: settings, + Knobs: base.TestingKnobs{ + Server: &server.TestingKnobs{ + DisableAutomaticVersionUpgrade: make(chan struct{}), + BinaryVersionOverride: v0, + }, + }, + }, + }) + + defer tc.Stopper().Stop(ctx) + ts := tc.Server(0) + hostDB := sqlutils.MakeSQLRunner(tc.ServerConn(0)) + + const limit = 1 + tenantID := roachpb.MakeTenantID(10) + tenantSettings := cluster.MakeTestingClusterSettingsWithVersions(v2, v0, false /* initializeVersion */) + require.NoError(t, clusterversion.Initialize(ctx, v0, &tenantSettings.SV)) + tenant, err := ts.StartTenant(ctx, base.TestTenantArgs{ + TenantID: tenantID, + TestingKnobs: base.TestingKnobs{ + SpanConfig: &spanconfig.TestingKnobs{ + ExcludeDroppedDescriptorsFromLookup: true, + LimiterLimitOverride: func() int64 { + return limit + }, + }, + }, + Settings: tenantSettings, + }) + require.NoError(t, err) + + pgURL, cleanupPGUrl := sqlutils.PGUrl(t, tenant.SQLAddr(), "Tenant", url.User(security.RootUser)) + defer cleanupPGUrl() + + tenantSQLDB, err := gosql.Open("postgres", pgURL.String()) + require.NoError(t, err) + defer func() { require.NoError(t, tenantSQLDB.Close()) }() + + // Upgrade the host cluster. + hostDB.Exec(t, "SET CLUSTER SETTING version = $1", v2.String()) + + tenantDB := sqlutils.MakeSQLRunner(tenantSQLDB) + tenantDB.Exec(t, `CREATE TABLE t1(k INT PRIMARY KEY)`) + tenantDB.Exec(t, `CREATE TABLE t2(k INT PRIMARY KEY)`) + tenantDB.Exec(t, `CREATE TABLE t3(k INT PRIMARY KEY)`) + + var spanCount int + tenantDB.Exec(t, "SET CLUSTER SETTING version = $1", v2.String()) + tenantDB.QueryRow(t, `SELECT span_count FROM system.span_count LIMIT 1`).Scan(&spanCount) + require.Equal(t, 9, spanCount) + + _, err = tenantDB.DB.ExecContext(ctx, `CREATE TABLE t4(k INT PRIMARY KEY)`) + require.True(t, testutils.IsError(err, "exceeded limit for number of table spans")) + + tenantDB.Exec(t, `DROP TABLE t3`) + tenantDB.QueryRow(t, `SELECT span_count FROM system.span_count LIMIT 1`).Scan(&spanCount) + require.Equal(t, 6, spanCount) +} diff --git a/pkg/ccl/spanconfigccl/spanconfigcomparedccl/testdata/multitenant b/pkg/ccl/spanconfigccl/spanconfigcomparedccl/testdata/multitenant index 9c8f237b7874..b3d79b2f7265 100644 --- a/pkg/ccl/spanconfigccl/spanconfigcomparedccl/testdata/multitenant +++ b/pkg/ccl/spanconfigccl/spanconfigcomparedccl/testdata/multitenant @@ -117,6 +117,7 @@ diff offset=48 +/Tenant/11/Table/43 database system (tenant) +/Tenant/11/Table/44 database system (tenant) +/Tenant/11/Table/46 database system (tenant) ++/Tenant/11/Table/50 database system (tenant) +/Tenant/11/Table/106 ttl_seconds=1000 num_replicas=42 +/Tenant/11/Table/107 range default diff --git a/pkg/ccl/spanconfigccl/spanconfiglimiterccl/BUILD.bazel b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/BUILD.bazel new file mode 100644 index 000000000000..128318644767 --- /dev/null +++ b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/BUILD.bazel @@ -0,0 +1,37 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_test") + +go_test( + name = "spanconfiglimiterccl_test", + srcs = [ + "datadriven_test.go", + "drop_table_test.go", + "main_test.go", + ], + data = glob(["testdata/**"]), + deps = [ + "//pkg/base", + "//pkg/ccl/kvccl/kvtenantccl", + "//pkg/ccl/partitionccl", + "//pkg/ccl/utilccl", + "//pkg/config", + "//pkg/config/zonepb", + "//pkg/keys", + "//pkg/roachpb", + "//pkg/security", + "//pkg/security/securitytest", + "//pkg/server", + "//pkg/spanconfig", + "//pkg/spanconfig/spanconfigtestutils/spanconfigtestcluster", + "//pkg/sql/gcjob", + "//pkg/testutils", + "//pkg/testutils/serverutils", + "//pkg/testutils/sqlutils", + "//pkg/testutils/testcluster", + "//pkg/util/leaktest", + "//pkg/util/log", + "//pkg/util/randutil", + "@com_github_cockroachdb_datadriven//:datadriven", + "@com_github_cockroachdb_errors//:errors", + "@com_github_stretchr_testify//require", + ], +) diff --git a/pkg/ccl/spanconfigccl/spanconfiglimiterccl/datadriven_test.go b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/datadriven_test.go new file mode 100644 index 000000000000..44d2922665ca --- /dev/null +++ b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/datadriven_test.go @@ -0,0 +1,147 @@ +// Copyright 2022 The Cockroach Authors. +// +// Licensed as a CockroachDB Enterprise file under the Cockroach Community +// License (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt + +package spanconfiglimiterccl + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/cockroachdb/cockroach/pkg/base" + _ "github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvtenantccl" + _ "github.com/cockroachdb/cockroach/pkg/ccl/partitionccl" + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/spanconfig" + "github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigtestutils/spanconfigtestcluster" + "github.com/cockroachdb/cockroach/pkg/testutils" + "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" + "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/datadriven" + "github.com/cockroachdb/errors" + "github.com/stretchr/testify/require" +) + +// TestDataDriven is a data-driven test for spanconfig.Limiter. It offers the +// following commands: +// +// - "initialize" tenant= +// Initialize a secondary tenant with the given ID. +// +// - "exec-sql" [tenant=] +// Executes the input SQL query for the given tenant. All statements are +// executed in a single transaction. +// +// - "query-sql" [tenant=] [retry] +// Executes the input SQL query for the given tenant and print the results. +// If retry is specified and the expected results do not match the actual +// results, the query will be retried under a testutils.SucceedsSoon block. +// If run with -rewrite, we insert a 500ms sleep before executing the query +// once. +// +// - override limit= +// Override the span limit each tenant is configured with. +// +func TestDataDriven(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + ctx := context.Background() + datadriven.Walk(t, testutils.TestDataPath(t), func(t *testing.T, path string) { + // TODO(irfansharif): This is a stop-gap for tenant read-only cluster + // settings. See https://github.com/cockroachdb/cockroach/pull/76929. Once + // that's done, this test should be updated to use: + // SET CLUSTER SETTING spanconfig.tenant_limit = + limitOverride := 50 + scKnobs := &spanconfig.TestingKnobs{ + // Instead of relying on the GC job to wait out TTLs and clear out + // descriptors, let's simply exclude dropped tables to simulate + // descriptors no longer existing. + ExcludeDroppedDescriptorsFromLookup: true, + LimiterLimitOverride: func() int64 { + return int64(limitOverride) + }, + } + tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + Knobs: base.TestingKnobs{ + SpanConfig: scKnobs, + }, + }, + }) + defer tc.Stopper().Stop(ctx) + { + tdb := sqlutils.MakeSQLRunner(tc.ServerConn(0)) + tdb.Exec(t, `SET CLUSTER SETTING kv.closed_timestamp.target_duration = '20ms'`) + } + + spanConfigTestCluster := spanconfigtestcluster.NewHandle(t, tc, scKnobs) + defer spanConfigTestCluster.Cleanup() + + spanConfigTestCluster.InitializeTenant(ctx, roachpb.SystemTenantID) + datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string { + tenantID := roachpb.SystemTenantID + if d.HasArg("tenant") { + var id uint64 + d.ScanArgs(t, "tenant", &id) + tenantID = roachpb.MakeTenantID(id) + } + + tenant, found := spanConfigTestCluster.LookupTenant(tenantID) + if d.Cmd != "initialize" { + require.Truef(t, found, "tenant %s not found (was it initialized?)", tenantID) + } + + switch d.Cmd { + case "initialize": + spanConfigTestCluster.InitializeTenant(ctx, tenantID) + + case "exec-sql": + if err := tenant.ExecWithErr(d.Input); err != nil { + return fmt.Sprintf("err: %s", err) + } + + case "query-sql": + query := func() string { + rows := tenant.Query(d.Input) + output, err := sqlutils.RowsToDataDrivenOutput(rows) + require.NoError(t, err) + return output + } + if !d.HasArg("retry") { + return query() + } + + if d.Rewrite { + time.Sleep(500 * time.Millisecond) + return query() + } + + var output string + testutils.SucceedsSoon(t, func() error { + if output = query(); output != d.Expected { + return errors.Newf("expected %q, got %q; retrying..", d.Expected, output) + } + return nil + }) + return output + + case "override": + d.ScanArgs(t, "limit", &limitOverride) + + default: + t.Fatalf("unknown command: %s", d.Cmd) + } + + return "" + }) + }) +} diff --git a/pkg/ccl/spanconfigccl/spanconfiglimiterccl/drop_table_test.go b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/drop_table_test.go new file mode 100644 index 000000000000..172c73213ad0 --- /dev/null +++ b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/drop_table_test.go @@ -0,0 +1,82 @@ +// Copyright 2022 The Cockroach Authors. +// +// Licensed as a CockroachDB Enterprise file under the Cockroach Community +// License (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt + +package spanconfiglimiterccl + +import ( + "context" + gosql "database/sql" + "net/url" + "testing" + + "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/config" + "github.com/cockroachdb/cockroach/pkg/config/zonepb" + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/security" + "github.com/cockroachdb/cockroach/pkg/sql/gcjob" + "github.com/cockroachdb/cockroach/pkg/testutils" + "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" + "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/errors" + "github.com/stretchr/testify/require" +) + +func TestDropTableLowersSpanCount(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + defer gcjob.SetSmallMaxGCIntervalForTest()() + + ctx := context.Background() + tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{}) + + defer tc.Stopper().Stop(ctx) + ts := tc.Server(0) + + tenantID := roachpb.MakeTenantID(10) + tenant, err := ts.StartTenant(ctx, base.TestTenantArgs{ + TenantID: tenantID, + TestingKnobs: base.TestingKnobs{}, + }) + require.NoError(t, err) + + pgURL, cleanupPGUrl := sqlutils.PGUrl(t, tenant.SQLAddr(), "Tenant", url.User(security.RootUser)) + defer cleanupPGUrl() + + tenantSQLDB, err := gosql.Open("postgres", pgURL.String()) + + zoneConfig := zonepb.DefaultZoneConfig() + zoneConfig.GC.TTLSeconds = 1 + config.TestingSetupZoneConfigHook(tc.Stopper()) + // TODO(irfansharif): Work around for #75864. + config.TestingSetZoneConfig(keys.TenantsRangesID, zoneConfig) + + require.NoError(t, err) + defer func() { require.NoError(t, tenantSQLDB.Close()) }() + + tenantDB := sqlutils.MakeSQLRunner(tenantSQLDB) + tenantDB.Exec(t, `CREATE TABLE t(k INT PRIMARY KEY)`) + + var spanCount int + tenantDB.QueryRow(t, `SELECT span_count FROM system.span_count LIMIT 1`).Scan(&spanCount) + require.Equal(t, 3, spanCount) + + tenantDB.Exec(t, `DROP TABLE t`) + + testutils.SucceedsSoon(t, func() error { + tenantDB.QueryRow(t, `SELECT span_count FROM system.span_count LIMIT 1`).Scan(&spanCount) + if spanCount != 0 { + return errors.Newf("expected zero span count, found %d", spanCount) + } + return nil + }) +} diff --git a/pkg/ccl/spanconfigccl/spanconfiglimiterccl/main_test.go b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/main_test.go new file mode 100644 index 000000000000..da37b69224ff --- /dev/null +++ b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/main_test.go @@ -0,0 +1,33 @@ +// Copyright 2022 The Cockroach Authors. +// +// Licensed as a CockroachDB Enterprise file under the Cockroach Community +// License (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt + +package spanconfiglimiterccl + +import ( + "os" + "testing" + + "github.com/cockroachdb/cockroach/pkg/ccl/utilccl" + "github.com/cockroachdb/cockroach/pkg/security" + "github.com/cockroachdb/cockroach/pkg/security/securitytest" + "github.com/cockroachdb/cockroach/pkg/server" + "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" + "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" + "github.com/cockroachdb/cockroach/pkg/util/randutil" +) + +//go:generate ../../../util/leaktest/add-leaktest.sh *_test.go + +func TestMain(m *testing.M) { + defer utilccl.TestingEnableEnterprise()() + security.SetAssetLoader(securitytest.EmbeddedAssets) + randutil.SeedForTests() + serverutils.InitTestServerFactory(server.TestServerFactory) + serverutils.InitTestClusterFactory(testcluster.TestClusterFactory) + os.Exit(m.Run()) +} diff --git a/pkg/ccl/spanconfigccl/spanconfiglimiterccl/testdata/indexes b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/testdata/indexes new file mode 100644 index 000000000000..9ed6a061b0db --- /dev/null +++ b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/testdata/indexes @@ -0,0 +1,56 @@ +# Ensure that system.span_count is maintained appropriately when creating and +# dropping secondary indexes, and then dropping the table entirely. + +initialize tenant=10 +---- + +query-sql tenant=10 +SELECT count(*) FROM system.span_count; +---- +0 + +exec-sql tenant=10 +CREATE DATABASE db; +CREATE TABLE db.t(i INT PRIMARY KEY, j INT); +---- + +query-sql tenant=10 +SELECT span_count FROM system.span_count; +---- +3 + +exec-sql tenant=10 +CREATE INDEX idx2 ON db.t (j); +---- + +query-sql tenant=10 +SELECT span_count FROM system.span_count; +---- +5 + +exec-sql tenant=10 +DROP INDEX db.t@idx2; +---- + +query-sql tenant=10 +SELECT span_count FROM system.span_count; +---- +3 + +exec-sql tenant=10 +CREATE INDEX idx4 ON db.t (j); +---- + +query-sql tenant=10 +SELECT span_count FROM system.span_count; +---- +5 + +exec-sql tenant=10 +DROP TABLE db.t; +---- + +query-sql tenant=10 +SELECT span_count FROM system.span_count; +---- +0 diff --git a/pkg/ccl/spanconfigccl/spanconfiglimiterccl/testdata/limit b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/testdata/limit new file mode 100644 index 000000000000..a4aff9c617eb --- /dev/null +++ b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/testdata/limit @@ -0,0 +1,33 @@ +# Ensure that we respect tenant span config limits, rejecting schema change +# operations that take us past it. + +initialize tenant=10 +---- + +exec-sql tenant=10 +CREATE DATABASE db; +CREATE TABLE db.t1(i INT PRIMARY KEY); +---- + +query-sql tenant=10 +SELECT span_count FROM system.span_count; +---- +3 + +override limit=3 +---- + +exec-sql tenant=10 +CREATE TABLE db.t2(i INT PRIMARY KEY); +---- +err: pq: exceeded limit for number of table spans + +query-sql tenant=10 +SELECT span_count FROM system.span_count; +---- +3 + +query-sql tenant=10 +SELECT table_name FROM [SHOW TABLES FROM db]; +---- +t1 diff --git a/pkg/ccl/spanconfigccl/spanconfiglimiterccl/testdata/tables b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/testdata/tables new file mode 100644 index 000000000000..8d17e44f239c --- /dev/null +++ b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/testdata/tables @@ -0,0 +1,55 @@ +# Ensure that system.span_count is maintained appropriately when creating and +# dropping tables. + +initialize tenant=10 +---- + +query-sql tenant=10 +SELECT count(*) FROM system.span_count; +---- +0 + +exec-sql tenant=10 +CREATE DATABASE db; +CREATE TABLE db.t1(i INT PRIMARY KEY); +---- + +query-sql tenant=10 +SELECT span_count FROM system.span_count; +---- +3 + +exec-sql tenant=10 +CREATE TABLE db.t2(i INT PRIMARY KEY); +CREATE TABLE db.t3(i INT PRIMARY KEY); +CREATE TABLE db.t4(i INT PRIMARY KEY); +CREATE TABLE db.t5(i INT PRIMARY KEY); +---- + +query-sql tenant=10 +SELECT span_count FROM system.span_count; +---- +15 + +exec-sql tenant=10 +DROP TABLE db.t1; +---- + +exec-sql tenant=10 +DROP TABLE db.t2; +DROP TABLE db.t3; +---- + +query-sql tenant=10 +SELECT span_count FROM system.span_count; +---- +6 + +exec-sql tenant=10 +DROP DATABASE db CASCADE; +---- + +query-sql tenant=10 +SELECT span_count FROM system.span_count; +---- +0 diff --git a/pkg/ccl/spanconfigccl/spanconfiglimiterccl/testdata/unlimited b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/testdata/unlimited new file mode 100644 index 000000000000..309ff26a3121 --- /dev/null +++ b/pkg/ccl/spanconfigccl/spanconfiglimiterccl/testdata/unlimited @@ -0,0 +1,19 @@ +# Ensure that the system tenant entirely ignores span config limits. + +exec-sql +CREATE DATABASE db; +CREATE TABLE db.t1(i INT PRIMARY KEY); +---- + +override limit=3 +---- + +exec-sql +CREATE TABLE db.t2(i INT PRIMARY KEY); +---- + +query-sql +SELECT table_name FROM [SHOW TABLES FROM db]; +---- +t1 +t2 diff --git a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/multitenant/basic b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/multitenant/basic index 40d4562c566d..2369e373f5e1 100644 --- a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/multitenant/basic +++ b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/multitenant/basic @@ -67,6 +67,7 @@ upsert /Tenant/10/Table/4{2-3} database system (tenant) upsert /Tenant/10/Table/4{3-4} database system (tenant) upsert /Tenant/10/Table/4{4-5} database system (tenant) upsert /Tenant/10/Table/4{6-7} database system (tenant) +upsert /Tenant/10/Table/5{0-1} database system (tenant) state offset=47 ---- @@ -106,6 +107,7 @@ state offset=47 /Tenant/10/Table/4{3-4} database system (tenant) /Tenant/10/Table/4{4-5} database system (tenant) /Tenant/10/Table/4{6-7} database system (tenant) +/Tenant/10/Table/5{0-1} database system (tenant) /Tenant/11{-"\x00"} database system (tenant) exec-sql tenant=10 @@ -130,6 +132,7 @@ state offset=81 ---- ... /Tenant/10/Table/4{6-7} database system (tenant) +/Tenant/10/Table/5{0-1} database system (tenant) /Tenant/10/Table/10{6-7} range default /Tenant/10/Table/10{7-8} range default /Tenant/10/Table/11{2-3} range default diff --git a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/multitenant/protectedts b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/multitenant/protectedts index f92e008367b1..9105b88efa33 100644 --- a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/multitenant/protectedts +++ b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/multitenant/protectedts @@ -83,6 +83,7 @@ upsert /Tenant/10/Table/4{2-3} database system (tenant) upsert /Tenant/10/Table/4{3-4} database system (tenant) upsert /Tenant/10/Table/4{4-5} database system (tenant) upsert /Tenant/10/Table/4{6-7} database system (tenant) +upsert /Tenant/10/Table/5{0-1} database system (tenant) exec-sql tenant=10 CREATE DATABASE db; diff --git a/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/tenant/full_translate b/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/tenant/full_translate index efcaed411bee..cb2c73c90e76 100644 --- a/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/tenant/full_translate +++ b/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/tenant/full_translate @@ -51,6 +51,7 @@ full-translate /Tenant/10/Table/4{3-4} database system (tenant) /Tenant/10/Table/4{4-5} database system (tenant) /Tenant/10/Table/4{6-7} database system (tenant) +/Tenant/10/Table/5{0-1} database system (tenant) /Tenant/10/Table/11{0-1} range default /Tenant/10/Table/11{1-2} range default /Tenant/10/Table/11{2-3} range default @@ -92,6 +93,7 @@ translate named-zone=default /Tenant/10/Table/4{3-4} database system (tenant) /Tenant/10/Table/4{4-5} database system (tenant) /Tenant/10/Table/4{6-7} database system (tenant) +/Tenant/10/Table/5{0-1} database system (tenant) /Tenant/10/Table/11{0-1} range default /Tenant/10/Table/11{1-2} range default /Tenant/10/Table/11{2-3} range default diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go index 146706485bb2..76e7cdfe3f7e 100644 --- a/pkg/clusterversion/cockroach_versions.go +++ b/pkg/clusterversion/cockroach_versions.go @@ -313,11 +313,9 @@ const ( // PebbleFormatSplitUserKeysMarked performs a Pebble-level migration and // upgrades the Pebble format major version to FormatSplitUserKeysMarked. PebbleFormatSplitUserKeysMarked - // IncrementalBackupSubdir enables backing up new incremental backups to a // dedicated subdirectory, to make it easier to apply a different ttl. IncrementalBackupSubdir - // DateStyleIntervalStyleCastRewrite rewrites cast that cause inconsistencies // when DateStyle/IntervalStyle is enabled. DateStyleIntervalStyleCastRewrite @@ -327,7 +325,6 @@ const ( // ClusterLocksVirtualTable enables querying the crdb_internal.cluster_locks // virtual table, which sends a QueryLocksRequest RPC to all cluster ranges. ClusterLocksVirtualTable - // AutoStatsTableSettings is the version where we allow auto stats related // table settings. AutoStatsTableSettings @@ -335,6 +332,15 @@ const ( ForecastStats // SuperRegions enables the usage on super regions. SuperRegions + // SpanCountTable adds system.span_count to track the number of committed + // tenant spans. + SpanCountTable + // PreSeedSpanCountTable precedes PreSeedSpanCountTable, it enables span + // accounting for incremental schema changes. + PreSeedSpanCountTable + // SeedSpanCountTable seeds system.span_count with the number of committed + // tenant spans. + SeedSpanCountTable // ************************************************* // Step (1): Add new versions here. @@ -570,6 +576,18 @@ var versionsSingleton = keyedVersions{ Key: SuperRegions, Version: roachpb.Version{Major: 21, Minor: 2, Internal: 104}, }, + { + Key: SpanCountTable, + Version: roachpb.Version{Major: 21, Minor: 2, Internal: 106}, + }, + { + Key: PreSeedSpanCountTable, + Version: roachpb.Version{Major: 21, Minor: 2, Internal: 108}, + }, + { + Key: SeedSpanCountTable, + Version: roachpb.Version{Major: 21, Minor: 2, Internal: 110}, + }, // ************************************************* // Step (2): Add new versions here. diff --git a/pkg/clusterversion/key_string.go b/pkg/clusterversion/key_string.go index 2c11e27a1097..d14331ee26d4 100644 --- a/pkg/clusterversion/key_string.go +++ b/pkg/clusterversion/key_string.go @@ -58,11 +58,14 @@ func _() { _ = x[AutoStatsTableSettings-47] _ = x[ForecastStats-48] _ = x[SuperRegions-49] + _ = x[SpanCountTable-50] + _ = x[PreSeedSpanCountTable-51] + _ = x[SeedSpanCountTable-52] } -const _Key_name = "V21_2Start22_1TargetBytesAvoidExcessAvoidDrainingNamesDrainingNamesMigrationTraceIDDoesntImplyStructuredRecordingAlterSystemTableStatisticsAddAvgSizeColAlterSystemStmtDiagReqsMVCCAddSSTableInsertPublicSchemaNamespaceEntryOnRestoreUnsplitRangesInAsyncGCJobsValidateGrantOptionPebbleFormatBlockPropertyCollectorProbeRequestSelectRPCsTakeTracingInfoInbandPreSeedTenantSpanConfigsSeedTenantSpanConfigsPublicSchemasWithDescriptorsEnsureSpanConfigReconciliationEnsureSpanConfigSubscriptionEnableSpanConfigStoreScanWholeRowsSCRAMAuthenticationUnsafeLossOfQuorumRecoveryRangeLogAlterSystemProtectedTimestampAddColumnEnableProtectedTimestampsForTenantDeleteCommentsWithDroppedIndexesRemoveIncompatibleDatabasePrivilegesAddRaftAppliedIndexTermMigrationPostAddRaftAppliedIndexTermMigrationDontProposeWriteTimestampForLeaseTransfersTenantSettingsTableEnablePebbleFormatVersionBlockPropertiesDisableSystemConfigGossipTriggerMVCCIndexBackfillerEnableLeaseHolderRemovalBackupResolutionInJobLooselyCoupledRaftLogTruncationChangefeedIdlenessBackupDoesNotOverwriteLatestAndCheckpointEnableDeclarativeSchemaChangerRowLevelTTLPebbleFormatSplitUserKeysMarkedIncrementalBackupSubdirDateStyleIntervalStyleCastRewriteEnableNewStoreRebalancerClusterLocksVirtualTableAutoStatsTableSettingsForecastStatsSuperRegions" +const _Key_name = "V21_2Start22_1TargetBytesAvoidExcessAvoidDrainingNamesDrainingNamesMigrationTraceIDDoesntImplyStructuredRecordingAlterSystemTableStatisticsAddAvgSizeColAlterSystemStmtDiagReqsMVCCAddSSTableInsertPublicSchemaNamespaceEntryOnRestoreUnsplitRangesInAsyncGCJobsValidateGrantOptionPebbleFormatBlockPropertyCollectorProbeRequestSelectRPCsTakeTracingInfoInbandPreSeedTenantSpanConfigsSeedTenantSpanConfigsPublicSchemasWithDescriptorsEnsureSpanConfigReconciliationEnsureSpanConfigSubscriptionEnableSpanConfigStoreScanWholeRowsSCRAMAuthenticationUnsafeLossOfQuorumRecoveryRangeLogAlterSystemProtectedTimestampAddColumnEnableProtectedTimestampsForTenantDeleteCommentsWithDroppedIndexesRemoveIncompatibleDatabasePrivilegesAddRaftAppliedIndexTermMigrationPostAddRaftAppliedIndexTermMigrationDontProposeWriteTimestampForLeaseTransfersTenantSettingsTableEnablePebbleFormatVersionBlockPropertiesDisableSystemConfigGossipTriggerMVCCIndexBackfillerEnableLeaseHolderRemovalBackupResolutionInJobLooselyCoupledRaftLogTruncationChangefeedIdlenessBackupDoesNotOverwriteLatestAndCheckpointEnableDeclarativeSchemaChangerRowLevelTTLPebbleFormatSplitUserKeysMarkedIncrementalBackupSubdirDateStyleIntervalStyleCastRewriteEnableNewStoreRebalancerClusterLocksVirtualTableAutoStatsTableSettingsForecastStatsSuperRegionsSpanCountTablePreSeedSpanCountTableSeedSpanCountTable" -var _Key_index = [...]uint16{0, 5, 14, 36, 54, 76, 113, 152, 175, 189, 230, 256, 275, 309, 321, 352, 376, 397, 425, 455, 483, 504, 517, 536, 570, 608, 642, 674, 710, 742, 778, 820, 839, 879, 911, 930, 954, 975, 1006, 1024, 1065, 1095, 1106, 1137, 1160, 1193, 1217, 1241, 1263, 1276, 1288} +var _Key_index = [...]uint16{0, 5, 14, 36, 54, 76, 113, 152, 175, 189, 230, 256, 275, 309, 321, 352, 376, 397, 425, 455, 483, 504, 517, 536, 570, 608, 642, 674, 710, 742, 778, 820, 839, 879, 911, 930, 954, 975, 1006, 1024, 1065, 1095, 1106, 1137, 1160, 1193, 1217, 1241, 1263, 1276, 1288, 1302, 1323, 1341} func (i Key) String() string { if i < 0 || i >= Key(len(_Key_index)-1) { diff --git a/pkg/migration/migrationjob/migration_job.go b/pkg/migration/migrationjob/migration_job.go index 672e86e90493..f924f27df94c 100644 --- a/pkg/migration/migrationjob/migration_job.go +++ b/pkg/migration/migrationjob/migration_job.go @@ -92,6 +92,7 @@ func (r resumer) Resume(ctx context.Context, execCtxI interface{}) error { TestingKnobs: execCtx.ExecCfg().MigrationTestingKnobs, } tenantDeps.SpanConfig.KVAccessor = execCtx.ExecCfg().SpanConfigKVAccessor + tenantDeps.SpanConfig.Splitter = execCtx.ExecCfg().SpanConfigSplitter tenantDeps.SpanConfig.Default = execCtx.ExecCfg().DefaultZoneConfig.AsSpanConfig() err = m.Run(ctx, cv, tenantDeps, r.j) diff --git a/pkg/migration/migrations/BUILD.bazel b/pkg/migration/migrations/BUILD.bazel index 12b68055b2dc..8ec16e6dee32 100644 --- a/pkg/migration/migrations/BUILD.bazel +++ b/pkg/migration/migrations/BUILD.bazel @@ -19,6 +19,7 @@ go_library( "remove_invalid_database_privileges.go", "schema_changes.go", "seed_tenant_span_configs.go", + "span_count_table.go", "tenant_settings.go", ], importpath = "github.com/cockroachdb/cockroach/pkg/migration/migrations", diff --git a/pkg/migration/migrations/helpers_test.go b/pkg/migration/migrations/helpers_test.go index 07ce34bfaf2e..59dd1dedd667 100644 --- a/pkg/migration/migrations/helpers_test.go +++ b/pkg/migration/migrations/helpers_test.go @@ -84,7 +84,7 @@ func InjectLegacyTable( return err } tab.TableDescriptor = builder.BuildCreatedMutableTable().TableDescriptor - tab.Version = tab.ClusterVersion.Version + 1 + tab.Version = tab.ClusterVersion().Version + 1 return descriptors.WriteDesc(ctx, false /* kvTrace */, tab, txn) }) require.NoError(t, err) diff --git a/pkg/migration/migrations/migrations.go b/pkg/migration/migrations/migrations.go index 4f567579f54e..190bfc3561f6 100644 --- a/pkg/migration/migrations/migrations.go +++ b/pkg/migration/migrations/migrations.go @@ -131,6 +131,18 @@ var migrations = []migration.Migration{ NoPrecondition, fixCastForStyleMigration, ), + migration.NewTenantMigration( + "add the system.span_count table", + toCV(clusterversion.SpanCountTable), + NoPrecondition, + spanCountTableMigration, + ), + migration.NewTenantMigration( + "seed system.span_count with span count for existing tenants", + toCV(clusterversion.SeedSpanCountTable), + NoPrecondition, + seedSpanCountTableMigration, + ), } func init() { diff --git a/pkg/migration/migrations/span_count_table.go b/pkg/migration/migrations/span_count_table.go new file mode 100644 index 000000000000..8abdf96c6336 --- /dev/null +++ b/pkg/migration/migrations/span_count_table.go @@ -0,0 +1,97 @@ +// 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 migrations + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/clusterversion" + "github.com/cockroachdb/cockroach/pkg/jobs" + "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/migration" + "github.com/cockroachdb/cockroach/pkg/security" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" + "github.com/cockroachdb/errors" +) + +// spanCountTableMigration creates the system.span_count table for secondary +// tenants. +func spanCountTableMigration( + ctx context.Context, _ clusterversion.ClusterVersion, d migration.TenantDeps, _ *jobs.Job, +) error { + if d.Codec.ForSystemTenant() { + return nil // only applicable for secondary tenants + } + + return createSystemTable( + ctx, d.DB, d.Codec, systemschema.SpanCountTable, + ) +} + +// seedSpanCountTableMigration seeds system.span_count with data for existing +// secondary tenants. +func seedSpanCountTableMigration( + ctx context.Context, _ clusterversion.ClusterVersion, d migration.TenantDeps, _ *jobs.Job, +) error { + if d.Codec.ForSystemTenant() { + return nil // only applicable for secondary tenants + } + + return d.CollectionFactory.Txn(ctx, d.InternalExecutor, d.DB, func(ctx context.Context, txn *kv.Txn, descriptors *descs.Collection) error { + dbs, err := descriptors.GetAllDatabaseDescriptors(ctx, txn) + if err != nil { + return err + } + + var spanCount int + for _, db := range dbs { + if db.GetID() == systemschema.SystemDB.GetID() { + continue // we don't count system table descriptors + } + + tables, err := descriptors.GetAllTableDescriptorsInDatabase(ctx, txn, db.GetID()) + if err != nil { + return err + } + + for _, table := range tables { + splits, err := d.SpanConfig.Splitter.Splits(ctx, table) + if err != nil { + return err + } + spanCount += splits + } + } + + const seedSpanCountStmt = ` +INSERT INTO system.span_count (span_count) VALUES ($1) +ON CONFLICT (singleton) +DO UPDATE SET span_count = $1 +RETURNING span_count +` + datums, err := d.InternalExecutor.QueryRowEx(ctx, "seed-span-count", txn, + sessiondata.InternalExecutorOverride{User: security.RootUserName()}, + seedSpanCountStmt, spanCount) + if err != nil { + return err + } + if len(datums) != 1 { + return errors.AssertionFailedf("expected to return 1 row, return %d", len(datums)) + } + if insertedSpanCount := int64(tree.MustBeDInt(datums[0])); insertedSpanCount != int64(spanCount) { + return errors.AssertionFailedf("expected to insert %d, got %d", spanCount, insertedSpanCount) + } + return nil + }) +} diff --git a/pkg/migration/tenant_migration.go b/pkg/migration/tenant_migration.go index 4c6ca923bf66..f0204e1349db 100644 --- a/pkg/migration/tenant_migration.go +++ b/pkg/migration/tenant_migration.go @@ -39,6 +39,7 @@ type TenantDeps struct { SpanConfig struct { // deps for span config migrations; can be removed accordingly spanconfig.KVAccessor + spanconfig.Splitter Default roachpb.SpanConfig } diff --git a/pkg/server/BUILD.bazel b/pkg/server/BUILD.bazel index 47dd9900e594..1278d83c9c53 100644 --- a/pkg/server/BUILD.bazel +++ b/pkg/server/BUILD.bazel @@ -138,9 +138,11 @@ go_library( "//pkg/spanconfig/spanconfigjob", "//pkg/spanconfig/spanconfigkvaccessor", "//pkg/spanconfig/spanconfigkvsubscriber", + "//pkg/spanconfig/spanconfiglimiter", "//pkg/spanconfig/spanconfigmanager", "//pkg/spanconfig/spanconfigptsreader", "//pkg/spanconfig/spanconfigreconciler", + "//pkg/spanconfig/spanconfigsplitter", "//pkg/spanconfig/spanconfigsqltranslator", "//pkg/spanconfig/spanconfigsqlwatcher", "//pkg/sql", diff --git a/pkg/server/server_sql.go b/pkg/server/server_sql.go index 97fa00881131..af93a4b227c6 100644 --- a/pkg/server/server_sql.go +++ b/pkg/server/server_sql.go @@ -51,8 +51,10 @@ import ( "github.com/cockroachdb/cockroach/pkg/server/tracedumper" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/spanconfig" + "github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfiglimiter" "github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigmanager" "github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigreconciler" + "github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigsplitter" "github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigsqltranslator" "github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigsqlwatcher" "github.com/cockroachdb/cockroach/pkg/sql" @@ -548,11 +550,33 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) { compactEngineSpanFunc = cli.CompactEngineSpan } + spanConfig := struct { + manager *spanconfigmanager.Manager + sqlTranslatorFactory *spanconfigsqltranslator.Factory + sqlWatcher *spanconfigsqlwatcher.SQLWatcher + splitter spanconfig.Splitter + limiter spanconfig.Limiter + }{} + if codec.ForSystemTenant() { + spanConfig.limiter = spanconfiglimiter.NoopLimiter{} + spanConfig.splitter = spanconfigsplitter.NoopSplitter{} + } else { + spanConfigKnobs, _ := cfg.TestingKnobs.SpanConfig.(*spanconfig.TestingKnobs) + spanConfig.splitter = spanconfigsplitter.New(codec, spanConfigKnobs) + spanConfig.limiter = spanconfiglimiter.New( + cfg.circularInternalExecutor, + cfg.Settings, + spanConfigKnobs, + ) + } + collectionFactory := descs.NewCollectionFactory( cfg.Settings, leaseMgr, virtualSchemas, hydratedTablesCache, + spanConfig.splitter, + spanConfig.limiter, ) // Set up the DistSQL server. @@ -929,11 +953,6 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) { execCfg.MigrationTestingKnobs = knobs } - spanConfig := struct { - manager *spanconfigmanager.Manager - sqlTranslatorFactory *spanconfigsqltranslator.Factory - sqlWatcher *spanconfigsqlwatcher.SQLWatcher - }{} if !codec.ForSystemTenant() || !cfg.SpanConfigsDisabled { // Instantiate a span config manager. If we're the host tenant we'll // only do it unless COCKROACH_DISABLE_SPAN_CONFIGS is set. @@ -973,6 +992,8 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) { execCfg.SpanConfigReconciler = spanConfigReconciler } execCfg.SpanConfigKVAccessor = cfg.spanConfigAccessor + execCfg.SpanConfigLimiter = spanConfig.limiter + execCfg.SpanConfigSplitter = spanConfig.splitter temporaryObjectCleaner := sql.NewTemporaryObjectCleaner( cfg.Settings, diff --git a/pkg/spanconfig/BUILD.bazel b/pkg/spanconfig/BUILD.bazel index 19b4a19b8598..0fbf5adb9886 100644 --- a/pkg/spanconfig/BUILD.bazel +++ b/pkg/spanconfig/BUILD.bazel @@ -20,8 +20,10 @@ go_library( "//pkg/roachpb", "//pkg/sql/catalog", "//pkg/sql/catalog/descpb", + "//pkg/sql/catalog/systemschema", "//pkg/util/encoding", "//pkg/util/hlc", + "//pkg/util/log", "@com_github_cockroachdb_errors//:errors", "@com_github_stretchr_testify//require", ], diff --git a/pkg/spanconfig/spanconfig.go b/pkg/spanconfig/spanconfig.go index 5ba8758603fa..dfcefb884eb7 100644 --- a/pkg/spanconfig/spanconfig.go +++ b/pkg/spanconfig/spanconfig.go @@ -18,7 +18,9 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema" "github.com/cockroachdb/cockroach/pkg/util/hlc" + "github.com/cockroachdb/cockroach/pkg/util/log" ) // KVAccessor mediates access to KV span configurations pertaining to a given @@ -257,6 +259,15 @@ type StoreReader interface { GetSpanConfigForKey(ctx context.Context, key roachpb.RKey) (roachpb.SpanConfig, error) } +// Limiter is used to limit the number of span configs installed by secondary +// tenants. It takes in a delta (typically the difference in span configs +// between the committed and uncommitted state in the txn), uses it to maintain +// an aggregate counter, and informs the caller if exceeding the prescribed +// limit. +type Limiter interface { + ShouldLimit(ctx context.Context, txn *kv.Txn, delta int) (bool, error) +} + // Splitter returns the number of split points for the given table descriptor. // It steps through every "unit" that we can apply configurations over (table, // indexes, partitions and sub-partitions) and figures out the actual key @@ -288,6 +299,39 @@ type Splitter interface { Splits(ctx context.Context, table catalog.TableDescriptor) (int, error) } +// Delta considers both the committed and uncommitted state of a table +// descriptor and computes the difference in the number of spans we can apply a +// configuration over. +func Delta( + ctx context.Context, s Splitter, committed, uncommitted catalog.TableDescriptor, +) (int, error) { + if committed == nil && uncommitted == nil { + log.Fatalf(ctx, "unexpected: got two nil table descriptors") + } + + var nonNilDesc catalog.TableDescriptor + if committed != nil { + nonNilDesc = committed + } else { + nonNilDesc = uncommitted + } + if nonNilDesc.GetParentID() == systemschema.SystemDB.GetID() { + return 0, nil // we don't count tables in the system database + } + + committedSplits, err := s.Splits(ctx, committed) + if err != nil { + return 0, err + } + uncommittedSplits, err := s.Splits(ctx, uncommitted) + if err != nil { + return 0, err + } + + delta := uncommittedSplits - committedSplits + return delta, nil +} + // SQLUpdate captures either a descriptor or a protected timestamp update. // It is the unit emitted by the SQLWatcher. type SQLUpdate struct { diff --git a/pkg/spanconfig/spanconfiglimiter/BUILD.bazel b/pkg/spanconfig/spanconfiglimiter/BUILD.bazel new file mode 100644 index 000000000000..db8f3f60d9ac --- /dev/null +++ b/pkg/spanconfig/spanconfiglimiter/BUILD.bazel @@ -0,0 +1,23 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "spanconfiglimiter", + srcs = [ + "limiter.go", + "noop.go", + ], + importpath = "github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfiglimiter", + visibility = ["//visibility:public"], + deps = [ + "//pkg/clusterversion", + "//pkg/kv", + "//pkg/security", + "//pkg/settings", + "//pkg/settings/cluster", + "//pkg/spanconfig", + "//pkg/sql/sem/tree", + "//pkg/sql/sessiondata", + "//pkg/sql/sqlutil", + "@com_github_cockroachdb_errors//:errors", + ], +) diff --git a/pkg/spanconfig/spanconfiglimiter/limiter.go b/pkg/spanconfig/spanconfiglimiter/limiter.go new file mode 100644 index 000000000000..fba1c5f1d527 --- /dev/null +++ b/pkg/spanconfig/spanconfiglimiter/limiter.go @@ -0,0 +1,99 @@ +// 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 spanconfiglimiter is used to limit how many span configs are +// installed by tenants. +package spanconfiglimiter + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/clusterversion" + "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/security" + "github.com/cockroachdb/cockroach/pkg/settings" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/spanconfig" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" + "github.com/cockroachdb/cockroach/pkg/sql/sqlutil" + "github.com/cockroachdb/errors" +) + +var _ spanconfig.Limiter = &Limiter{} + +// tenantLimitSetting controls how many span configs a secondary tenant is +// allowed to install. It's settable only by the system tenant. +var tenantLimitSetting = settings.RegisterIntSetting( + settings.TenantReadOnly, + "spanconfig.tenant_limit", + "limit on the number of span configs a tenant is allowed to install", + 5000, +) + +// Limiter is used to limit the number of span configs installed by secondary +// tenants. It's a concrete implementation of the spanconfig.Limiter interface. +type Limiter struct { + ie sqlutil.InternalExecutor + settings *cluster.Settings + knobs *spanconfig.TestingKnobs +} + +// New constructs and returns a Limiter. +func New( + ie sqlutil.InternalExecutor, settings *cluster.Settings, knobs *spanconfig.TestingKnobs, +) *Limiter { + if knobs == nil { + knobs = &spanconfig.TestingKnobs{} + } + return &Limiter{ + ie: ie, + settings: settings, + knobs: knobs, + } +} + +// ShouldLimit is part of the spanconfig.Limiter interface. +func (l *Limiter) ShouldLimit(ctx context.Context, txn *kv.Txn, delta int) (bool, error) { + if !l.settings.Version.IsActive(ctx, clusterversion.PreSeedSpanCountTable) { + return false, nil // nothing to do + } + + if delta == 0 { + return false, nil + } + + limit := tenantLimitSetting.Get(&l.settings.SV) + if overrideFn := l.knobs.LimiterLimitOverride; overrideFn != nil { + limit = overrideFn() + } + + const updateSpanCountStmt = ` +INSERT INTO system.span_count (span_count) VALUES ($1) +ON CONFLICT (singleton) +DO UPDATE SET span_count = system.span_count.span_count + $1 +RETURNING span_count +` + datums, err := l.ie.QueryRowEx(ctx, "update-span-count", txn, + sessiondata.InternalExecutorOverride{User: security.RootUserName()}, + updateSpanCountStmt, delta) + if err != nil { + return false, err + } + if len(datums) != 1 { + return false, errors.AssertionFailedf("expected to return 1 row, return %d", len(datums)) + } + + if delta < 0 { + return false, nil // always allowed to lower span count + } + spanCountWithDelta := int64(tree.MustBeDInt(datums[0])) + return spanCountWithDelta > limit, nil +} diff --git a/pkg/spanconfig/spanconfiglimiter/noop.go b/pkg/spanconfig/spanconfiglimiter/noop.go new file mode 100644 index 000000000000..423d192fc2b0 --- /dev/null +++ b/pkg/spanconfig/spanconfiglimiter/noop.go @@ -0,0 +1,28 @@ +// 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 spanconfiglimiter + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/spanconfig" +) + +var _ spanconfig.Limiter = &NoopLimiter{} + +// NoopLimiter is a Limiter that simply no-ops (i.e. doesn't limit anything). +type NoopLimiter struct{} + +// ShouldLimit is part of the spanconfig.Limiter interface. +func (n NoopLimiter) ShouldLimit(context.Context, *kv.Txn, int) (bool, error) { + return false, nil +} diff --git a/pkg/spanconfig/spanconfigsplitter/BUILD.bazel b/pkg/spanconfig/spanconfigsplitter/BUILD.bazel index 54127852ae4a..1473539fda39 100644 --- a/pkg/spanconfig/spanconfigsplitter/BUILD.bazel +++ b/pkg/spanconfig/spanconfigsplitter/BUILD.bazel @@ -2,7 +2,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "spanconfigsplitter", - srcs = ["splitter.go"], + srcs = [ + "noop.go", + "splitter.go", + ], importpath = "github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigsplitter", visibility = ["//visibility:public"], deps = [ diff --git a/pkg/spanconfig/spanconfigsplitter/noop.go b/pkg/spanconfig/spanconfigsplitter/noop.go new file mode 100644 index 000000000000..2cb74d538579 --- /dev/null +++ b/pkg/spanconfig/spanconfigsplitter/noop.go @@ -0,0 +1,28 @@ +// Copyright 2021 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 spanconfigsplitter + +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/spanconfig" + "github.com/cockroachdb/cockroach/pkg/sql/catalog" +) + +var _ spanconfig.Splitter = &NoopSplitter{} + +// NoopSplitter is a Splitter that only returns "illegal use" errors. +type NoopSplitter struct{} + +// Splits is part of spanconfig.Splitter. +func (i NoopSplitter) Splits(context.Context, catalog.TableDescriptor) (int, error) { + return 0, nil +} diff --git a/pkg/spanconfig/spanconfigsplitter/splitter.go b/pkg/spanconfig/spanconfigsplitter/splitter.go index 27a253509d8a..97d58327d640 100644 --- a/pkg/spanconfig/spanconfigsplitter/splitter.go +++ b/pkg/spanconfig/spanconfigsplitter/splitter.go @@ -15,6 +15,7 @@ package spanconfigsplitter import ( "context" "fmt" + "reflect" "strings" "github.com/cockroachdb/cockroach/pkg/keys" @@ -106,6 +107,10 @@ func New(codec keys.SQLCodec, knobs *spanconfig.TestingKnobs) *Splitter { // descriptors that refer to them. This interface is used near by this GC // activity, so type information is not always available. func (s *Splitter) Splits(ctx context.Context, table catalog.TableDescriptor) (int, error) { + if isNil(table) { + return 0, nil // nothing to do + } + if s.knobs.ExcludeDroppedDescriptorsFromLookup && table.Dropped() { return 0, nil // we're excluding this descriptor; nothing to do here } @@ -197,3 +202,8 @@ type partition struct { index catalog.Index // index being partitioned level int // recursion level, used only for test-logging } + +func isNil(table catalog.TableDescriptor) bool { + vTable := reflect.ValueOf(table) + return vTable.Kind() == reflect.Ptr && vTable.IsNil() || table == nil +} diff --git a/pkg/spanconfig/spanconfigtestutils/spanconfigtestcluster/tenant_state.go b/pkg/spanconfig/spanconfigtestutils/spanconfigtestcluster/tenant_state.go index 8ce7595eec53..d8eb4080d1c9 100644 --- a/pkg/spanconfig/spanconfigtestutils/spanconfigtestcluster/tenant_state.go +++ b/pkg/spanconfig/spanconfigtestutils/spanconfigtestcluster/tenant_state.go @@ -82,8 +82,15 @@ func (s *Tenant) updateTimestampAfterLastSQLChange() { // the execution timestamp for subsequent use. func (s *Tenant) Exec(query string, args ...interface{}) { s.db.Exec(s.t, query, args...) + s.updateTimestampAfterLastSQLChange() +} +// ExecWithErr is like Exec but returns the error, if any. It records the +// execution timestamp for subsequent use. +func (s *Tenant) ExecWithErr(query string, args ...interface{}) error { + _, err := s.db.DB.ExecContext(context.Background(), query, args...) s.updateTimestampAfterLastSQLChange() + return err } // TimestampAfterLastSQLChange returns a timestamp after the last time Exec was diff --git a/pkg/spanconfig/testing_knobs.go b/pkg/spanconfig/testing_knobs.go index 255d83b89e56..7474ba0a1c72 100644 --- a/pkg/spanconfig/testing_knobs.go +++ b/pkg/spanconfig/testing_knobs.go @@ -94,6 +94,10 @@ type TestingKnobs struct { // ProtectedTSReaderOverrideFn returns a ProtectedTSReader which is used to // override the ProtectedTSReader used when setting up a new store. ProtectedTSReaderOverrideFn func(clock *hlc.Clock) ProtectedTSReader + + // LimiterLimitOverride, if set, allows tests to dynamically override the span + // config limit. + LimiterLimitOverride func() int64 } // ModuleTestingKnobs is part of the base.ModuleTestingKnobs interface. diff --git a/pkg/sql/alter_column_type.go b/pkg/sql/alter_column_type.go index b1eec3e89293..57c590de651c 100644 --- a/pkg/sql/alter_column_type.go +++ b/pkg/sql/alter_column_type.go @@ -154,7 +154,7 @@ func AlterColumnType( if err := alterColumnTypeGeneral(ctx, tableDesc, col, typ, t.Using, params, cmds, tn); err != nil { return err } - if err := params.p.createOrUpdateSchemaChangeJob(params.ctx, tableDesc, tree.AsStringWithFQNames(t, params.Ann()), tableDesc.ClusterVersion.NextMutationID); err != nil { + if err := params.p.createOrUpdateSchemaChangeJob(params.ctx, tableDesc, tree.AsStringWithFQNames(t, params.Ann()), tableDesc.ClusterVersion().NextMutationID); err != nil { return err } params.p.BufferClientNotice(params.ctx, pgnotice.Newf("ALTER COLUMN TYPE changes are finalized asynchronously; "+ @@ -270,7 +270,7 @@ func alterColumnTypeGeneral( // Disallow ALTER COLUMN TYPE general if the table is already undergoing // a schema change. - currentMutationID := tableDesc.ClusterVersion.NextMutationID + currentMutationID := tableDesc.ClusterVersion().NextMutationID for i := range tableDesc.Mutations { mut := &tableDesc.Mutations[i] if mut.MutationID < currentMutationID { diff --git a/pkg/sql/alter_index.go b/pkg/sql/alter_index.go index 39d8687eea1d..3311930787ae 100644 --- a/pkg/sql/alter_index.go +++ b/pkg/sql/alter_index.go @@ -154,7 +154,7 @@ func (n *alterIndexNode) startExec(params runParams) error { } mutationID := descpb.InvalidMutationID if addedMutations { - mutationID = n.tableDesc.ClusterVersion.NextMutationID + mutationID = n.tableDesc.ClusterVersion().NextMutationID } if err := params.p.writeSchemaChange( params.ctx, n.tableDesc, mutationID, tree.AsStringWithFQNames(n.n, params.Ann()), diff --git a/pkg/sql/alter_primary_key.go b/pkg/sql/alter_primary_key.go index 47deeb2c9ae4..a0451d598497 100644 --- a/pkg/sql/alter_primary_key.go +++ b/pkg/sql/alter_primary_key.go @@ -85,7 +85,7 @@ func (p *planner) AlterPrimaryKey( // Ensure that other schema changes on this table are not currently // executing, and that other schema changes have not been performed // in the current transaction. - currentMutationID := tableDesc.ClusterVersion.NextMutationID + currentMutationID := tableDesc.ClusterVersion().NextMutationID for i := range tableDesc.Mutations { mut := &tableDesc.Mutations[i] if mut.MutationID == currentMutationID { diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index 017ba071d7aa..c61959acf208 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -847,7 +847,7 @@ func (n *alterTableNode) startExec(params runParams) error { mutationID := descpb.InvalidMutationID if addedMutations { - mutationID = n.tableDesc.ClusterVersion.NextMutationID + mutationID = n.tableDesc.ClusterVersion().NextMutationID } if err := params.p.writeSchemaChange( params.ctx, n.tableDesc, mutationID, tree.AsStringWithFQNames(n.n, params.Ann()), diff --git a/pkg/sql/alter_table_locality.go b/pkg/sql/alter_table_locality.go index 540bb1d0fc0f..d5bf46687e08 100644 --- a/pkg/sql/alter_table_locality.go +++ b/pkg/sql/alter_table_locality.go @@ -445,7 +445,7 @@ func (n *alterTableSetLocalityNode) alterTableLocalityFromOrToRegionalByRow( return params.p.writeSchemaChange( params.ctx, n.tableDesc, - n.tableDesc.ClusterVersion.NextMutationID, + n.tableDesc.ClusterVersion().NextMutationID, tree.AsStringWithFQNames(&n.n, params.Ann()), ) } diff --git a/pkg/sql/backfill.go b/pkg/sql/backfill.go index 7cc69587c6fb..542c7391c69e 100644 --- a/pkg/sql/backfill.go +++ b/pkg/sql/backfill.go @@ -2061,7 +2061,8 @@ func (sc *SchemaChanger) mergeFromTemporaryIndex( }); err != nil { return err } - tableDesc := tabledesc.NewBuilder(&tbl.ClusterVersion).BuildImmutableTable() + clusterVersion := tbl.ClusterVersion() + tableDesc := tabledesc.NewBuilder(&clusterVersion).BuildImmutableTable() if err := sc.distIndexMerge(ctx, tableDesc, addingIndexes, temporaryIndexes, fractionScaler); err != nil { return err } @@ -2425,7 +2426,7 @@ func validateCheckInTxn( checkExpr string, ) error { var syntheticDescs []catalog.Descriptor - if tableDesc.Version > tableDesc.ClusterVersion.Version { + if tableDesc.Version > tableDesc.ClusterVersion().Version { syntheticDescs = append(syntheticDescs, tableDesc) } ie := ief(ctx, sessionData) @@ -2456,7 +2457,7 @@ func validateFkInTxn( fkName string, ) error { var syntheticTable catalog.TableDescriptor - if srcTable.Version > srcTable.ClusterVersion.Version { + if srcTable.Version > srcTable.ClusterVersion().Version { syntheticTable = srcTable } var fk *descpb.ForeignKeyConstraint @@ -2507,7 +2508,7 @@ func validateUniqueWithoutIndexConstraintInTxn( constraintName string, ) error { var syntheticDescs []catalog.Descriptor - if tableDesc.Version > tableDesc.ClusterVersion.Version { + if tableDesc.Version > tableDesc.ClusterVersion().Version { syntheticDescs = append(syntheticDescs, tableDesc) } diff --git a/pkg/sql/catalog/bootstrap/metadata.go b/pkg/sql/catalog/bootstrap/metadata.go index 154b205cc657..4662378acb06 100644 --- a/pkg/sql/catalog/bootstrap/metadata.go +++ b/pkg/sql/catalog/bootstrap/metadata.go @@ -319,7 +319,9 @@ func addSystemDescriptorsToSchema(target *MetadataSchema) { target.AddDescriptorForSystemTenant(systemschema.SpanConfigurationsTable) // Tables introduced in 22.1. + target.AddDescriptorForSystemTenant(systemschema.TenantSettingsTable) + target.AddDescriptorForNonSystemTenant(systemschema.SpanCountTable) // Adding a new system table? It should be added here to the metadata schema, // and also created as a migration for older clusters. diff --git a/pkg/sql/catalog/catconstants/constants.go b/pkg/sql/catalog/catconstants/constants.go index aeceec0fbb88..3cbebb2e5dab 100644 --- a/pkg/sql/catalog/catconstants/constants.go +++ b/pkg/sql/catalog/catconstants/constants.go @@ -84,6 +84,7 @@ const ( SQLInstancesTableName SystemTableName = "sql_instances" SpanConfigurationsTableName SystemTableName = "span_configurations" TenantSettingsTableName SystemTableName = "tenant_settings" + SpanCountTableName SystemTableName = "span_count" ) // Oid for virtual database and table. diff --git a/pkg/sql/catalog/catprivilege/system.go b/pkg/sql/catalog/catprivilege/system.go index 08fb45b071aa..06e27d97ddf1 100644 --- a/pkg/sql/catalog/catprivilege/system.go +++ b/pkg/sql/catalog/catprivilege/system.go @@ -63,6 +63,7 @@ var ( catconstants.SQLInstancesTableName, catconstants.SpanConfigurationsTableName, catconstants.TenantSettingsTableName, + catconstants.SpanCountTableName, } systemSuperuserPrivileges = func() map[descpb.NameInfo]privilege.List { diff --git a/pkg/sql/catalog/descs/BUILD.bazel b/pkg/sql/catalog/descs/BUILD.bazel index d68b6be84499..5b7dc77b1c6a 100644 --- a/pkg/sql/catalog/descs/BUILD.bazel +++ b/pkg/sql/catalog/descs/BUILD.bazel @@ -36,6 +36,7 @@ go_library( "//pkg/kv", "//pkg/settings", "//pkg/settings/cluster", + "//pkg/spanconfig", "//pkg/sql/catalog", "//pkg/sql/catalog/bootstrap", "//pkg/sql/catalog/catalogkeys", diff --git a/pkg/sql/catalog/descs/factory.go b/pkg/sql/catalog/descs/factory.go index 89a608f0999a..038f15b7d2f6 100644 --- a/pkg/sql/catalog/descs/factory.go +++ b/pkg/sql/catalog/descs/factory.go @@ -15,6 +15,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/spanconfig" "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/hydratedtables" "github.com/cockroachdb/cockroach/pkg/sql/catalog/lease" @@ -22,12 +23,14 @@ import ( // CollectionFactory is used to construct a new Collection. type CollectionFactory struct { - settings *cluster.Settings - codec keys.SQLCodec - leaseMgr *lease.Manager - virtualSchemas catalog.VirtualSchemas - hydratedTables *hydratedtables.Cache - systemDatabase *systemDatabaseNamespaceCache + settings *cluster.Settings + codec keys.SQLCodec + leaseMgr *lease.Manager + virtualSchemas catalog.VirtualSchemas + hydratedTables *hydratedtables.Cache + systemDatabase *systemDatabaseNamespaceCache + spanConfigSplitter spanconfig.Splitter + spanConfigLimiter spanconfig.Limiter } // NewCollectionFactory constructs a new CollectionFactory which holds onto @@ -37,14 +40,18 @@ func NewCollectionFactory( leaseMgr *lease.Manager, virtualSchemas catalog.VirtualSchemas, hydratedTables *hydratedtables.Cache, + spanConfigSplitter spanconfig.Splitter, + spanConfigLimiter spanconfig.Limiter, ) *CollectionFactory { return &CollectionFactory{ - settings: settings, - codec: leaseMgr.Codec(), - leaseMgr: leaseMgr, - virtualSchemas: virtualSchemas, - hydratedTables: hydratedTables, - systemDatabase: newSystemDatabaseNamespaceCache(leaseMgr.Codec()), + settings: settings, + codec: leaseMgr.Codec(), + leaseMgr: leaseMgr, + virtualSchemas: virtualSchemas, + hydratedTables: hydratedTables, + systemDatabase: newSystemDatabaseNamespaceCache(leaseMgr.Codec()), + spanConfigSplitter: spanConfigSplitter, + spanConfigLimiter: spanConfigLimiter, } } diff --git a/pkg/sql/catalog/descs/txn.go b/pkg/sql/catalog/descs/txn.go index dbe85df9fb5a..edcba664d361 100644 --- a/pkg/sql/catalog/descs/txn.go +++ b/pkg/sql/catalog/descs/txn.go @@ -17,6 +17,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/spanconfig" "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/lease" "github.com/cockroachdb/cockroach/pkg/sql/sqlutil" @@ -26,6 +27,7 @@ import ( ) var errTwoVersionInvariantViolated = errors.Errorf("two version invariant violated") +var errExceededSpanCountLimit = errors.Errorf("exceeded limit for number of table spans") // Txn enables callers to run transactions with a *Collection such that all // retrieved immutable descriptors are properly leased and all mutable @@ -90,6 +92,9 @@ func (cf *CollectionFactory) Txn( return err } modifiedDescriptors = descsCol.GetDescriptorsWithNewVersion() + if err := CheckSpanCountLimit(ctx, &descsCol, cf.spanConfigSplitter, cf.spanConfigLimiter, txn); err != nil { + return err + } retryErr, err := CheckTwoVersionInvariant( ctx, db.Clock(), ie, &descsCol, txn, nil /* onRetryBackoff */) if retryErr { @@ -219,3 +224,44 @@ func CheckTwoVersionInvariant( } return true, retryErr } + +// CheckSpanCountLimit checks whether committing the set of uncommitted tables +// would exceed the span count limit we're allowed (applicable only to secondary +// tenants). +func CheckSpanCountLimit( + ctx context.Context, + descsCol *Collection, + splitter spanconfig.Splitter, + limiter spanconfig.Limiter, + txn *kv.Txn, +) error { + if !descsCol.codec().ForSystemTenant() { + var totalSpanCountDelta int + for _, ut := range descsCol.GetUncommittedTables() { + uncommittedMutTable, err := descsCol.GetUncommittedMutableTableByID(ut.GetID()) + if err != nil { + return err + } + + var originalTableDesc catalog.TableDescriptor + if originalDesc := uncommittedMutTable.OriginalDescriptor(); originalDesc != nil { + originalTableDesc = originalDesc.(catalog.TableDescriptor) + } + delta, err := spanconfig.Delta(ctx, splitter, originalTableDesc, uncommittedMutTable) + if err != nil { + return err + } + totalSpanCountDelta += delta + } + + shouldLimit, err := limiter.ShouldLimit(ctx, txn, totalSpanCountDelta) + if err != nil { + return err + } + if shouldLimit { + return errExceededSpanCountLimit + } + } + + return nil +} diff --git a/pkg/sql/catalog/systemschema/system.go b/pkg/sql/catalog/systemschema/system.go index 389b3848d56c..bef93006ac86 100644 --- a/pkg/sql/catalog/systemschema/system.go +++ b/pkg/sql/catalog/systemschema/system.go @@ -667,6 +667,15 @@ CREATE TABLE system.tenant_settings ( CONSTRAINT "primary" PRIMARY KEY (tenant_id, name), FAMILY (tenant_id, name, value, last_updated, value_type, reason) );` + + SpanCountTableSchema = ` +CREATE TABLE system.span_count ( + singleton BOOL DEFAULT TRUE, + span_count INT NOT NULL, + CONSTRAINT "primary" PRIMARY KEY (singleton), + CONSTRAINT single_row CHECK (singleton), + FAMILY "primary" (singleton, span_count) +);` ) func pk(name string) descpb.IndexDescriptor { @@ -2357,6 +2366,36 @@ var ( Version: descpb.StrictIndexColumnIDGuaranteesVersion, }, )) + + // SpanCountTable is the descriptor for the split count table. + SpanCountTable = registerSystemTable( + SpanCountTableSchema, + systemTable( + catconstants.SpanCountTableName, + descpb.InvalidID, // dynamically assigned + []descpb.ColumnDescriptor{ + {Name: "singleton", ID: 1, Type: types.Bool, DefaultExpr: &trueBoolString}, + {Name: "span_count", ID: 2, Type: types.Int}, + }, + []descpb.ColumnFamilyDescriptor{ + { + Name: "primary", + ID: 0, + DefaultColumnID: 2, + ColumnNames: []string{"singleton", "span_count"}, + ColumnIDs: []descpb.ColumnID{1, 2}, + }, + }, + pk("singleton"), + ), + func(tbl *descpb.TableDescriptor) { + tbl.Checks = []*descpb.TableDescriptor_CheckConstraint{{ + Name: "single_row", + Expr: "singleton", + ColumnIDs: []descpb.ColumnID{1}, + }} + }, + ) ) type descRefByName struct { diff --git a/pkg/sql/catalog/tabledesc/safe_format_test.go b/pkg/sql/catalog/tabledesc/safe_format_test.go index b27e693083e5..739f8d3f43a9 100644 --- a/pkg/sql/catalog/tabledesc/safe_format_test.go +++ b/pkg/sql/catalog/tabledesc/safe_format_test.go @@ -240,7 +240,7 @@ func TestSafeMessage(t *testing.T) { mutable.Families[0].ColumnNames = append(mutable.Families[0].ColumnNames, "c") mutable.Families[0].ColumnIDs = append(mutable.Families[0].ColumnIDs, 5) mutable.ModificationTime = hlc.Timestamp{WallTime: 1e9} - mutable.ClusterVersion = *mutable.TableDesc() + mutable.TestingSetClusterVersion(*mutable.TableDesc()) return mutable.ImmutableCopy().(catalog.TableDescriptor) }, }, @@ -262,7 +262,7 @@ func TestSafeMessage(t *testing.T) { "Indexes: [{ID: 1, Unique: true, KeyColumns: [{ID: 1, Dir: ASC}]}]" + "}", f: func(mutable *tabledesc.Mutable) catalog.TableDescriptor { - mutable.ClusterVersion = *mutable.TableDesc() + mutable.TestingSetClusterVersion(*mutable.TableDesc()) return mutable.ImmutableCopy().(catalog.TableDescriptor) }, }, diff --git a/pkg/sql/catalog/tabledesc/structured.go b/pkg/sql/catalog/tabledesc/structured.go index dc21740b7076..f46b9215f590 100644 --- a/pkg/sql/catalog/tabledesc/structured.go +++ b/pkg/sql/catalog/tabledesc/structured.go @@ -48,8 +48,9 @@ import ( type Mutable struct { wrapper - // ClusterVersion represents the version of the table descriptor read from the store. - ClusterVersion descpb.TableDescriptor + // original represents the version of the table descriptor read from the + // store. + original *immutable } const ( @@ -983,7 +984,7 @@ func fitColumnToFamily(desc *Mutable, col descpb.ColumnDescriptor) (int, bool) { // MaybeIncrementVersion implements the MutableDescriptor interface. func (desc *Mutable) MaybeIncrementVersion() { // Already incremented, no-op. - if desc.Version == desc.ClusterVersion.Version+1 || desc.ClusterVersion.Version == 0 { + if desc.Version == desc.ClusterVersion().Version+1 || desc.ClusterVersion().Version == 0 { return } desc.Version++ @@ -996,17 +997,37 @@ func (desc *Mutable) MaybeIncrementVersion() { // OriginalName implements the MutableDescriptor interface. func (desc *Mutable) OriginalName() string { - return desc.ClusterVersion.Name + return desc.ClusterVersion().Name } // OriginalID implements the MutableDescriptor interface. func (desc *Mutable) OriginalID() descpb.ID { - return desc.ClusterVersion.ID + return desc.ClusterVersion().ID } // OriginalVersion implements the MutableDescriptor interface. func (desc *Mutable) OriginalVersion() descpb.DescriptorVersion { - return desc.ClusterVersion.Version + return desc.ClusterVersion().Version +} + +// ClusterVersion returns the version of the table descriptor read from the +// store, if any. +// +// TODO(ajwerner): Make this deal in catalog.TableDescriptor instead. +func (desc *Mutable) ClusterVersion() descpb.TableDescriptor { + if desc.original == nil { + return descpb.TableDescriptor{} + } + return desc.original.TableDescriptor +} + +// OriginalDescriptor returns the original state of the descriptor prior to +// the mutations. +func (desc *Mutable) OriginalDescriptor() catalog.Descriptor { + if desc.original != nil { + return desc.original + } + return nil } // FamilyHeuristicTargetBytes is the target total byte size of columns that the @@ -1263,7 +1284,7 @@ func (desc *Mutable) RenameColumnDescriptor(column catalog.Column, newColName st // It returns either an active column or a column that was added in the // same transaction that is currently running. func (desc *Mutable) FindActiveOrNewColumnByName(name tree.Name) (catalog.Column, error) { - currentMutationID := desc.ClusterVersion.NextMutationID + currentMutationID := desc.ClusterVersion().NextMutationID for _, col := range desc.DeletableColumns() { if col.ColName() == name && ((col.Public()) || @@ -2170,8 +2191,8 @@ func (desc *Mutable) addMutationWithNextID(m descpb.DescriptorMutation) { // For tables created in the same transaction the next mutation ID will // not have been allocated and the added mutation will use an invalid ID. // This is fine because the mutation will be processed immediately. - m.MutationID = desc.ClusterVersion.NextMutationID - desc.NextMutationID = desc.ClusterVersion.NextMutationID + 1 + m.MutationID = desc.ClusterVersion().NextMutationID + desc.NextMutationID = desc.ClusterVersion().NextMutationID + 1 desc.Mutations = append(desc.Mutations, m) } @@ -2233,7 +2254,7 @@ func (desc *wrapper) HasColumnBackfillMutation() bool { // IsNew returns true if the table was created in the current // transaction. func (desc *Mutable) IsNew() bool { - return desc.ClusterVersion.ID == descpb.InvalidID + return desc.ClusterVersion().ID == descpb.InvalidID } // ColumnsSelectors generates Select expressions for cols. diff --git a/pkg/sql/catalog/tabledesc/table_desc.go b/pkg/sql/catalog/tabledesc/table_desc.go index 3db457e49281..fb62cff41fd5 100644 --- a/pkg/sql/catalog/tabledesc/table_desc.go +++ b/pkg/sql/catalog/tabledesc/table_desc.go @@ -139,7 +139,8 @@ func (desc *Mutable) NewBuilder() catalog.DescriptorBuilder { // IsUncommittedVersion implements the Descriptor interface. func (desc *Mutable) IsUncommittedVersion() bool { - return desc.IsNew() || desc.GetVersion() != desc.ClusterVersion.GetVersion() + clusterVersion := desc.ClusterVersion() + return desc.IsNew() || desc.GetVersion() != clusterVersion.GetVersion() } // SetDrainingNames implements the MutableDescriptor interface. diff --git a/pkg/sql/catalog/tabledesc/table_desc_builder.go b/pkg/sql/catalog/tabledesc/table_desc_builder.go index 5f507696bac7..de17bc019165 100644 --- a/pkg/sql/catalog/tabledesc/table_desc_builder.go +++ b/pkg/sql/catalog/tabledesc/table_desc_builder.go @@ -161,7 +161,7 @@ func (tdb *tableDescriptorBuilder) BuildExistingMutableTable() *Mutable { TableDescriptor: *tdb.maybeModified, changes: tdb.changes, }, - ClusterVersion: *tdb.original, + original: makeImmutable(tdb.original), } } diff --git a/pkg/sql/catalog/tabledesc/table_desc_test.go b/pkg/sql/catalog/tabledesc/table_desc_test.go index a5814bc3cbea..f34a27256144 100644 --- a/pkg/sql/catalog/tabledesc/table_desc_test.go +++ b/pkg/sql/catalog/tabledesc/table_desc_test.go @@ -49,3 +49,9 @@ func TestMaybeIncrementVersion(t *testing.T) { require.Equal(t, descpb.DescriptorVersion(2), mut.GetVersion()) }) } + +// TestingSetClusterVersion is a test helper to override the original table +// descriptor. +func (desc *Mutable) TestingSetClusterVersion(d descpb.TableDescriptor) { + desc.original = makeImmutable(&d) +} diff --git a/pkg/sql/conn_executor_exec.go b/pkg/sql/conn_executor_exec.go index 130139919cc5..4eba91d0524b 100644 --- a/pkg/sql/conn_executor_exec.go +++ b/pkg/sql/conn_executor_exec.go @@ -834,6 +834,17 @@ func (ex *connExecutor) checkDescriptorTwoVersionInvariant(ctx context.Context) if knobs := ex.server.cfg.SchemaChangerTestingKnobs; knobs != nil { inRetryBackoff = knobs.TwoVersionLeaseViolation } + + if err := descs.CheckSpanCountLimit( + ctx, + &ex.extraTxnState.descCollection, + ex.server.cfg.SpanConfigSplitter, + ex.server.cfg.SpanConfigLimiter, + ex.state.mu.txn, + ); err != nil { + return err + } + retryErr, err := descs.CheckTwoVersionInvariant( ctx, ex.server.cfg.Clock, diff --git a/pkg/sql/create_index.go b/pkg/sql/create_index.go index dc4ad595e328..35a4303178e9 100644 --- a/pkg/sql/create_index.go +++ b/pkg/sql/create_index.go @@ -689,7 +689,7 @@ func (n *createIndexNode) startExec(params runParams) error { index := n.tableDesc.Mutations[mutationIdx].GetIndex() indexName := index.Name - mutationID := n.tableDesc.ClusterVersion.NextMutationID + mutationID := n.tableDesc.ClusterVersion().NextMutationID if err := params.p.writeSchemaChange( params.ctx, n.tableDesc, mutationID, tree.AsStringWithFQNames(n.n, params.Ann()), ); err != nil { diff --git a/pkg/sql/create_view.go b/pkg/sql/create_view.go index f6eb454dfd2c..ef2a5e036a6a 100644 --- a/pkg/sql/create_view.go +++ b/pkg/sql/create_view.go @@ -522,7 +522,7 @@ func (p *planner) replaceViewDesc( // Compare toReplace against its ClusterVersion to verify if // its new set of columns is valid for a replacement view. if err := verifyReplacingViewColumns( - toReplace.ClusterVersion.Columns, + toReplace.ClusterVersion().Columns, toReplace.Columns, ); err != nil { return nil, err diff --git a/pkg/sql/drop_index.go b/pkg/sql/drop_index.go index e63e65893e72..35e7f67ed38f 100644 --- a/pkg/sql/drop_index.go +++ b/pkg/sql/drop_index.go @@ -264,7 +264,7 @@ func (n *dropIndexNode) finalizeDropColumn(params runParams, tableDesc *tabledes if err := tableDesc.AllocateIDs(params.ctx, version); err != nil { return err } - mutationID := tableDesc.ClusterVersion.NextMutationID + mutationID := tableDesc.ClusterVersion().NextMutationID if err := params.p.writeSchemaChange( params.ctx, tableDesc, mutationID, tree.AsStringWithFQNames(n.n, params.Ann()), ); err != nil { @@ -550,7 +550,7 @@ func (p *planner) dropIndexByName( return err } - mutationID := tableDesc.ClusterVersion.NextMutationID + mutationID := tableDesc.ClusterVersion().NextMutationID if err := p.writeSchemaChange(ctx, tableDesc, mutationID, jobDesc); err != nil { return err } diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go index 1ff2c933ac39..b21cc9a4860c 100644 --- a/pkg/sql/exec_util.go +++ b/pkg/sql/exec_util.go @@ -1276,6 +1276,13 @@ type ExecutorConfig struct { // and related migrations. SpanConfigReconciler spanconfig.Reconciler + // SpanConfigSplitter is used during migrations to seed system.span_count with + // the right number of tenant spans. + SpanConfigSplitter spanconfig.Splitter + + // SpanConfigLimiter is used to limit how many span configs installed. + SpanConfigLimiter spanconfig.Limiter + // SpanConfigKVAccessor is used when creating and deleting tenant // records. SpanConfigKVAccessor spanconfig.KVAccessor diff --git a/pkg/sql/gcjob/table_garbage_collection.go b/pkg/sql/gcjob/table_garbage_collection.go index 3269e27e3407..ad9491321d80 100644 --- a/pkg/sql/gcjob/table_garbage_collection.go +++ b/pkg/sql/gcjob/table_garbage_collection.go @@ -20,6 +20,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/settings" + "github.com/cockroachdb/cockroach/pkg/spanconfig" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" @@ -71,6 +72,19 @@ func gcTables( return errors.Wrapf(err, "clearing data for table %d", table.GetID()) } + delta, err := spanconfig.Delta(ctx, execCfg.SpanConfigSplitter, table, nil /* uncommitted */) + if err != nil { + return err + } + + // Deduct from system.span_count appropriately. + if err := execCfg.DB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { + _, err := execCfg.SpanConfigLimiter.ShouldLimit(ctx, txn, delta) + return err + }); err != nil { + return errors.Wrapf(err, "deducting span count for table %d", table.GetID()) + } + // Finished deleting all the table data, now delete the table meta data. if err := sql.DeleteTableDescAndZoneConfig( ctx, execCfg.DB, execCfg.Settings, execCfg.Codec, table, diff --git a/pkg/sql/logictest/testdata/logic_test/system b/pkg/sql/logictest/testdata/logic_test/system index da11a92e9802..2e6f92d439df 100644 --- a/pkg/sql/logictest/testdata/logic_test/system +++ b/pkg/sql/logictest/testdata/logic_test/system @@ -76,6 +76,7 @@ public role_members table NULL 0 NULL public role_options table NULL 0 NULL public scheduled_jobs table NULL 0 NULL public settings table NULL 0 NULL +public span_count table NULL 0 NULL public sql_instances table NULL 0 NULL public sqlliveness table NULL 0 NULL public statement_bundle_chunks table NULL 0 NULL @@ -179,6 +180,7 @@ SELECT id FROM system.descriptor 43 44 46 +50 100 101 102 @@ -828,6 +830,16 @@ system public settings root GRANT true system public settings root INSERT true system public settings root SELECT true system public settings root UPDATE true +system public span_count admin DELETE true +system public span_count admin GRANT true +system public span_count admin INSERT true +system public span_count admin SELECT true +system public span_count admin UPDATE true +system public span_count root DELETE true +system public span_count root GRANT true +system public span_count root INSERT true +system public span_count root SELECT true +system public span_count root UPDATE true system public sql_instances admin DELETE true system public sql_instances admin GRANT true system public sql_instances admin INSERT true diff --git a/pkg/sql/logictest/testdata/logic_test/system_namespace b/pkg/sql/logictest/testdata/logic_test/system_namespace index 295ba69ee910..24ff6f69b29d 100644 --- a/pkg/sql/logictest/testdata/logic_test/system_namespace +++ b/pkg/sql/logictest/testdata/logic_test/system_namespace @@ -82,6 +82,7 @@ SELECT * FROM system.namespace 1 29 role_options 33 1 29 scheduled_jobs 37 1 29 settings 6 +1 29 span_count 50 1 29 sql_instances 46 1 29 sqlliveness 39 1 29 statement_bundle_chunks 34 diff --git a/pkg/sql/refresh_materialized_view.go b/pkg/sql/refresh_materialized_view.go index 296202dea1da..db46b896a4bc 100644 --- a/pkg/sql/refresh_materialized_view.go +++ b/pkg/sql/refresh_materialized_view.go @@ -118,7 +118,7 @@ func (n *refreshMaterializedViewNode) startExec(params runParams) error { return params.p.writeSchemaChange( params.ctx, n.desc, - n.desc.ClusterVersion.NextMutationID, + n.desc.ClusterVersion().NextMutationID, tree.AsStringWithFQNames(n.n, params.Ann()), ) } diff --git a/pkg/sql/schema_changer.go b/pkg/sql/schema_changer.go index 23a04bf8b9c7..5dc4e589a980 100644 --- a/pkg/sql/schema_changer.go +++ b/pkg/sql/schema_changer.go @@ -2791,10 +2791,10 @@ func (sc *SchemaChanger) queueCleanupJob( ctx context.Context, scDesc *tabledesc.Mutable, txn *kv.Txn, ) (jobspb.JobID, error) { // Create jobs for dropped columns / indexes to be deleted. - mutationID := scDesc.ClusterVersion.NextMutationID + mutationID := scDesc.ClusterVersion().NextMutationID span := scDesc.PrimaryIndexSpan(sc.execCfg.Codec) var spanList []jobspb.ResumeSpanList - for j := len(scDesc.ClusterVersion.Mutations); j < len(scDesc.Mutations); j++ { + for j := len(scDesc.ClusterVersion().Mutations); j < len(scDesc.Mutations); j++ { spanList = append(spanList, jobspb.ResumeSpanList{ ResumeSpans: roachpb.Spans{span}, diff --git a/pkg/sql/table.go b/pkg/sql/table.go index 98db79fe961a..e886ce051417 100644 --- a/pkg/sql/table.go +++ b/pkg/sql/table.go @@ -134,7 +134,7 @@ func (p *planner) createOrUpdateSchemaChangeJob( } } span := tableDesc.PrimaryIndexSpan(p.ExecCfg().Codec) - for i := len(tableDesc.ClusterVersion.Mutations) + len(spanList); i < len(tableDesc.Mutations); i++ { + for i := len(tableDesc.ClusterVersion().Mutations) + len(spanList); i < len(tableDesc.Mutations); i++ { var resumeSpans []roachpb.Span mut := tableDesc.Mutations[i] if mut.GetIndex() != nil && mut.GetIndex().UseDeletePreservingEncoding { diff --git a/pkg/sql/tests/system_table_test.go b/pkg/sql/tests/system_table_test.go index 30836846c97b..91ab0d8e0453 100644 --- a/pkg/sql/tests/system_table_test.go +++ b/pkg/sql/tests/system_table_test.go @@ -177,7 +177,7 @@ func TestSystemTableLiterals(t *testing.T) { } } - const expectedNumberOfSystemTables = 38 + const expectedNumberOfSystemTables = 39 require.Equal(t, expectedNumberOfSystemTables, len(testcases)) for name, test := range testcases { diff --git a/pkg/sql/tests/testdata/initial_keys b/pkg/sql/tests/testdata/initial_keys index 50723f24206a..0ffbdd9c4904 100644 --- a/pkg/sql/tests/testdata/initial_keys +++ b/pkg/sql/tests/testdata/initial_keys @@ -129,7 +129,7 @@ initial-keys tenant=system initial-keys tenant=5 ---- -73 keys: +75 keys: /Tenant/5/Table/3/1/1/2/1 /Tenant/5/Table/3/1/3/2/1 /Tenant/5/Table/3/1/4/2/1 @@ -165,6 +165,7 @@ initial-keys tenant=5 /Tenant/5/Table/3/1/43/2/1 /Tenant/5/Table/3/1/44/2/1 /Tenant/5/Table/3/1/46/2/1 + /Tenant/5/Table/3/1/50/2/1 /Tenant/5/Table/5/1/0/2/1 /Tenant/5/Table/7/1/0/0 /Tenant/5/NamespaceTable/30/1/0/0/"system"/4/1 @@ -191,6 +192,7 @@ initial-keys tenant=5 /Tenant/5/NamespaceTable/30/1/1/29/"role_options"/4/1 /Tenant/5/NamespaceTable/30/1/1/29/"scheduled_jobs"/4/1 /Tenant/5/NamespaceTable/30/1/1/29/"settings"/4/1 + /Tenant/5/NamespaceTable/30/1/1/29/"span_count"/4/1 /Tenant/5/NamespaceTable/30/1/1/29/"sql_instances"/4/1 /Tenant/5/NamespaceTable/30/1/1/29/"sqlliveness"/4/1 /Tenant/5/NamespaceTable/30/1/1/29/"statement_bundle_chunks"/4/1 @@ -208,7 +210,7 @@ initial-keys tenant=5 initial-keys tenant=999 ---- -73 keys: +75 keys: /Tenant/999/Table/3/1/1/2/1 /Tenant/999/Table/3/1/3/2/1 /Tenant/999/Table/3/1/4/2/1 @@ -244,6 +246,7 @@ initial-keys tenant=999 /Tenant/999/Table/3/1/43/2/1 /Tenant/999/Table/3/1/44/2/1 /Tenant/999/Table/3/1/46/2/1 + /Tenant/999/Table/3/1/50/2/1 /Tenant/999/Table/5/1/0/2/1 /Tenant/999/Table/7/1/0/0 /Tenant/999/NamespaceTable/30/1/0/0/"system"/4/1 @@ -270,6 +273,7 @@ initial-keys tenant=999 /Tenant/999/NamespaceTable/30/1/1/29/"role_options"/4/1 /Tenant/999/NamespaceTable/30/1/1/29/"scheduled_jobs"/4/1 /Tenant/999/NamespaceTable/30/1/1/29/"settings"/4/1 + /Tenant/999/NamespaceTable/30/1/1/29/"span_count"/4/1 /Tenant/999/NamespaceTable/30/1/1/29/"sql_instances"/4/1 /Tenant/999/NamespaceTable/30/1/1/29/"sqlliveness"/4/1 /Tenant/999/NamespaceTable/30/1/1/29/"statement_bundle_chunks"/4/1