diff --git a/docs/generated/http/full.md b/docs/generated/http/full.md index 5ef3e1a52e6a..134885a73142 100644 --- a/docs/generated/http/full.md +++ b/docs/generated/http/full.md @@ -4603,6 +4603,7 @@ a table. | zone_config_level | [ZoneConfigurationLevel](#cockroach.server.serverpb.TableDetailsResponse-cockroach.server.serverpb.ZoneConfigurationLevel) | | The level at which this object's zone configuration is set. | [reserved](#support-status) | | descriptor_id | [int64](#cockroach.server.serverpb.TableDetailsResponse-int64) | | descriptor_id is an identifier used to uniquely identify this table. It can be used to find events pertaining to this table by filtering on the 'target_id' field of events. | [reserved](#support-status) | | configure_zone_statement | [string](#cockroach.server.serverpb.TableDetailsResponse-string) | | configure_zone_statement is the output of "SHOW ZONE CONFIGURATION FOR TABLE" for this table. It is a SQL statement that would re-configure the table's current zone if executed. | [reserved](#support-status) | +| stats_last_created_at | [google.protobuf.Timestamp](#cockroach.server.serverpb.TableDetailsResponse-google.protobuf.Timestamp) | | stats_last_created_at is the time at which statistics were last created. | [reserved](#support-status) | diff --git a/docs/generated/swagger/spec.json b/docs/generated/swagger/spec.json index 0188077a2ae3..5914ec0c4249 100644 --- a/docs/generated/swagger/spec.json +++ b/docs/generated/swagger/spec.json @@ -1279,6 +1279,12 @@ "format": "int64", "x-go-name": "RangeCount" }, + "stats_last_created_at": { + "description": "stats_last_created_at is the time at which statistics were last created.", + "type": "string", + "format": "date-time", + "x-go-name": "StatsLastCreatedAt" + }, "zone_config": { "$ref": "#/definitions/ZoneConfig" }, diff --git a/pkg/base/test_server_args.go b/pkg/base/test_server_args.go index 3e5e85f539a6..9f609c84b5a1 100644 --- a/pkg/base/test_server_args.go +++ b/pkg/base/test_server_args.go @@ -147,6 +147,11 @@ type TestServerArgs struct { // TODO(irfansharif): Remove all uses of this when we rip out the system // config span. DisableSpanConfigs bool + + // TestServer will probabilistically start a single SQL server on each + // node for multi-tenant testing, and default all connections to that + // SQL server. Use this flag to disable that behavior. + DisableDefaultSQLServer bool } // TestClusterArgs contains the parameters one can set when creating a test @@ -244,9 +249,10 @@ const ( type TestTenantArgs struct { TenantID roachpb.TenantID - // Existing, if true, indicates an existing tenant, rather than a new tenant - // to be created by StartTenant. - Existing bool + // DisableCreateTenant disables the explicit creation of a tenant when + // StartTenant is attempted. It's used in cases where we want to validate + // that a tenant doesn't start if it isn't existing. + DisableCreateTenant bool // Settings allows the caller to control the settings object used for the // tenant cluster. diff --git a/pkg/ccl/backupccl/backup_test.go b/pkg/ccl/backupccl/backup_test.go index f2aa7b36b05c..b4496c3018aa 100644 --- a/pkg/ccl/backupccl/backup_test.go +++ b/pkg/ccl/backupccl/backup_test.go @@ -170,6 +170,10 @@ func (d *datadrivenTestState) addServer( var cleanup func() params := base.TestClusterArgs{} params.ServerArgs.ExternalIODirConfig = ioConf + // Unfortunately, we have to disable the default SQL server here because + // of numerous failures in the data driven test suite when it's enabled. + // This should be investigated. Tracked with #76378. + params.ServerArgs.DisableDefaultSQLServer = true if tempCleanupFrequency != "" { duration, err := time.ParseDuration(tempCleanupFrequency) if err != nil { @@ -1180,7 +1184,12 @@ func backupAndRestore( t.Fatal(err) } } - args := base.TestServerArgs{ExternalIODir: tc.Servers[backupNodeID].ClusterSettings().ExternalIODir} + args := base.TestServerArgs{ + ExternalIODir: tc.Servers[backupNodeID].ClusterSettings().ExternalIODir, + // Test fails when run under a SQL server. More investigation is + // required. Tracked with #76378. + DisableDefaultSQLServer: tc.Servers[backupNodeID].Cfg.DisableDefaultSQLServer, + } tcRestore := testcluster.StartTestCluster(t, singleNode, base.TestClusterArgs{ServerArgs: args}) defer tcRestore.Stopper().Stop(ctx) sqlDBRestore := sqlutils.MakeSQLRunner(tcRestore.Conns[0]) @@ -5777,7 +5786,9 @@ func TestBackupRestoreSequenceOwnership(t *testing.T) { const numAccounts = 1 _, origDB, dir, cleanupFn := backupRestoreTestSetup(t, singleNode, numAccounts, InitManualReplication) defer cleanupFn() - args := base.TestServerArgs{ExternalIODir: dir} + // Test fails when run under a SQL server. More investigation is + // required. Tracked with #76378. + args := base.TestServerArgs{ExternalIODir: dir, DisableDefaultSQLServer: true} // Setup for sequence ownership backup/restore tests in the same database. backupLoc := localFoo + `/d` @@ -6342,6 +6353,9 @@ func TestProtectedTimestampsDuringBackup(t *testing.T) { defer dirCleanupFn() params := base.TestClusterArgs{} params.ServerArgs.ExternalIODir = dir + // This test hangs when run from a SQL server. More investigation is + // required. Tracked with #76378. + params.ServerArgs.DisableDefaultSQLServer = true params.ServerArgs.Knobs.Store = &kvserver.StoreTestingKnobs{ TestingRequestFilter: func(ctx context.Context, ba roachpb.BatchRequest) *roachpb.Error { for _, ru := range ba.Requests { @@ -6792,6 +6806,9 @@ func TestRestoreErrorPropagates(t *testing.T) { defer dirCleanupFn() params := base.TestClusterArgs{} params.ServerArgs.ExternalIODir = dir + // This test fails when run from a SQL server. More investigation is + // required. Tracked with #76378. + params.ServerArgs.DisableDefaultSQLServer = true jobsTableKey := keys.SystemSQLCodec.TablePrefix(uint32(systemschema.JobsTable.GetID())) var shouldFail, failures int64 params.ServerArgs.Knobs.Store = &kvserver.StoreTestingKnobs{ @@ -6843,6 +6860,9 @@ func TestProtectedTimestampsFailDueToLimits(t *testing.T) { defer dirCleanupFn() params := base.TestClusterArgs{} params.ServerArgs.ExternalIODir = dir + // This test fails when run from a SQL server. More investigation is + // required. Tracked with #76378. + params.ServerArgs.DisableDefaultSQLServer = true tc := testcluster.StartTestCluster(t, 1, params) defer tc.Stopper().Stop(ctx) db := tc.ServerConn(0) @@ -7248,7 +7268,10 @@ func TestBackupRestoreTenant(t *testing.T) { restoreTC := testcluster.StartTestCluster( t, singleNode, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ ExternalIODir: dir, - Knobs: base.TestingKnobs{JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals()}, + // This test fails when run from a SQL server as it assumes that + // it's not running in SQL server. Tracked with #76378. + DisableDefaultSQLServer: true, + Knobs: base.TestingKnobs{JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals()}, }}, ) defer restoreTC.Stopper().Stop(ctx) @@ -7269,7 +7292,7 @@ func TestBackupRestoreTenant(t *testing.T) { ten10Stopper := stop.NewStopper() _, restoreConn10 := serverutils.StartTenant( t, restoreTC.Server(0), base.TestTenantArgs{ - TenantID: roachpb.MakeTenantID(10), Existing: true, Stopper: ten10Stopper, + TenantID: roachpb.MakeTenantID(10), Stopper: ten10Stopper, }, ) restoreTenant10 := sqlutils.MakeSQLRunner(restoreConn10) @@ -7311,7 +7334,7 @@ func TestBackupRestoreTenant(t *testing.T) { ) _, restoreConn10 = serverutils.StartTenant( - t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10), Existing: true}, + t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10)}, ) defer restoreConn10.Close() restoreTenant10 = sqlutils.MakeSQLRunner(restoreConn10) @@ -7337,7 +7360,12 @@ func TestBackupRestoreTenant(t *testing.T) { t.Run("restore-t10-from-cluster-backup", func(t *testing.T) { restoreTC := testcluster.StartTestCluster( - t, singleNode, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ExternalIODir: dir}}, + t, singleNode, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ + ExternalIODir: dir, + // This test fails when run from a SQL server as it assumes that + // it's not running in SQL server. Tracked with #76378. + DisableDefaultSQLServer: true, + }}, ) defer restoreTC.Stopper().Stop(ctx) restoreDB := sqlutils.MakeSQLRunner(restoreTC.Conns[0]) @@ -7350,7 +7378,7 @@ func TestBackupRestoreTenant(t *testing.T) { ) _, restoreConn10 := serverutils.StartTenant( - t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10), Existing: true}, + t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10)}, ) defer restoreConn10.Close() restoreTenant10 := sqlutils.MakeSQLRunner(restoreConn10) @@ -7361,7 +7389,12 @@ func TestBackupRestoreTenant(t *testing.T) { t.Run("restore-all-from-cluster-backup", func(t *testing.T) { restoreTC := testcluster.StartTestCluster( - t, singleNode, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ExternalIODir: dir}}, + t, singleNode, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ + ExternalIODir: dir, + // This test fails when run from a SQL server as it assumes that + // it's not running in SQL server. Tracked with #76378. + DisableDefaultSQLServer: true, + }}, ) defer restoreTC.Stopper().Stop(ctx) @@ -7379,7 +7412,7 @@ func TestBackupRestoreTenant(t *testing.T) { ) _, restoreConn10 := serverutils.StartTenant( - t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10), Existing: true}, + t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10)}, ) defer restoreConn10.Close() restoreTenant10 := sqlutils.MakeSQLRunner(restoreConn10) @@ -7388,7 +7421,7 @@ func TestBackupRestoreTenant(t *testing.T) { restoreTenant10.CheckQueryResults(t, `select * from foo.bar2`, tenant10.QueryStr(t, `select * from foo.bar2`)) _, restoreConn11 := serverutils.StartTenant( - t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(11), Existing: true}, + t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(11)}, ) defer restoreConn11.Close() restoreTenant11 := sqlutils.MakeSQLRunner(restoreConn11) @@ -7398,7 +7431,12 @@ func TestBackupRestoreTenant(t *testing.T) { t.Run("restore-tenant10-to-ts1", func(t *testing.T) { restoreTC := testcluster.StartTestCluster( - t, singleNode, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ExternalIODir: dir}}, + t, singleNode, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ + ExternalIODir: dir, + // This test fails when run from a SQL server as it assumes that + // it's not running in SQL server. Tracked with #76378. + DisableDefaultSQLServer: true, + }}, ) defer restoreTC.Stopper().Stop(ctx) restoreDB := sqlutils.MakeSQLRunner(restoreTC.Conns[0]) @@ -7406,7 +7444,7 @@ func TestBackupRestoreTenant(t *testing.T) { restoreDB.Exec(t, `RESTORE TENANT 10 FROM 'nodelocal://1/t10' AS OF SYSTEM TIME `+ts1) _, restoreConn10 := serverutils.StartTenant( - t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10), Existing: true}, + t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10)}, ) defer restoreConn10.Close() restoreTenant10 := sqlutils.MakeSQLRunner(restoreConn10) @@ -7416,7 +7454,12 @@ func TestBackupRestoreTenant(t *testing.T) { t.Run("restore-tenant20-to-latest", func(t *testing.T) { restoreTC := testcluster.StartTestCluster( - t, singleNode, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ExternalIODir: dir}}, + t, singleNode, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ + ExternalIODir: dir, + // This test fails when run from a SQL server as it assumes that + // it's not running in SQL server. Tracked with #76378. + DisableDefaultSQLServer: true, + }}, ) defer restoreTC.Stopper().Stop(ctx) restoreDB := sqlutils.MakeSQLRunner(restoreTC.Conns[0]) @@ -7424,7 +7467,7 @@ func TestBackupRestoreTenant(t *testing.T) { restoreDB.Exec(t, `RESTORE TENANT 20 FROM 'nodelocal://1/t20'`) _, restoreConn20 := serverutils.StartTenant( - t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(20), Existing: true}, + t, restoreTC.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(20)}, ) defer restoreConn20.Close() restoreTenant20 := sqlutils.MakeSQLRunner(restoreConn20) @@ -8333,6 +8376,8 @@ func TestFullClusterTemporaryBackupAndRestore(t *testing.T) { defer dirCleanupFn() params := base.TestClusterArgs{} params.ServerArgs.ExternalIODir = dir + // This test fails when run from a SQL server. Tracked with #76378. + params.ServerArgs.DisableDefaultSQLServer = true params.ServerArgs.UseDatabase = "defaultdb" params.ServerArgs.Settings = settings knobs := base.TestingKnobs{ @@ -9095,6 +9140,11 @@ func TestGCDropIndexSpanExpansion(t *testing.T) { ctx := context.Background() tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ ExternalIODir: baseDir, + // This test hangs when run from a SQL server. It's likely that + // the cause of the hang is the fact that we're waiting on the GC to + // complete, and we don't have visibility into the GC completing from + // the tenant. More investigation is required. Tracked with #76378. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ GCJob: &sql.GCJobTestingKnobs{RunBeforePerformGC: func(id jobspb.JobID) error { gcJobID = id diff --git a/pkg/ccl/backupccl/create_scheduled_backup_test.go b/pkg/ccl/backupccl/create_scheduled_backup_test.go index 954c9cdba208..a91038d6bf76 100644 --- a/pkg/ccl/backupccl/create_scheduled_backup_test.go +++ b/pkg/ccl/backupccl/create_scheduled_backup_test.go @@ -89,6 +89,9 @@ func newTestHelper(t *testing.T) (*testHelper, func()) { args := base.TestServerArgs{ ExternalIODir: dir, + // Some scheduled backup tests fail when run from a SQL server. More + // investigation is required. Tracked with #76378. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ JobsTestingKnobs: knobs, }, diff --git a/pkg/ccl/backupccl/full_cluster_backup_restore_test.go b/pkg/ccl/backupccl/full_cluster_backup_restore_test.go index 1e6d52e4a88e..8835e1ec3a68 100644 --- a/pkg/ccl/backupccl/full_cluster_backup_restore_test.go +++ b/pkg/ccl/backupccl/full_cluster_backup_restore_test.go @@ -1056,7 +1056,10 @@ func TestRestoreWithRecreatedDefaultDB(t *testing.T) { defer log.Scope(t).Close(t) sqlDB, tempDir, cleanupFn := createEmptyCluster(t, singleNode) - _, sqlDBRestore, cleanupEmptyCluster := backupRestoreTestSetupEmpty(t, singleNode, tempDir, InitManualReplication, base.TestClusterArgs{}) + _, sqlDBRestore, cleanupEmptyCluster := backupRestoreTestSetupEmpty(t, singleNode, tempDir, InitManualReplication, + // Disabling the default SQL server due to test failures. More + // investigation is required. Tracked with #76378. + base.TestClusterArgs{ServerArgs: base.TestServerArgs{DisableDefaultSQLServer: true}}) defer cleanupFn() defer cleanupEmptyCluster() @@ -1081,7 +1084,10 @@ func TestRestoreWithDroppedDefaultDB(t *testing.T) { defer log.Scope(t).Close(t) sqlDB, tempDir, cleanupFn := createEmptyCluster(t, singleNode) - _, sqlDBRestore, cleanupEmptyCluster := backupRestoreTestSetupEmpty(t, singleNode, tempDir, InitManualReplication, base.TestClusterArgs{}) + _, sqlDBRestore, cleanupEmptyCluster := backupRestoreTestSetupEmpty(t, singleNode, tempDir, InitManualReplication, + // Disabling the default SQL server due to test failures. More + // investigation is required. Tracked with #76378. + base.TestClusterArgs{ServerArgs: base.TestServerArgs{DisableDefaultSQLServer: true}}) defer cleanupFn() defer cleanupEmptyCluster() diff --git a/pkg/ccl/backupccl/helpers_test.go b/pkg/ccl/backupccl/helpers_test.go new file mode 100644 index 000000000000..72ce123dd1d0 --- /dev/null +++ b/pkg/ccl/backupccl/helpers_test.go @@ -0,0 +1,415 @@ +// Copyright 2020 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 backupccl + +import ( + "context" + gosql "database/sql" + "fmt" + "io" + "io/ioutil" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/kv" + roachpb "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/desctestutils" + "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/protoutil" + "github.com/cockroachdb/cockroach/pkg/workload/bank" + "github.com/cockroachdb/cockroach/pkg/workload/workloadsql" + "github.com/cockroachdb/errors" + "github.com/cockroachdb/logtags" + "github.com/kr/pretty" + "github.com/stretchr/testify/require" +) + +const ( + singleNode = 1 + MultiNode = 3 + backupRestoreDefaultRanges = 10 + backupRestoreRowPayloadSize = 100 + LocalFoo = "nodelocal://0/foo" +) + +func backupRestoreTestSetupWithParams( + t testing.TB, + clusterSize int, + numAccounts int, + init func(tc *testcluster.TestCluster), + params base.TestClusterArgs, +) (tc *testcluster.TestCluster, sqlDB *sqlutils.SQLRunner, tempDir string, cleanup func()) { + ctx := logtags.AddTag(context.Background(), "backup-restore-test-setup", nil) + + dir, dirCleanupFn := testutils.TempDir(t) + params.ServerArgs.ExternalIODir = dir + params.ServerArgs.UseDatabase = "data" + // Need to disable the SQL server here. Below we're creating a database + // which gets used in various ways in different tests. One way it's used + // is to fetch the database's descriptor using TestingGetTableDescriptor + // which currently isn't multi-tenant enabled. The end result is that we + // can't find the created database and the test fails. Long term we should + // change TestingGetTableDescriptor so that it's multi-tenant enabled. + // Tracked with #76378. + params.ServerArgs.DisableDefaultSQLServer = true + if len(params.ServerArgsPerNode) > 0 { + for i := range params.ServerArgsPerNode { + param := params.ServerArgsPerNode[i] + param.ExternalIODir = dir + param.UseDatabase = "data" + param.DisableDefaultSQLServer = true + params.ServerArgsPerNode[i] = param + } + } + + tc = testcluster.StartTestCluster(t, clusterSize, params) + init(tc) + + const payloadSize = 100 + splits := 10 + if numAccounts == 0 { + splits = 0 + } + bankData := bank.FromConfig(numAccounts, numAccounts, payloadSize, splits) + + sqlDB = sqlutils.MakeSQLRunner(tc.Conns[0]) + + // Set the max buffer size to something low to prevent backup/restore tests + // from hitting OOM errors. If any test cares about this setting in + // particular, they will override it inline after setting up the test cluster. + sqlDB.Exec(t, `SET CLUSTER SETTING bulkio.backup.merge_file_buffer_size = '16MiB'`) + + sqlDB.Exec(t, `CREATE DATABASE data`) + l := workloadsql.InsertsDataLoader{BatchSize: 1000, Concurrency: 4} + if _, err := workloadsql.Setup(ctx, sqlDB.DB.(*gosql.DB), bankData, l); err != nil { + t.Fatalf("%+v", err) + } + + if err := tc.WaitForFullReplication(); err != nil { + t.Fatal(err) + } + + cleanupFn := func() { + tc.Stopper().Stop(ctx) // cleans up in memory storage's auxiliary dirs + dirCleanupFn() // cleans up dir, which is the nodelocal:// storage + } + + return tc, sqlDB, dir, cleanupFn +} + +func BackupRestoreTestSetup( + t testing.TB, clusterSize int, numAccounts int, init func(*testcluster.TestCluster), +) (tc *testcluster.TestCluster, sqlDB *sqlutils.SQLRunner, tempDir string, cleanup func()) { + return backupRestoreTestSetupWithParams(t, clusterSize, numAccounts, init, base.TestClusterArgs{}) +} + +func backupRestoreTestSetupEmpty( + t testing.TB, + clusterSize int, + tempDir string, + init func(*testcluster.TestCluster), + params base.TestClusterArgs, +) (tc *testcluster.TestCluster, sqlDB *sqlutils.SQLRunner, cleanup func()) { + return backupRestoreTestSetupEmptyWithParams(t, clusterSize, tempDir, init, params) +} + +func verifyBackupRestoreStatementResult( + t *testing.T, sqlDB *sqlutils.SQLRunner, query string, args ...interface{}, +) error { + t.Helper() + rows := sqlDB.Query(t, query, args...) + + columns, err := rows.Columns() + if err != nil { + return err + } + if e, a := columns, []string{ + "job_id", "status", "fraction_completed", "rows", "index_entries", "bytes", + }; !reflect.DeepEqual(e, a) { + return errors.Errorf("unexpected columns:\n%s", strings.Join(pretty.Diff(e, a), "\n")) + } + + type job struct { + id int64 + status string + fractionCompleted float32 + } + + var expectedJob job + var actualJob job + var unused int64 + + if !rows.Next() { + if err := rows.Err(); err != nil { + return err + } + return errors.New("zero rows in result") + } + if err := rows.Scan( + &actualJob.id, &actualJob.status, &actualJob.fractionCompleted, &unused, &unused, &unused, + ); err != nil { + return err + } + if rows.Next() { + return errors.New("more than one row in result") + } + + sqlDB.QueryRow(t, + `SELECT job_id, status, fraction_completed FROM crdb_internal.jobs WHERE job_id = $1`, actualJob.id, + ).Scan( + &expectedJob.id, &expectedJob.status, &expectedJob.fractionCompleted, + ) + + if e, a := expectedJob, actualJob; !reflect.DeepEqual(e, a) { + return errors.Errorf("result does not match system.jobs:\n%s", + strings.Join(pretty.Diff(e, a), "\n")) + } + + return nil +} + +func backupRestoreTestSetupEmptyWithParams( + t testing.TB, + clusterSize int, + dir string, + init func(tc *testcluster.TestCluster), + params base.TestClusterArgs, +) (tc *testcluster.TestCluster, sqlDB *sqlutils.SQLRunner, cleanup func()) { + ctx := logtags.AddTag(context.Background(), "backup-restore-test-setup-empty", nil) + + // Need to disable SQL server here. Much of the backup/restore tests + // perform validation of the restore by checking in the ranges directly. + // This is not supported from within a SQL server. Tracked with #76378. + params.ServerArgs.DisableDefaultSQLServer = true + params.ServerArgs.ExternalIODir = dir + if len(params.ServerArgsPerNode) > 0 { + for i := range params.ServerArgsPerNode { + param := params.ServerArgsPerNode[i] + param.ExternalIODir = dir + param.DisableDefaultSQLServer = true + params.ServerArgsPerNode[i] = param + } + } + tc = testcluster.StartTestCluster(t, clusterSize, params) + init(tc) + + sqlDB = sqlutils.MakeSQLRunner(tc.Conns[0]) + + cleanupFn := func() { + tc.Stopper().Stop(ctx) // cleans up in memory storage's auxiliary dirs + } + + return tc, sqlDB, cleanupFn +} + +func createEmptyCluster( + t testing.TB, clusterSize int, +) (sqlDB *sqlutils.SQLRunner, tempDir string, cleanup func()) { + ctx := context.Background() + + dir, dirCleanupFn := testutils.TempDir(t) + params := base.TestClusterArgs{} + params.ServerArgs.ExternalIODir = dir + // Disabling SQL server due to test failures. More investigation is + // required. Tracked with #76378. + params.ServerArgs.DisableDefaultSQLServer = true + tc := testcluster.StartTestCluster(t, clusterSize, params) + + sqlDB = sqlutils.MakeSQLRunner(tc.Conns[0]) + + cleanupFn := func() { + tc.Stopper().Stop(ctx) // cleans up in memory storage's auxiliary dirs + dirCleanupFn() // cleans up dir, which is the nodelocal:// storage + } + + return sqlDB, dir, cleanupFn +} + +// getStatsQuery returns a SQL query that will return the properties of the +// statistics on a table that are expected to remain the same after being +// restored on a new cluster. +func getStatsQuery(tableName string) string { + return fmt.Sprintf(`SELECT + statistics_name, + column_names, + row_count, + distinct_count, + null_count + FROM [SHOW STATISTICS FOR TABLE %s]`, tableName) +} + +// injectStats directly injects some arbitrary statistic into a given table for +// a specified column. +// See injectStatsWithRowCount. +func injectStats( + t *testing.T, sqlDB *sqlutils.SQLRunner, tableName string, columnName string, +) [][]string { + return injectStatsWithRowCount(t, sqlDB, tableName, columnName, 100 /* rowCount */) +} + +// injectStatsWithRowCount directly injects some statistics specifying some row +// count for a column in the given table. +// N.B. This should be used in backup testing over CREATE STATISTICS since it +// ensures that the stats cache will be up to date during a subsequent BACKUP. +func injectStatsWithRowCount( + t *testing.T, sqlDB *sqlutils.SQLRunner, tableName string, columnName string, rowCount int, +) [][]string { + sqlDB.Exec(t, fmt.Sprintf(`ALTER TABLE %s INJECT STATISTICS '[ + { + "columns": ["%s"], + "created_at": "2018-01-01 1:00:00.00000+00:00", + "row_count": %d, + "distinct_count": %d + } + ]'`, tableName, columnName, rowCount, rowCount)) + return sqlDB.QueryStr(t, getStatsQuery(tableName)) +} + +func makeInsecureHTTPServer(t *testing.T) (string, func()) { + t.Helper() + + const badHeadResponse = "bad-head-response" + + tmp, dirCleanup := testutils.TempDir(t) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + localfile := filepath.Join(tmp, filepath.Base(r.URL.Path)) + switch r.Method { + case "PUT": + f, err := os.Create(localfile) + if err != nil { + http.Error(w, err.Error(), 500) + return + } + defer f.Close() + if _, err := io.Copy(f, r.Body); err != nil { + http.Error(w, err.Error(), 500) + return + } + w.WriteHeader(201) + case "GET", "HEAD": + if filepath.Base(localfile) == badHeadResponse { + http.Error(w, "HEAD not implemented", 500) + return + } + http.ServeFile(w, r, localfile) + case "DELETE": + if err := os.Remove(localfile); err != nil { + http.Error(w, err.Error(), 500) + return + } + w.WriteHeader(204) + default: + http.Error(w, "unsupported method "+r.Method, 400) + } + })) + + cleanup := func() { + srv.Close() + dirCleanup() + } + + t.Logf("Mock HTTP Storage %q", srv.URL) + uri, err := url.Parse(srv.URL) + if err != nil { + srv.Close() + t.Fatal(err) + } + uri.Path = filepath.Join(uri.Path, "testing") + return uri.String(), cleanup +} + +// thresholdBlocker is a small wrapper around channels that are commonly used to +// block operations during testing. +// For example, it can be used in conjection with the RunBeforeBackfillChunk and +// BulkAdderFlushesEveryBatch cluster settings. The SQLSchemaChanger knob can be +// used to control the chunk size. +type thresholdBlocker struct { + threshold int + reachedThreshold chan struct{} + canProceed chan struct{} +} + +func (t thresholdBlocker) maybeBlock(count int) { + if count == t.threshold { + close(t.reachedThreshold) + <-t.canProceed + } +} + +func (t thresholdBlocker) waitUntilBlocked() { + <-t.reachedThreshold +} + +func (t thresholdBlocker) allowToProceed() { + close(t.canProceed) +} + +func makeThresholdBlocker(threshold int) thresholdBlocker { + return thresholdBlocker{ + threshold: threshold, + reachedThreshold: make(chan struct{}), + canProceed: make(chan struct{}), + } +} + +// getSpansFromManifest returns the spans that describe the data included in a +// given backup. +func getSpansFromManifest(ctx context.Context, t *testing.T, backupPath string) roachpb.Spans { + backupManifestBytes, err := ioutil.ReadFile(backupPath + "/" + backupManifestName) + require.NoError(t, err) + var backupManifest BackupManifest + decompressedBytes, err := decompressData(ctx, nil, backupManifestBytes) + require.NoError(t, err) + require.NoError(t, protoutil.Unmarshal(decompressedBytes, &backupManifest)) + spans := make([]roachpb.Span, 0, len(backupManifest.Files)) + for _, file := range backupManifest.Files { + spans = append(spans, file.Span) + } + mergedSpans, _ := roachpb.MergeSpans(&spans) + return mergedSpans +} + +func getKVCount(ctx context.Context, kvDB *kv.DB, dbName, tableName string) (int, error) { + tableDesc := desctestutils.TestingGetPublicTableDescriptor(kvDB, keys.SystemSQLCodec, dbName, tableName) + tablePrefix := keys.SystemSQLCodec.TablePrefix(uint32(tableDesc.GetID())) + tableEnd := tablePrefix.PrefixEnd() + kvs, err := kvDB.Scan(ctx, tablePrefix, tableEnd, 0) + return len(kvs), err +} + +// uriFmtStringAndArgs returns format strings like "$1" or "($1, $2, $3)" and +// an []interface{} of URIs for the BACKUP/RESTORE queries. +func uriFmtStringAndArgs(uris []string) (string, []interface{}) { + urisForFormat := make([]interface{}, len(uris)) + var fmtString strings.Builder + if len(uris) > 1 { + fmtString.WriteString("(") + } + for i, uri := range uris { + if i > 0 { + fmtString.WriteString(", ") + } + fmtString.WriteString(fmt.Sprintf("$%d", i+1)) + urisForFormat[i] = uri + } + if len(uris) > 1 { + fmtString.WriteString(")") + } + return fmtString.String(), urisForFormat +} diff --git a/pkg/ccl/backupccl/insert_missing_public_schema_namespace_entry_restore_test.go b/pkg/ccl/backupccl/insert_missing_public_schema_namespace_entry_restore_test.go index 7206c1e5815d..7f566d000f55 100644 --- a/pkg/ccl/backupccl/insert_missing_public_schema_namespace_entry_restore_test.go +++ b/pkg/ccl/backupccl/insert_missing_public_schema_namespace_entry_restore_test.go @@ -34,7 +34,10 @@ func TestInsertMissingPublicSchemaNamespaceEntry(t *testing.T) { defer cleanup() tc := testcluster.StartTestCluster(t, 3, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - ExternalIODir: dir, + // Disabling the default SQL server due to test failures. More + // investigation is required. Tracked with #76378. + DisableDefaultSQLServer: true, + ExternalIODir: dir, Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ DisableAutomaticVersionUpgrade: 1, diff --git a/pkg/ccl/backupccl/restore_mid_schema_change_test.go b/pkg/ccl/backupccl/restore_mid_schema_change_test.go index 89da8db3eadc..c59b58c3c031 100644 --- a/pkg/ccl/backupccl/restore_mid_schema_change_test.go +++ b/pkg/ccl/backupccl/restore_mid_schema_change_test.go @@ -235,7 +235,12 @@ func restoreMidSchemaChange( params := base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ ExternalIODir: dir, - Knobs: base.TestingKnobs{JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals()}, + // This test fails when run with the default SQL server because + // it relies on TestingGetTableDescriptor which isn't supported + // in multi-tenancy. More work is required here. Tracked with + // #76378. + DisableDefaultSQLServer: true, + Knobs: base.TestingKnobs{JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals()}, }, } tc := testcluster.StartTestCluster(t, singleNode, params) diff --git a/pkg/ccl/backupccl/restore_old_versions_test.go b/pkg/ccl/backupccl/restore_old_versions_test.go index f9d20a2b9d37..844daa93b5b3 100644 --- a/pkg/ccl/backupccl/restore_old_versions_test.go +++ b/pkg/ccl/backupccl/restore_old_versions_test.go @@ -159,7 +159,10 @@ func TestRestoreOldVersions(t *testing.T) { ctx := context.Background() tc := testcluster.StartTestCluster(t, singleNode, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - ExternalIODir: externalDir, + // Disabling the SQL server due to test cases failures. + // More investigation is required. Tracked with #76378. + DisableDefaultSQLServer: true, + ExternalIODir: externalDir, }, }) sqlDB := sqlutils.MakeSQLRunner(tc.Conns[0]) @@ -504,7 +507,10 @@ func restoreOldVersionClusterTest(exportDir string) func(t *testing.T) { ctx := context.Background() tc := testcluster.StartTestCluster(t, singleNode, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - ExternalIODir: externalDir, + // Disabling the SQL server due to test failures. More + // investigation is required. Tracked with #76378. + DisableDefaultSQLServer: true, + ExternalIODir: externalDir, }, }) sqlDB := sqlutils.MakeSQLRunner(tc.Conns[0]) @@ -676,7 +682,9 @@ func TestRestoreOldBackupMissingOfflineIndexes(t *testing.T) { badBackups, err := filepath.Abs(testutils.TestDataPath(t, "restore_old_versions", "inc_missing_addsst", "v20.2.7")) require.NoError(t, err) - args := base.TestServerArgs{ExternalIODir: badBackups} + // Disabling the SQL server due to test cases failures. More + // investigation is required. Tracked with #76378. + args := base.TestServerArgs{ExternalIODir: badBackups, DisableDefaultSQLServer: true} backupDirs := make([]string, 9) for i := range backupDirs { backupDirs[i] = fmt.Sprintf("'nodelocal://0/%d'", i) @@ -753,7 +761,15 @@ func TestRestoreWithDroppedSchemaCorruption(t *testing.T) { fromDir = "nodelocal://0/" ) - args := base.TestServerArgs{ExternalIODir: backupDir} + args := base.TestServerArgs{ + ExternalIODir: backupDir, + // Disabling SQL server because this test case traps when run + // from the SQL server. The problem occurs because we try to + // reference a nil pointer below where we're expecting a database + // descriptor to exist. More investigation is required. + // Tracked with #76378. + DisableDefaultSQLServer: true, + } s, sqlDB, _ := serverutils.StartServer(t, args) tdb := sqlutils.MakeSQLRunner(sqlDB) defer s.Stopper().Stop(ctx) diff --git a/pkg/ccl/backupccl/system_schema_test.go b/pkg/ccl/backupccl/system_schema_test.go index bb4a84831405..43886cd4b686 100644 --- a/pkg/ccl/backupccl/system_schema_test.go +++ b/pkg/ccl/backupccl/system_schema_test.go @@ -26,7 +26,13 @@ func TestAllSystemTablesHaveBackupConfig(t *testing.T) { defer log.Scope(t).Close(t) ctx := context.Background() - tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{}) + tc := testcluster.StartTestCluster(t, 1, + base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + // Disabling the SQL server due to test failures. More + // investigation is required. Tracked with #76378. + DisableDefaultSQLServer: true, + }}) defer tc.Stopper().Stop(ctx) sqlDB := sqlutils.MakeSQLRunner(tc.Conns[0]) diff --git a/pkg/ccl/changefeedccl/changefeed_test.go b/pkg/ccl/changefeedccl/changefeed_test.go index 7be60d2a2e17..7c59926060d1 100644 --- a/pkg/ccl/changefeedccl/changefeed_test.go +++ b/pkg/ccl/changefeedccl/changefeed_test.go @@ -4308,9 +4308,12 @@ func TestChangefeedHandlesDrainingNodes(t *testing.T) { tc := serverutils.StartNewTestCluster(t, 4, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - UseDatabase: "test", - Knobs: knobs, - ExternalIODir: sinkDir, + // Test uses SPLIT AT, which isn't currently supported for + // non-system SQL servers. + DisableDefaultSQLServer: true, + UseDatabase: "test", + Knobs: knobs, + ExternalIODir: sinkDir, }}) defer tc.Stopper().Stop(context.Background()) @@ -5077,6 +5080,8 @@ func TestDistSenderRangeFeedPopulatesVirtualTable(t *testing.T) { defer log.Scope(t).Close(t) s, db, _ := serverutils.StartServer(t, base.TestServerArgs{ + // Test fails with SQL server. More investigation is required. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(), }, diff --git a/pkg/ccl/changefeedccl/helpers_test.go b/pkg/ccl/changefeedccl/helpers_test.go index 3fb7327229ba..56e2b34aa284 100644 --- a/pkg/ccl/changefeedccl/helpers_test.go +++ b/pkg/ccl/changefeedccl/helpers_test.go @@ -333,9 +333,12 @@ func startTestFullServer( options.knobsFn(&knobs) } args := base.TestServerArgs{ - Knobs: knobs, - UseDatabase: `d`, - ExternalIODir: options.externalIODir, + Knobs: knobs, + // This test suite is already probabilistically running with SQL + // servers. No need for the SQL server. + DisableDefaultSQLServer: true, + UseDatabase: `d`, + ExternalIODir: options.externalIODir, } if options.argsFn != nil { @@ -481,7 +484,7 @@ func withKnobsFn(fn updateKnobsFn) feedTestOption { var _ = withKnobsFn(nil /* fn */) func newTestOptions() feedTestOptions { - // percentTenant is the percentange of tests that will be run against + // percentTenant is the percentage of tests that will be run against // a SQL-node in a multi-tenant server. 1 for all tests to be run on a // tenant. const percentTenant = 0.25 diff --git a/pkg/ccl/cliccl/debug_backup_test.go b/pkg/ccl/cliccl/debug_backup_test.go index e28512f20a56..2171b7fc599d 100644 --- a/pkg/ccl/cliccl/debug_backup_test.go +++ b/pkg/ccl/cliccl/debug_backup_test.go @@ -539,7 +539,13 @@ func TestExportDataAOST(t *testing.T) { ctx := context.Background() dir, cleanFn := testutils.TempDir(t) defer cleanFn() - srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{ExternalIODir: dir, Insecure: true}) + srv, db, _ := serverutils.StartServer(t, + base.TestServerArgs{ + ExternalIODir: dir, + Insecure: true, + // Have to disable testing in MT mode until backups with revision + // history are supported for SQL servers. + DisableDefaultSQLServer: true}) defer srv.Stopper().Stop(ctx) sqlDB := sqlutils.MakeSQLRunner(db) @@ -767,7 +773,13 @@ func TestExportDataWithRevisions(t *testing.T) { ctx := context.Background() dir, cleanFn := testutils.TempDir(t) defer cleanFn() - srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{ExternalIODir: dir, Insecure: true}) + srv, db, _ := serverutils.StartServer(t, + base.TestServerArgs{ + ExternalIODir: dir, + Insecure: true, + // Have to disable testing in MT mode until backups with revision + // history are supported for SQL servers. + DisableDefaultSQLServer: true}) defer srv.Stopper().Stop(ctx) sqlDB := sqlutils.MakeSQLRunner(db) diff --git a/pkg/ccl/importccl/exportcsv_test.go b/pkg/ccl/importccl/exportcsv_test.go index ba425cc0f192..a9d53bc468c3 100644 --- a/pkg/ccl/importccl/exportcsv_test.go +++ b/pkg/ccl/importccl/exportcsv_test.go @@ -48,9 +48,11 @@ func setupExportableBank(t *testing.T, nodes, rows int) (*sqlutils.SQLRunner, st tc := testcluster.StartTestCluster(t, nodes, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - ExternalIODir: dir, - UseDatabase: "test", - DisableSpanConfigs: true, + // Disabled due to underlying tests' use of SCATTER. + DisableDefaultSQLServer: true, + ExternalIODir: dir, + UseDatabase: "test", + DisableSpanConfigs: true, }, }, ) diff --git a/pkg/ccl/importccl/exportparquet_test.go b/pkg/ccl/importccl/exportparquet_test.go index 04f96bafbd26..0e622e1865c1 100644 --- a/pkg/ccl/importccl/exportparquet_test.go +++ b/pkg/ccl/importccl/exportparquet_test.go @@ -175,8 +175,10 @@ func TestRandomParquetExports(t *testing.T) { params := base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - UseDatabase: dbName, - ExternalIODir: dir, + // Test fails under default tenant, more investigation is required. + DisableDefaultSQLServer: true, + UseDatabase: dbName, + ExternalIODir: dir, }, } ctx := context.Background() @@ -240,8 +242,10 @@ func TestBasicParquetTypes(t *testing.T) { dbName := "baz" params := base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - UseDatabase: dbName, - ExternalIODir: dir, + // Test fails under default tenant, more investigation is required. + DisableDefaultSQLServer: true, + UseDatabase: dbName, + ExternalIODir: dir, }, } ctx := context.Background() diff --git a/pkg/ccl/importccl/import_into_test.go b/pkg/ccl/importccl/import_into_test.go index 8f3517b2bc05..27301736a8f1 100644 --- a/pkg/ccl/importccl/import_into_test.go +++ b/pkg/ccl/importccl/import_into_test.go @@ -55,7 +55,12 @@ func TestProtectedTimestampsDuringImportInto(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - args := base.TestClusterArgs{} + args := base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + // Test hangs under SQL server. More investigation is required. + DisableDefaultSQLServer: true, + }, + } tc := testcluster.StartTestCluster(t, 1, args) defer tc.Stopper().Stop(ctx) diff --git a/pkg/ccl/importccl/import_processor_test.go b/pkg/ccl/importccl/import_processor_test.go index b38d1ca4e8e9..b6a53e9d91b9 100644 --- a/pkg/ccl/importccl/import_processor_test.go +++ b/pkg/ccl/importccl/import_processor_test.go @@ -646,6 +646,9 @@ func TestCSVImportCanBeResumed(t *testing.T) { s, db, _ := serverutils.StartServer(t, base.TestServerArgs{ + // Hangs when run from the SQL server. More investigation is + // required here. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(), DistSQL: &execinfra.TestingKnobs{ @@ -751,6 +754,9 @@ func TestCSVImportMarksFilesFullyProcessed(t *testing.T) { s, db, _ := serverutils.StartServer(t, base.TestServerArgs{ + // Test hangs when run under the SQL server. More investigation + // is required here. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(), DistSQL: &execinfra.TestingKnobs{ diff --git a/pkg/ccl/importccl/import_stmt_test.go b/pkg/ccl/importccl/import_stmt_test.go index 6c502b67c408..8f9c40ac6876 100644 --- a/pkg/ccl/importccl/import_stmt_test.go +++ b/pkg/ccl/importccl/import_stmt_test.go @@ -1877,8 +1877,12 @@ func TestFailedImportGC(t *testing.T) { ctx := context.Background() baseDir := testutils.TestDataPath(t, "pgdump") tc := testcluster.StartTestCluster(t, nodes, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ - SQLMemoryPoolSize: 256 << 20, - ExternalIODir: baseDir, + // Test fails with the SQL server. This may be because we're trying + // to access files in nodelocal://1, which is off node. More + // investigation is required. Tracked with #76378. + DisableDefaultSQLServer: true, + SQLMemoryPoolSize: 256 << 20, + ExternalIODir: baseDir, Knobs: base.TestingKnobs{ GCJob: &sql.GCJobTestingKnobs{RunBeforeResume: func(_ jobspb.JobID) error { <-blockGC; return nil }}, }, @@ -1974,10 +1978,15 @@ func TestImportCSVStmt(t *testing.T) { ctx := context.Background() baseDir := testutils.TestDataPath(t, "csv") - tc := testcluster.StartTestCluster(t, nodes, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ - SQLMemoryPoolSize: 256 << 20, - ExternalIODir: baseDir, - }}) + tc := testcluster.StartTestCluster(t, nodes, + base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + // Test fails when run under the SQL server. More + // investigation is required. Tracked with #76378. + DisableDefaultSQLServer: true, + SQLMemoryPoolSize: 256 << 20, + ExternalIODir: baseDir, + }}) defer tc.Stopper().Stop(ctx) conn := tc.Conns[0] @@ -2525,8 +2534,11 @@ func TestImportObjectLevelRBAC(t *testing.T) { ctx := context.Background() baseDir := testutils.TestDataPath(t, "pgdump") tc := testcluster.StartTestCluster(t, nodes, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ - ExternalIODir: baseDir, - SQLMemoryPoolSize: 256 << 20, + // Test fails when run under the SQL server. More investigation + // is required. Tracked with #76378. + DisableDefaultSQLServer: true, + ExternalIODir: baseDir, + SQLMemoryPoolSize: 256 << 20, }}) defer tc.Stopper().Stop(ctx) conn := tc.Conns[0] @@ -2791,7 +2803,11 @@ func TestImportIntoCSV(t *testing.T) { ctx := context.Background() baseDir := testutils.TestDataPath(t, "csv") - tc := testcluster.StartTestCluster(t, nodes, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ExternalIODir: baseDir}}) + tc := testcluster.StartTestCluster(t, nodes, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ + // Test fails when run under the SQL server. More investigation + // is required. Tracked with #76378. + DisableDefaultSQLServer: true, + ExternalIODir: baseDir}}) defer tc.Stopper().Stop(ctx) conn := tc.Conns[0] @@ -4389,6 +4405,9 @@ func TestImportDefaultWithResume(t *testing.T) { s, db, _ := serverutils.StartServer(t, base.TestServerArgs{ + // Test hangs when run with the SQL server. More investigation + // is required. Tracked with #76378. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(), DistSQL: &execinfra.TestingKnobs{ @@ -4903,7 +4922,13 @@ func TestImportControlJobRBAC(t *testing.T) { defer jobs.ResetConstructors()() ctx := context.Background() - tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{}) + tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + // Test fails when run under the SQL server. More investigation + // is required. Tracked with #76378. + DisableDefaultSQLServer: true, + }, + }) defer tc.Stopper().Stop(ctx) rootDB := sqlutils.MakeSQLRunner(tc.Conns[0]) @@ -6051,6 +6076,9 @@ func TestImportPgDumpSchemas(t *testing.T) { t.Run("inject-error-ensure-cleanup", func(t *testing.T) { defer gcjob.SetSmallMaxGCIntervalForTest()() + // Test fails with the SQL server. More investigation is required. + // Tracked with #76378. + args.DisableDefaultSQLServer = true tc := testcluster.StartTestCluster(t, nodes, base.TestClusterArgs{ServerArgs: args}) defer tc.Stopper().Stop(ctx) conn := tc.Conns[0] @@ -6875,6 +6903,9 @@ func TestImportInTenant(t *testing.T) { ctx := context.Background() baseDir := testutils.TestDataPath(t) args := base.TestServerArgs{ExternalIODir: baseDir} + // Test fails with the SQL server. More investigation is required. + // Tracked with #76378. + args.DisableDefaultSQLServer = true tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ServerArgs: args}) defer tc.Stopper().Stop(ctx) conn := tc.Conns[0] @@ -7067,6 +7098,9 @@ func TestImportJobEventLogging(t *testing.T) { ctx := context.Background() baseDir := testutils.TestDataPath(t, "avro") args := base.TestServerArgs{ExternalIODir: baseDir} + // Test fails with the SQL server. More investigation is required. + // Tracked with #76378. + args.DisableDefaultSQLServer = true args.Knobs = base.TestingKnobs{JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals()} params := base.TestClusterArgs{ServerArgs: args} tc := testcluster.StartTestCluster(t, nodes, params) @@ -7378,6 +7412,9 @@ func TestImportMixedVersion(t *testing.T) { tc := testcluster.StartTestCluster( t, 1, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ + // Test fails with the SQL server. More investigation is + // required. Tracked with #76378. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(), Server: &server.TestingKnobs{ diff --git a/pkg/ccl/importccl/read_import_mysql_test.go b/pkg/ccl/importccl/read_import_mysql_test.go index b523c3577757..792b018ac92c 100644 --- a/pkg/ccl/importccl/read_import_mysql_test.go +++ b/pkg/ccl/importccl/read_import_mysql_test.go @@ -129,7 +129,10 @@ func readMysqlCreateFrom( walltime := testEvalCtx.StmtTimestamp.UnixNano() s, conn, _ := serverutils.StartServer(t, base.TestServerArgs{ - Settings: cluster.MakeTestingClusterSettings(), + // Test relies on descriptor validation, which doesn't seem to be + // supported within SQL servers. Tracked with #76378. + DisableDefaultSQLServer: true, + Settings: cluster.MakeTestingClusterSettings(), }) ctx := context.Background() defer s.Stopper().Stop(ctx) diff --git a/pkg/ccl/jobsccl/jobsprotectedtsccl/jobs_protected_ts_test.go b/pkg/ccl/jobsccl/jobsprotectedtsccl/jobs_protected_ts_test.go index 587ab90bf6b6..c463e0a0e105 100644 --- a/pkg/ccl/jobsccl/jobsprotectedtsccl/jobs_protected_ts_test.go +++ b/pkg/ccl/jobsccl/jobsprotectedtsccl/jobs_protected_ts_test.go @@ -139,6 +139,9 @@ func TestJobsProtectedTimestamp(t *testing.T) { ctx := context.Background() tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ + // Tests fail with SQL server. Disabling until we can + // investigate further. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ ProtectedTS: &protectedts.TestingKnobs{ EnableProtectedTimestampForMultiTenant: true, @@ -257,6 +260,9 @@ func TestSchedulesProtectedTimestamp(t *testing.T) { ctx := context.Background() tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ + // Test fails with SQL server. Disabling pending further + // investigation. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ ProtectedTS: &protectedts.TestingKnobs{ EnableProtectedTimestampForMultiTenant: true, diff --git a/pkg/ccl/kvccl/kvtenantccl/tenant_kv_test.go b/pkg/ccl/kvccl/kvtenantccl/tenant_kv_test.go index ced390c5c1e8..b7d3681be35a 100644 --- a/pkg/ccl/kvccl/kvtenantccl/tenant_kv_test.go +++ b/pkg/ccl/kvccl/kvtenantccl/tenant_kv_test.go @@ -33,6 +33,9 @@ func TestTenantRangeQPSStat(t *testing.T) { tc := serverutils.StartNewTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ DisableWebSessionAuthentication: true, + // Must disable SQL server because test below assumes that + // it is connecting to the host tenant. + DisableDefaultSQLServer: true, }, }) defer tc.Stopper().Stop(ctx) diff --git a/pkg/ccl/kvccl/kvtenantccl/tenant_trace_test.go b/pkg/ccl/kvccl/kvtenantccl/tenant_trace_test.go index 4399ccdba441..00e083063af0 100644 --- a/pkg/ccl/kvccl/kvtenantccl/tenant_trace_test.go +++ b/pkg/ccl/kvccl/kvtenantccl/tenant_trace_test.go @@ -52,6 +52,8 @@ func testTenantTracesAreRedactedImpl(t *testing.T, redactable bool) { recCh := make(chan tracing.Recording, 1) args := base.TestServerArgs{ + // Test hangs with SQL server. More investigation is required. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ Store: &kvserver.StoreTestingKnobs{ EvalKnobs: kvserverbase.BatchEvalTestingKnobs{ diff --git a/pkg/ccl/kvccl/kvtenantccl/tenant_upgrade_test.go b/pkg/ccl/kvccl/kvtenantccl/tenant_upgrade_test.go index a3dc2f3b8804..a0aedab3851f 100644 --- a/pkg/ccl/kvccl/kvtenantccl/tenant_upgrade_test.go +++ b/pkg/ccl/kvccl/kvtenantccl/tenant_upgrade_test.go @@ -55,7 +55,9 @@ func TestTenantUpgrade(t *testing.T) { clusterversion.TestingBinaryMinSupportedVersion, &settings.SV)) tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - Settings: settings, + // Test validates tenant behavior. No need for a SQL server. + DisableDefaultSQLServer: true, + Settings: settings, Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ DisableAutomaticVersionUpgrade: 1, @@ -129,7 +131,6 @@ func TestTenantUpgrade(t *testing.T) { { tenantServer, err := tc.Server(0).StartTenant(ctx, base.TestTenantArgs{ TenantID: roachpb.MakeTenantID(initialTenantID), - Existing: true, }) require.NoError(t, err) initialTenant, cleanup = connectToTenant(t, tenantServer.SQLAddr()) @@ -154,7 +155,6 @@ func TestTenantUpgrade(t *testing.T) { { tenantServer, err := tc.Server(0).StartTenant(ctx, base.TestTenantArgs{ TenantID: roachpb.MakeTenantID(postUpgradeTenantID), - Existing: true, }) require.NoError(t, err) postUpgradeTenant, cleanup = connectToTenant(t, tenantServer.SQLAddr()) @@ -207,7 +207,9 @@ func TestTenantUpgradeFailure(t *testing.T) { // Initialize the version to the BinaryMinSupportedVersion. tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - Settings: settings, + // Test validates tenant behavior. No need for a SQL server. + DisableDefaultSQLServer: true, + Settings: settings, Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ DisableAutomaticVersionUpgrade: 1, @@ -222,7 +224,6 @@ func TestTenantUpgradeFailure(t *testing.T) { startAndConnectToTenant := func(t *testing.T, tenantInfo *tenantInfo) (_ *gosql.DB, cleanup func()) { tenant, err := tc.Server(0).StartTenant(ctx, *tenantInfo.tenantArgs) require.NoError(t, err) - tenantInfo.tenantArgs.Existing = true pgURL, cleanupPGUrl := sqlutils.PGUrl(t, tenant.SQLAddr(), "Tenant", url.User(security.RootUser)) tenantDB, err := gosql.Open("postgres", pgURL.String()) require.NoError(t, err) @@ -231,7 +232,7 @@ func TestTenantUpgradeFailure(t *testing.T) { cleanupPGUrl() } } - mkTenant := func(t *testing.T, id uint64, existing bool) *tenantInfo { + mkTenant := func(t *testing.T, id uint64) *tenantInfo { settings := cluster.MakeTestingClusterSettingsWithVersions( v2, v0, @@ -244,7 +245,6 @@ func TestTenantUpgradeFailure(t *testing.T) { tenantArgs := base.TestTenantArgs{ Stopper: v2onMigrationStopper, TenantID: roachpb.MakeTenantID(id), - Existing: existing, TestingKnobs: base.TestingKnobs{ MigrationManager: &migration.TestingKnobs{ ListBetweenOverride: func(from, to clusterversion.ClusterVersion) []clusterversion.ClusterVersion { @@ -288,7 +288,7 @@ func TestTenantUpgradeFailure(t *testing.T) { t.Run("upgrade tenant have it crash then resume", func(t *testing.T) { // Create a tenant before upgrading anything and verify its version. const initialTenantID = 10 - tenantInfo := mkTenant(t, initialTenantID, false /*existing*/) + tenantInfo := mkTenant(t, initialTenantID) tenant, cleanup := startAndConnectToTenant(t, tenantInfo) initialTenantRunner := sqlutils.MakeSQLRunner(tenant) // Ensure that the tenant works. @@ -319,7 +319,7 @@ func TestTenantUpgradeFailure(t *testing.T) { clusterversion.TestingBinaryVersion.String()) <-waitForTenantClose cleanup() - tenantInfo = mkTenant(t, initialTenantID, true /*existing*/) + tenantInfo = mkTenant(t, initialTenantID) tenant, cleanup = startAndConnectToTenant(t, tenantInfo) initialTenantRunner = sqlutils.MakeSQLRunner(tenant) // Ensure that the tenant still works and the target diff --git a/pkg/ccl/logictestccl/testdata/logic_test/multi_region_backup b/pkg/ccl/logictestccl/testdata/logic_test/multi_region_backup index dee4eddaa12e..b48bbd0ed80e 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/multi_region_backup +++ b/pkg/ccl/logictestccl/testdata/logic_test/multi_region_backup @@ -375,9 +375,9 @@ ALTER DATABASE "mr-backup-1" CONFIGURE ZONE USING gc.ttlseconds = 1; ALTER DATABASE "mr-backup-2" CONFIGURE ZONE USING gc.ttlseconds = 1 statement ok -BACKUP DATABASE "mr-backup-1" TO 'nodelocal://1/mr-backup-1/'; -BACKUP DATABASE "mr-backup-2" TO 'nodelocal://1/mr-backup-2/'; -BACKUP DATABASE "mr-backup-1", "mr-backup-2" TO 'nodelocal://1/mr-backup-combined/' +BACKUP DATABASE "mr-backup-1" TO 'nodelocal://0/mr-backup-1/'; +BACKUP DATABASE "mr-backup-2" TO 'nodelocal://0/mr-backup-2/'; +BACKUP DATABASE "mr-backup-1", "mr-backup-2" TO 'nodelocal://0/mr-backup-combined/' query T select database_name from [show databases] @@ -402,7 +402,7 @@ system test statement ok -RESTORE DATABASE "mr-backup-1" FROM 'nodelocal://1/mr-backup-1/' +RESTORE DATABASE "mr-backup-1" FROM 'nodelocal://0/mr-backup-1/' query T select database_name from [show databases] @@ -556,7 +556,7 @@ TABLE regional_by_table_in_ca_central_1 ALTER TABLE regional_by_table_in_ca_cen lease_preferences = '[[+region=ca-central-1]]' statement ok -RESTORE DATABASE "mr-backup-2" FROM 'nodelocal://1/mr-backup-2/' +RESTORE DATABASE "mr-backup-2" FROM 'nodelocal://0/mr-backup-2/' query T select database_name from [show databases] @@ -723,7 +723,7 @@ system test statement ok -RESTORE DATABASE "mr-backup-1", "mr-backup-2" FROM 'nodelocal://1/mr-backup-combined/' +RESTORE DATABASE "mr-backup-1", "mr-backup-2" FROM 'nodelocal://0/mr-backup-combined/' query T select database_name from [show databases] @@ -1023,16 +1023,16 @@ TABLE regional_by_table_in_ca_central_1 ALTER TABLE regional_by_table_in_ca_cen subtest multiregion_table_backup_and_restore statement ok -BACKUP TABLE regional_by_row_table TO 'nodelocal://1/rbr_table/'; +BACKUP TABLE regional_by_row_table TO 'nodelocal://0/rbr_table/'; statement ok -BACKUP TABLE regional_by_table_in_primary_region TO 'nodelocal://1/rbt_table_in_primary_region/'; +BACKUP TABLE regional_by_table_in_primary_region TO 'nodelocal://0/rbt_table_in_primary_region/'; statement ok -BACKUP TABLE regional_by_table_in_ca_central_1 TO 'nodelocal://1/rbt_table_in_ca_central_1/'; +BACKUP TABLE regional_by_table_in_ca_central_1 TO 'nodelocal://0/rbt_table_in_ca_central_1/'; statement ok -BACKUP TABLE global_table TO 'nodelocal://1/global_table/'; +BACKUP TABLE global_table TO 'nodelocal://0/global_table/'; statement ok DROP TABLE regional_by_row_table; @@ -1041,16 +1041,16 @@ DROP TABLE regional_by_table_in_ca_central_1; DROP TABLE global_table; statement ok -RESTORE TABLE regional_by_row_table FROM 'nodelocal://1/rbr_table/'; +RESTORE TABLE regional_by_row_table FROM 'nodelocal://0/rbr_table/'; statement ok -RESTORE TABLE regional_by_table_in_primary_region FROM 'nodelocal://1/rbt_table_in_primary_region/'; +RESTORE TABLE regional_by_table_in_primary_region FROM 'nodelocal://0/rbt_table_in_primary_region/'; statement ok -RESTORE TABLE regional_by_table_in_ca_central_1 FROM 'nodelocal://1/rbt_table_in_ca_central_1/'; +RESTORE TABLE regional_by_table_in_ca_central_1 FROM 'nodelocal://0/rbt_table_in_ca_central_1/'; statement ok -RESTORE TABLE global_table FROM 'nodelocal://1/global_table/'; +RESTORE TABLE global_table FROM 'nodelocal://0/global_table/'; query IIIIT SELECT * FROM regional_by_row_table; @@ -1192,13 +1192,13 @@ RANGE default ALTER RANGE default CONFIGURE ZONE USING lease_preferences = '[]' statement ok -BACKUP TABLE non_mr_table TO 'nodelocal://1/non_mr_table/' +BACKUP TABLE non_mr_table TO 'nodelocal://0/non_mr_table/' statement ok DROP TABLE non_mr_table statement ok -RESTORE TABLE non_mr_table FROM 'nodelocal://1/non_mr_table/' +RESTORE TABLE non_mr_table FROM 'nodelocal://0/non_mr_table/' query TT SHOW ZONE CONFIGURATION FROM TABLE non_mr_table @@ -1212,11 +1212,11 @@ RANGE default ALTER RANGE default CONFIGURE ZONE USING lease_preferences = '[]' statement ok -RESTORE TABLE non_mr_table FROM 'nodelocal://1/non_mr_table/' WITH into_db = 'mr-backup-1' +RESTORE TABLE non_mr_table FROM 'nodelocal://0/non_mr_table/' WITH into_db = 'mr-backup-1' # Verify that an MR table cannot be restored in a non-MR database. statement error cannot restore descriptor for multi-region table regional_by_row_table into non-multi-region database non_mr_backup -RESTORE TABLE "mr-backup-2".regional_by_row_table FROM 'nodelocal://1/mr-backup-2/' WITH into_db = 'non_mr_backup' +RESTORE TABLE "mr-backup-2".regional_by_row_table FROM 'nodelocal://0/mr-backup-2/' WITH into_db = 'non_mr_backup' statement ok USE 'mr-backup-1' @@ -1258,7 +1258,7 @@ DROP TABLE global_table; subtest restore_tables_into_database_with_same_regions statement ok -RESTORE TABLE regional_by_row_table FROM 'nodelocal://1/mr-backup-2/' +RESTORE TABLE regional_by_row_table FROM 'nodelocal://0/mr-backup-2/' query TT SHOW CREATE TABLE regional_by_row_table @@ -1278,7 +1278,7 @@ regional_by_row_table CREATE TABLE public.regional_by_row_table ( ) LOCALITY REGIONAL BY ROW statement ok -RESTORE TABLE regional_by_table_in_primary_region FROM 'nodelocal://1/mr-backup-2/' +RESTORE TABLE regional_by_table_in_primary_region FROM 'nodelocal://0/mr-backup-2/' query TT SHOW CREATE TABLE regional_by_table_in_primary_region @@ -1292,7 +1292,7 @@ regional_by_table_in_primary_region CREATE TABLE public.regional_by_ statement ok -RESTORE TABLE regional_by_table_in_ca_central_1 FROM 'nodelocal://1/mr-backup-2/' +RESTORE TABLE regional_by_table_in_ca_central_1 FROM 'nodelocal://0/mr-backup-2/' # REGIONAL BY TABLE tables with a specific region are permitted if that region # exists in the database. @@ -1307,7 +1307,7 @@ regional_by_table_in_ca_central_1 CREATE TABLE public.regional_by_ ) LOCALITY REGIONAL BY TABLE IN "ca-central-1" statement ok -RESTORE TABLE global_table FROM 'nodelocal://1/mr-backup-2/' +RESTORE TABLE global_table FROM 'nodelocal://0/mr-backup-2/' query TT SHOW CREATE TABLE global_table @@ -1327,7 +1327,7 @@ statement ok CREATE DATABASE "mr-restore-1" primary region "ap-southeast-2" regions "us-east-1" statement ok -RESTORE TABLE "mr-backup-2".global_table FROM 'nodelocal://1/mr-backup-2/' WITH into_db='mr-restore-1'; +RESTORE TABLE "mr-backup-2".global_table FROM 'nodelocal://0/mr-backup-2/' WITH into_db='mr-restore-1'; USE "mr-restore-1"; query TT @@ -1356,7 +1356,7 @@ TABLE global_table ALTER TABLE global_table CONFIGURE ZONE USING statement ok -RESTORE TABLE "mr-backup-2".regional_by_table_in_primary_region FROM 'nodelocal://1/mr-backup-2/' WITH into_db='mr-restore-1'; +RESTORE TABLE "mr-backup-2".regional_by_table_in_primary_region FROM 'nodelocal://0/mr-backup-2/' WITH into_db='mr-restore-1'; query TT SHOW CREATE TABLE regional_by_table_in_primary_region @@ -1382,11 +1382,11 @@ DATABASE "mr-restore-1" ALTER DATABASE "mr-restore-1" CONFIGURE ZONE USING lease_preferences = '[[+region=ap-southeast-2]]' statement error "crdb_internal_region" is not compatible with type "crdb_internal_region" existing in cluster: could not find enum value "ca-central-1" in "crdb_internal_region" -RESTORE TABLE "mr-backup-2".regional_by_table_in_ap_southeast_2 FROM 'nodelocal://1/mr-backup-2/' WITH into_db='mr-restore-1'; +RESTORE TABLE "mr-backup-2".regional_by_table_in_ap_southeast_2 FROM 'nodelocal://0/mr-backup-2/' WITH into_db='mr-restore-1'; # Cannot restore a REGIONAL BY TABLE table that has different regions. statement error cannot restore REGIONAL BY TABLE regional_by_table_in_ca_central_1 IN REGION "ca-central-1" \(table ID: [0-9]+\) into database "mr-restore-1"; region "ca-central-1" not found in database regions "ap-southeast-2", "us-east-1" -RESTORE TABLE "mr-backup-2".regional_by_table_in_ca_central_1 FROM 'nodelocal://1/mr-backup-2/' WITH into_db='mr-restore-1' +RESTORE TABLE "mr-backup-2".regional_by_table_in_ca_central_1 FROM 'nodelocal://0/mr-backup-2/' WITH into_db='mr-restore-1' statement error "crdb_internal_region" is not compatible with type "crdb_internal_region" existing in cluster: could not find enum value "ca-central-1" in "crdb_internal_region" -RESTORE TABLE "mr-backup-2".regional_by_row_table FROM 'nodelocal://1/mr-backup-2/' WITH into_db='mr-restore-1' +RESTORE TABLE "mr-backup-2".regional_by_row_table FROM 'nodelocal://0/mr-backup-2/' WITH into_db='mr-restore-1' diff --git a/pkg/ccl/logictestccl/testdata/logic_test/multi_region_import_export b/pkg/ccl/logictestccl/testdata/logic_test/multi_region_import_export index 84c2e2217202..20275268389c 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/multi_region_import_export +++ b/pkg/ccl/logictestccl/testdata/logic_test/multi_region_import_export @@ -71,7 +71,7 @@ id name likes dislikes 2 otan {"Sydney suburbs",cricket,vim} {"flaky tests",onboarding} statement ok -EXPORT INTO CSV 'nodelocal://1/team_export/' WITH DELIMITER = '|' FROM TABLE team +EXPORT INTO CSV 'nodelocal://0/team_export/' WITH DELIMITER = '|' FROM TABLE team statement ok use multi_region_test_db; @@ -82,7 +82,7 @@ CREATE TABLE team ( dislikes string[], FAMILY "primary" (id, name, likes, dislikes) ); -IMPORT INTO team CSV DATA ('nodelocal://1/team_export/export*.csv') WITH DELIMITER = '|' +IMPORT INTO team CSV DATA ('nodelocal://0/team_export/export*.csv') WITH DELIMITER = '|' query ITTT colnames SELECT * FROM team diff --git a/pkg/ccl/logictestccl/testdata/logic_test/tenant_usage b/pkg/ccl/logictestccl/testdata/logic_test/tenant_usage index b81296f5d3db..f1207743f323 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/tenant_usage +++ b/pkg/ccl/logictestccl/testdata/logic_test/tenant_usage @@ -1,4 +1,6 @@ -# LogicTest: !3node-tenant +# LogicTest: local +# Only run this test in local mode due to its interaction with the default +# tenant. query error tenant "13" does not exist SELECT crdb_internal.update_tenant_resource_limits(13, 1000, 100, 0, now(), 0) diff --git a/pkg/ccl/multiregionccl/datadriven_test.go b/pkg/ccl/multiregionccl/datadriven_test.go index d98649b5d1ca..1ae80fc2f06e 100644 --- a/pkg/ccl/multiregionccl/datadriven_test.go +++ b/pkg/ccl/multiregionccl/datadriven_test.go @@ -145,6 +145,12 @@ func TestMultiRegionDataDriven(t *testing.T) { } serverArgs[i] = base.TestServerArgs{ Locality: localityCfg, + // We need to disable the SQL server here because + // it appears as though operations like + // "wait-for-zone-config-changes" only work correctly + // when called from the system SQL server. More + // investigation is required here. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ SQLExecutor: &sql.ExecutorTestingKnobs{ WithStatementTrace: func(trace tracing.Recording, stmt string) { diff --git a/pkg/ccl/multiregionccl/multiregionccltestutils/testutils.go b/pkg/ccl/multiregionccl/multiregionccltestutils/testutils.go index fe7dea7a47e9..1759a7e08940 100644 --- a/pkg/ccl/multiregionccl/multiregionccltestutils/testutils.go +++ b/pkg/ccl/multiregionccl/multiregionccltestutils/testutils.go @@ -78,6 +78,12 @@ func TestingCreateMultiRegionCluster( Knobs: knobs, ExternalIODir: params.baseDir, UseDatabase: params.useDatabase, + // Disabling this due to failures in the rtt_analysis tests. Ideally + // we could disable multi-tenancy just for those tests, but this is + // common code to create the MR cluster for all test cases. For + // bonus points, the code to re-enable this should also provide more + // flexibility in disabling this setting by callers. + DisableDefaultSQLServer: true, Locality: roachpb.Locality{ Tiers: []roachpb.Tier{{Key: "region", Value: regionNames[i]}}, }, diff --git a/pkg/ccl/multiregionccl/regional_by_row_test.go b/pkg/ccl/multiregionccl/regional_by_row_test.go index 95d6b5b2470e..7cd4ede657c8 100644 --- a/pkg/ccl/multiregionccl/regional_by_row_test.go +++ b/pkg/ccl/multiregionccl/regional_by_row_test.go @@ -349,6 +349,11 @@ func TestAlterTableLocalityRegionalByRowError(t *testing.T) { params.Locality.Tiers = []roachpb.Tier{ {Key: "region", Value: "ajstorm-1"}, } + // Need to disable the SQL server here because + // when running inside a tenant, for some reason + // this test doesn't error when expected. More + // investigation is required. + params.DisableDefaultSQLServer = true var sqlDB *gosql.DB params.Knobs = base.TestingKnobs{ SQLSchemaChanger: &sql.SchemaChangerTestingKnobs{ diff --git a/pkg/ccl/multiregionccl/unique_test.go b/pkg/ccl/multiregionccl/unique_test.go index 147a72d693f1..bab0af66aca3 100644 --- a/pkg/ccl/multiregionccl/unique_test.go +++ b/pkg/ccl/multiregionccl/unique_test.go @@ -31,7 +31,9 @@ import ( func TestValidateUniqueConstraints(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - s, db, kvDB := serverutils.StartServer(t, base.TestServerArgs{}) + // This test fails when run with the SQL server. More investigation is + // required. Tracked with #76378. + s, db, kvDB := serverutils.StartServer(t, base.TestServerArgs{DisableDefaultSQLServer: true}) defer s.Stopper().Stop(context.Background()) r := sqlutils.MakeSQLRunner(db) diff --git a/pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go b/pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go index 0bee3a737309..4b7fd1bae8b8 100644 --- a/pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go +++ b/pkg/ccl/multitenantccl/tenantcostclient/tenant_side_test.go @@ -639,7 +639,9 @@ func TestSQLLivenessExemption(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - hostServer, hostDB, hostKV := serverutils.StartServer(t, base.TestServerArgs{}) + // Have to disable the SQL server because test below assumes it's + // running on the host tenant and creates a user tenant. + hostServer, hostDB, hostKV := serverutils.StartServer(t, base.TestServerArgs{DisableDefaultSQLServer: true}) defer hostServer.Stopper().Stop(context.Background()) tenantID := serverutils.TestTenantID() @@ -656,7 +658,6 @@ func TestSQLLivenessExemption(t *testing.T) { slinstance.DefaultHeartBeat.Override(ctx, &st.SV, time.Millisecond) _, tenantDB := serverutils.StartTenant(t, hostServer, base.TestTenantArgs{ - Existing: true, TenantID: tenantID, Settings: st, AllowSettingClusterSettings: true, diff --git a/pkg/ccl/serverccl/admin_test.go b/pkg/ccl/serverccl/admin_test.go index a0e3e063caed..cf9985494a10 100644 --- a/pkg/ccl/serverccl/admin_test.go +++ b/pkg/ccl/serverccl/admin_test.go @@ -31,7 +31,15 @@ func TestAdminAPIDataDistributionPartitioning(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - testCluster := serverutils.StartNewTestCluster(t, 3, base.TestClusterArgs{}) + testCluster := serverutils.StartNewTestCluster(t, 3, + base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + // Need to disable the SQL server because this test fails + // when run through a tenant (with internal server error). + // More investigation is required. + DisableDefaultSQLServer: true, + }, + }) defer testCluster.Stopper().Stop(context.Background()) firstServer := testCluster.Server(0) diff --git a/pkg/ccl/serverccl/role_authentication_test.go b/pkg/ccl/serverccl/role_authentication_test.go index 4272abf4e937..ad9c5daab104 100644 --- a/pkg/ccl/serverccl/role_authentication_test.go +++ b/pkg/ccl/serverccl/role_authentication_test.go @@ -32,7 +32,14 @@ func TestVerifyPassword(t *testing.T) { defer log.Scope(t).Close(t) ctx := context.Background() - s, db, _ := serverutils.StartServer(t, base.TestServerArgs{}) + s, db, _ := serverutils.StartServer(t, + base.TestServerArgs{ + // Need to disable the SQL server here because it appears as + // though we don't have all the same roles in the tenant as we + // have in the host cluster (like root). + DisableDefaultSQLServer: true, + }, + ) defer s.Stopper().Stop(ctx) ie := sql.MakeInternalExecutor( diff --git a/pkg/ccl/serverccl/server_sql_test.go b/pkg/ccl/serverccl/server_sql_test.go index f15c38537520..c240ad414c6b 100644 --- a/pkg/ccl/serverccl/server_sql_test.go +++ b/pkg/ccl/serverccl/server_sql_test.go @@ -181,14 +181,18 @@ func TestNonExistentTenant(t *testing.T) { defer log.Scope(t).Close(t) ctx := context.Background() - tc := serverutils.StartNewTestCluster(t, 1, base.TestClusterArgs{}) + tc := serverutils.StartNewTestCluster(t, 1, base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + DisableDefaultSQLServer: true, + }, + }) defer tc.Stopper().Stop(ctx) _, err := tc.Server(0).StartTenant(ctx, base.TestTenantArgs{ - TenantID: serverutils.TestTenantID(), - Existing: true, - SkipTenantCheck: true, + TenantID: serverutils.TestTenantID(), + DisableCreateTenant: true, + SkipTenantCheck: true, }) require.Error(t, err) require.Equal(t, "system DB uninitialized, check if tenant is non existent", err.Error()) diff --git a/pkg/ccl/serverccl/statusccl/tenant_grpc_test.go b/pkg/ccl/serverccl/statusccl/tenant_grpc_test.go index 4095e8e83cb7..79f998f9b8e3 100644 --- a/pkg/ccl/serverccl/statusccl/tenant_grpc_test.go +++ b/pkg/ccl/serverccl/statusccl/tenant_grpc_test.go @@ -92,7 +92,6 @@ func TestTenantGRPCServices(t *testing.T) { tenant2, connTenant2 := serverutils.StartTenant(t, server, base.TestTenantArgs{ TenantID: tenantID, - Existing: true, TestingKnobs: testingKnobs, }) defer connTenant2.Close() diff --git a/pkg/ccl/serverccl/statusccl/tenant_status_test.go b/pkg/ccl/serverccl/statusccl/tenant_status_test.go index 1bea8f4d9fa1..37f00d9d5520 100644 --- a/pkg/ccl/serverccl/statusccl/tenant_status_test.go +++ b/pkg/ccl/serverccl/statusccl/tenant_status_test.go @@ -94,6 +94,10 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) { serverParams.Knobs.SpanConfig = &spanconfig.TestingKnobs{ ManagerDisableJobCreation: true, // TODO(irfansharif): #74919. } + // Need to disable the SQL server here as the non-tenant case below + // assumes that it's operating on the host cluster, but creating a default + // tenant will have it operate in the SQL server. + serverParams.DisableDefaultSQLServer = true testCluster := serverutils.StartNewTestCluster(t, 3 /* numNodes */, base.TestClusterArgs{ ServerArgs: serverParams, }) diff --git a/pkg/ccl/serverccl/statusccl/tenant_test_utils.go b/pkg/ccl/serverccl/statusccl/tenant_test_utils.go index 91c5a15acc51..b4a75526743e 100644 --- a/pkg/ccl/serverccl/statusccl/tenant_test_utils.go +++ b/pkg/ccl/serverccl/statusccl/tenant_test_utils.go @@ -47,14 +47,12 @@ type testTenant struct { func newTestTenant( t *testing.T, server serverutils.TestServerInterface, - existing bool, tenantID roachpb.TenantID, knobs base.TestingKnobs, ) *testTenant { t.Helper() tenantParams := tests.CreateTestTenantParams(tenantID) - tenantParams.Existing = existing tenantParams.TestingKnobs = knobs tenant, tenantConn := serverutils.StartTenant(t, server, tenantParams) @@ -144,11 +142,9 @@ func newTenantCluster( t.Helper() cluster := make([]*testTenant, tenantClusterSize) - existing := false for i := 0; i < tenantClusterSize; i++ { cluster[i] = - newTestTenant(t, server, existing, roachpb.MakeTenantID(tenantID), knobs) - existing = true + newTestTenant(t, server, roachpb.MakeTenantID(tenantID), knobs) } return cluster diff --git a/pkg/ccl/serverccl/tenant_decommissioned_host_test.go b/pkg/ccl/serverccl/tenant_decommissioned_host_test.go index 09640886e58c..e11f32a16582 100644 --- a/pkg/ccl/serverccl/tenant_decommissioned_host_test.go +++ b/pkg/ccl/serverccl/tenant_decommissioned_host_test.go @@ -58,7 +58,6 @@ func TestTenantWithDecommissionedID(t *testing.T) { for instanceID := 1; instanceID <= int(decommissionID); instanceID++ { sqlServer, tenant := serverutils.StartTenant(t, server, base.TestTenantArgs{ TenantID: tenantID, - Existing: instanceID != 1, // Set a low heartbeat interval. The first heartbeat succeeds // because the tenant needs to communicate with the kv node to // determine its instance id. diff --git a/pkg/ccl/spanconfigccl/spanconfigcomparedccl/datadriven_test.go b/pkg/ccl/spanconfigccl/spanconfigcomparedccl/datadriven_test.go index ca11df504192..bf7e3a7d2552 100644 --- a/pkg/ccl/spanconfigccl/spanconfigcomparedccl/datadriven_test.go +++ b/pkg/ccl/spanconfigccl/spanconfigcomparedccl/datadriven_test.go @@ -95,6 +95,9 @@ func TestDataDriven(t *testing.T) { } tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ + // We need to disable the SQL server here due to failures + // in the multitenant tests. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(), // speeds up test SpanConfig: scKnobs, diff --git a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/datadriven_test.go b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/datadriven_test.go index 48f211dc7519..a63236a51a75 100644 --- a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/datadriven_test.go +++ b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/datadriven_test.go @@ -88,6 +88,9 @@ func TestDataDriven(t *testing.T) { } tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ + // Test fails when run under the default test tenant. More + // investigation is required. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(), // speeds up test SpanConfig: scKnobs, diff --git a/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/datadriven_test.go b/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/datadriven_test.go index 84d106f5ca13..4132d2abcf63 100644 --- a/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/datadriven_test.go +++ b/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/datadriven_test.go @@ -99,6 +99,9 @@ func TestDataDriven(t *testing.T) { datadriven.Walk(t, testutils.TestDataPath(t), func(t *testing.T, path string) { tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ + // Test fails when run under a SQL server. More investigation + // is required. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ SpanConfig: scKnobs, ProtectedTS: ptsKnobs, diff --git a/pkg/ccl/spanconfigccl/spanconfigsqlwatcherccl/sqlwatcher_test.go b/pkg/ccl/spanconfigccl/spanconfigsqlwatcherccl/sqlwatcher_test.go index e3c1959bb843..2c3372bf5c37 100644 --- a/pkg/ccl/spanconfigccl/spanconfigsqlwatcherccl/sqlwatcher_test.go +++ b/pkg/ccl/spanconfigccl/spanconfigsqlwatcherccl/sqlwatcher_test.go @@ -61,6 +61,8 @@ func TestSQLWatcherReactsToUpdates(t *testing.T) { tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ ExternalIODir: dir, + // Test already runs from a SQL server. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ SpanConfig: &spanconfig.TestingKnobs{ ManagerDisableJobCreation: true, // disable the automatic job creation. @@ -269,6 +271,8 @@ func TestSQLWatcherMultiple(t *testing.T) { tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ + // Test already runs from a SQL server. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ SpanConfig: &spanconfig.TestingKnobs{ ManagerDisableJobCreation: true, // disable the automatic job creation. @@ -399,6 +403,8 @@ func TestSQLWatcherOnEventError(t *testing.T) { tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ + // Test already runs from a SQL server. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ SpanConfig: &spanconfig.TestingKnobs{ ManagerDisableJobCreation: true, // disable the automatic job creation. @@ -448,6 +454,8 @@ func TestSQLWatcherHandlerError(t *testing.T) { tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ + // Test already runs from a SQL server. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ SpanConfig: &spanconfig.TestingKnobs{ ManagerDisableJobCreation: true, // disable the automatic job creation. @@ -524,6 +532,8 @@ func TestWatcherReceivesNoopCheckpoints(t *testing.T) { tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ + // Test already runs from a SQL server. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ SpanConfig: &spanconfig.TestingKnobs{ ManagerDisableJobCreation: true, // disable the automatic job creation. diff --git a/pkg/ccl/sqlproxyccl/proxy_handler_test.go b/pkg/ccl/sqlproxyccl/proxy_handler_test.go index 9a8ab4bc9981..d63a80e539b4 100644 --- a/pkg/ccl/sqlproxyccl/proxy_handler_test.go +++ b/pkg/ccl/sqlproxyccl/proxy_handler_test.go @@ -211,7 +211,14 @@ func TestProxyAgainstSecureCRDB(t *testing.T) { te := newTester() defer te.Close() - sql, db, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) + sql, db, _ := serverutils.StartServer(t, base.TestServerArgs{ + Insecure: false, + // Need to disable the SQL server here because it appears as though + // we're not able to establish the necessary connections from within + // it. More investigation required. + DisableDefaultSQLServer: true, + }, + ) pgs := sql.(*server.TestServer).PGServer().(*pgwire.Server) pgs.TestingSetTrustClientProvidedRemoteAddr(true) pgs.TestingEnableAuthLogging() @@ -331,7 +338,15 @@ func TestProxyTLSClose(t *testing.T) { te := newTester() defer te.Close() - sql, db, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) + sql, db, _ := serverutils.StartServer(t, + base.TestServerArgs{ + Insecure: false, + // Need to disable the SQL server here because it appears as though + // we're not able to establish the necessary connections from within + // it. More investigation required. + DisableDefaultSQLServer: true, + }, + ) pgs := sql.(*server.TestServer).PGServer().(*pgwire.Server) pgs.TestingSetTrustClientProvidedRemoteAddr(true) pgs.TestingEnableAuthLogging() @@ -379,7 +394,15 @@ func TestProxyModifyRequestParams(t *testing.T) { te := newTester() defer te.Close() - sql, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) + sql, sqlDB, _ := serverutils.StartServer(t, + base.TestServerArgs{ + Insecure: false, + // Need to disable the SQL server here because it appears as though + // we're not able to establish the necessary connections from within + // it. More investigation required. + DisableDefaultSQLServer: true, + }, + ) pgs := sql.(*server.TestServer).PGServer().(*pgwire.Server) pgs.TestingSetTrustClientProvidedRemoteAddr(true) pgs.TestingEnableAuthLogging() @@ -435,7 +458,16 @@ func TestInsecureProxy(t *testing.T) { te := newTester() defer te.Close() - sql, db, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) + sql, db, _ := serverutils.StartServer(t, + base.TestServerArgs{ + // Need to disable the SQL server here as the test below + // complains about not being able to find the user. This may be + // because of the connection through the proxy server. More + // investigation is required. + DisableDefaultSQLServer: true, + Insecure: false, + }, + ) pgs := sql.(*server.TestServer).PGServer().(*pgwire.Server) pgs.TestingSetTrustClientProvidedRemoteAddr(true) pgs.TestingEnableAuthLogging() @@ -547,7 +579,15 @@ func TestDenylistUpdate(t *testing.T) { denyList, err := ioutil.TempFile("", "*_denylist.yml") require.NoError(t, err) - sql, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) + sql, sqlDB, _ := serverutils.StartServer(t, + base.TestServerArgs{ + Insecure: false, + // Need to disable the SQL server here because it appears as though + // we're not able to establish the necessary connections from within + // it. More investigation required. + DisableDefaultSQLServer: true, + }, + ) sql.(*server.TestServer).PGServer().(*pgwire.Server).TestingSetTrustClientProvidedRemoteAddr(true) defer sql.Stopper().Stop(ctx) @@ -617,7 +657,15 @@ func TestDirectoryConnect(t *testing.T) { te := newTester() defer te.Close() - srv, _, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: true}) + srv, _, _ := serverutils.StartServer(t, + base.TestServerArgs{ + Insecure: true, + // Need to disable the SQL server here because it appears as though + // we're not able to establish the necessary connections from within + // it. More investigation required. + DisableDefaultSQLServer: true, + }, + ) srv.(*server.TestServer).PGServer().(*pgwire.Server).TestingSetTrustClientProvidedRemoteAddr(true) defer srv.Stopper().Stop(ctx) @@ -1185,7 +1233,6 @@ func newDirectoryServer( tenantStopper := tenantdirsvr.NewSubStopper(tdsStopper) ten, err := srv.StartTenant(ctx, base.TestTenantArgs{ - Existing: true, TenantID: roachpb.MakeTenantID(tenantID), ForceInsecure: true, Stopper: tenantStopper, diff --git a/pkg/ccl/sqlproxyccl/tenant/directory_test.go b/pkg/ccl/sqlproxyccl/tenant/directory_test.go index 44771f41994b..7c0cc3272a4c 100644 --- a/pkg/ccl/sqlproxyccl/tenant/directory_test.go +++ b/pkg/ccl/sqlproxyccl/tenant/directory_test.go @@ -493,10 +493,12 @@ func startTenant( t, err := srv.StartTenant( ctx, base.TestTenantArgs{ - Existing: true, - TenantID: roachpb.MakeTenantID(id), - ForceInsecure: true, - Stopper: tenantStopper, + TenantID: roachpb.MakeTenantID(id), + // Disable tenant creation, since this function assumes a tenant + // already exists. + DisableCreateTenant: true, + ForceInsecure: true, + Stopper: tenantStopper, }) if err != nil { // Remap tenant "not found" error to GRPC NotFound error. @@ -522,10 +524,13 @@ func newTestDirectory( tds *tenantdirsvr.TestDirectoryServer, ) { tc = serverutils.StartNewTestCluster(t, 1, base.TestClusterArgs{ - // We need to start the cluster insecure in order to not - // care about TLS settings for the RPC client connection. ServerArgs: base.TestServerArgs{ + // We need to start the cluster insecure in order to not + // care about TLS settings for the RPC client connection. Insecure: true, + // Test fails when run under a SQL server. More investigation + // is required here. + DisableDefaultSQLServer: true, }, }) clusterStopper := tc.Stopper() diff --git a/pkg/ccl/streamingccl/streamclient/cockroach_sinkless_replication_client_test.go b/pkg/ccl/streamingccl/streamclient/cockroach_sinkless_replication_client_test.go index fb35dfe47392..e18c7c3717de 100644 --- a/pkg/ccl/streamingccl/streamclient/cockroach_sinkless_replication_client_test.go +++ b/pkg/ccl/streamingccl/streamclient/cockroach_sinkless_replication_client_test.go @@ -55,7 +55,14 @@ func TestSinklessReplicationClient(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - h, cleanup := streamingtest.NewReplicationHelper(t, base.TestServerArgs{}) + h, cleanup := streamingtest.NewReplicationHelper(t, + base.TestServerArgs{ + // Need to disable the SQL server here as the test below tries + // to enable streaming and streaming isn't yet supported at the + // tenant level. + DisableDefaultSQLServer: true, + }, + ) defer cleanup() ctx := context.Background() diff --git a/pkg/ccl/streamingccl/streamclient/partitioned_stream_client_test.go b/pkg/ccl/streamingccl/streamclient/partitioned_stream_client_test.go index 643652c44164..8e9508a1b8f9 100644 --- a/pkg/ccl/streamingccl/streamclient/partitioned_stream_client_test.go +++ b/pkg/ccl/streamingccl/streamclient/partitioned_stream_client_test.go @@ -53,6 +53,9 @@ func TestPartitionedStreamReplicationClient(t *testing.T) { skip.UnderRace(t, "partitionedStreamClient can't work under race") h, cleanup := streamingtest.NewReplicationHelper(t, base.TestServerArgs{ + // Need to disable the SQL server until tenant-level restore is + // supported. Tracked with #76378. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(), }, diff --git a/pkg/ccl/streamingccl/streamingest/stream_ingestion_job_test.go b/pkg/ccl/streamingccl/streamingest/stream_ingestion_job_test.go index 81b358e0104d..f052f62a41f6 100644 --- a/pkg/ccl/streamingccl/streamingest/stream_ingestion_job_test.go +++ b/pkg/ccl/streamingccl/streamingest/stream_ingestion_job_test.go @@ -48,15 +48,23 @@ func TestTenantStreaming(t *testing.T) { ctx := context.Background() - args := base.TestServerArgs{Knobs: base.TestingKnobs{ - JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals()}, + args := base.TestServerArgs{ + // Disabling the SQL server because the test below assumes that + // when it's monitoring the streaming job, it's doing so from the system + // SQL server and not from within a non-system SQL server. When inside + // the non-system SQL server, it won't be able to see the streaming job. + // This may also be impacted by the fact that we don't currently support + // tenant->tenant streaming. Tracked with #76378. + DisableDefaultSQLServer: true, + Knobs: base.TestingKnobs{ + JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals()}, } // Start the source server. source, sourceDB, _ := serverutils.StartServer(t, args) defer source.Stopper().Stop(ctx) - // Start tenant server in the srouce cluster. + // Start tenant server in the source cluster. tenantID := serverutils.TestTenantID() _, tenantConn := serverutils.StartTenant(t, source, base.TestTenantArgs{TenantID: tenantID}) defer tenantConn.Close() @@ -75,7 +83,10 @@ SET CLUSTER SETTING changefeed.experimental_poll_interval = '10ms' require.NoError(t, err) // Start the destination server. - hDest, cleanupDest := streamingtest.NewReplicationHelper(t, base.TestServerArgs{}) + hDest, cleanupDest := streamingtest.NewReplicationHelper(t, + // Test fails without the SQL server disabled. More investigation + // is required. Tracked with #76378. + base.TestServerArgs{DisableDefaultSQLServer: true}) defer cleanupDest() // destSQL refers to the system tenant as that's the one that's running the // job. @@ -139,9 +150,17 @@ func TestCutoverBuiltin(t *testing.T) { defer leaktest.AfterTest(t)() ctx := context.Background() - args := base.TestClusterArgs{ServerArgs: base.TestServerArgs{ - Knobs: base.TestingKnobs{JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals()}, - }} + args := base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + // Disable the SQL server as the test below looks for a + // streaming job assuming that it's on the host cluster. + // Tracked with #76378. + DisableDefaultSQLServer: true, + Knobs: base.TestingKnobs{ + JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(), + }, + }, + } tc := testcluster.StartTestCluster(t, 1, args) defer tc.Stopper().Stop(ctx) registry := tc.Server(0).JobRegistry().(*jobs.Registry) diff --git a/pkg/ccl/streamingccl/streamingest/stream_ingestion_test.go b/pkg/ccl/streamingccl/streamingest/stream_ingestion_test.go index fa134541d683..57c398f7983a 100644 --- a/pkg/ccl/streamingccl/streamingest/stream_ingestion_test.go +++ b/pkg/ccl/streamingccl/streamingest/stream_ingestion_test.go @@ -106,7 +106,12 @@ func TestStreamIngestionJobWithRandomClient(t *testing.T) { var receivedRevertRequest chan struct{} var allowResponse chan struct{} var revertRangeTargetTime hlc.Timestamp - params := base.TestClusterArgs{} + params := base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + // Test hangs with SQL server. More investigation is required. + DisableDefaultSQLServer: true, + }, + } params.ServerArgs.Knobs.Store = &kvserver.StoreTestingKnobs{ TestingRequestFilter: func(_ context.Context, ba roachpb.BatchRequest) *roachpb.Error { for _, req := range ba.Requests { diff --git a/pkg/ccl/streamingccl/streamproducer/producer_job_test.go b/pkg/ccl/streamingccl/streamproducer/producer_job_test.go index 89bf9dce2cd7..969a77391313 100644 --- a/pkg/ccl/streamingccl/streamproducer/producer_job_test.go +++ b/pkg/ccl/streamingccl/streamproducer/producer_job_test.go @@ -101,6 +101,9 @@ func TestStreamReplicationProducerJob(t *testing.T) { ctx := context.Background() clusterArgs := base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ + // Test fails with the SQL server defined. More investigation + // is required. Tracked with #76378. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(), }, diff --git a/pkg/ccl/streamingccl/streamproducer/replication_stream_test.go b/pkg/ccl/streamingccl/streamproducer/replication_stream_test.go index 65ac67015dd2..2782280890be 100644 --- a/pkg/ccl/streamingccl/streamproducer/replication_stream_test.go +++ b/pkg/ccl/streamingccl/streamproducer/replication_stream_test.go @@ -204,7 +204,14 @@ func startReplication( func TestReplicationStreamTenant(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - h, cleanup := streamingtest.NewReplicationHelper(t, base.TestServerArgs{}) + h, cleanup := streamingtest.NewReplicationHelper(t, + base.TestServerArgs{ + // This test fails when run from the SQL server. This is likely due + // to the lack of support for tenant streaming, but more investigation + // is required. Tracked with #76378. + DisableDefaultSQLServer: true, + }, + ) defer cleanup() h.Tenant.SQL.Exec(t, ` @@ -279,6 +286,10 @@ func TestReplicationStreamInitialization(t *testing.T) { defer log.Scope(t).Close(t) serverArgs := base.TestServerArgs{ + // This test fails when run from the SQL server. This is likely due + // to the lack of support for tenant streaming, but more investigation + // is required. Tracked with #76378. + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(), }, @@ -353,7 +364,13 @@ func TestReplicationStreamInitialization(t *testing.T) { func TestStreamPartition(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - h, cleanup := streamingtest.NewReplicationHelper(t, base.TestServerArgs{}) + h, cleanup := streamingtest.NewReplicationHelper(t, + base.TestServerArgs{ + // Test fails with SQL server. More investigation is required. + // Tracked with #76378. + DisableDefaultSQLServer: true, + }, + ) defer cleanup() h.Tenant.SQL.Exec(t, ` diff --git a/pkg/ccl/testccl/sqlccl/temp_table_clean_test.go b/pkg/ccl/testccl/sqlccl/temp_table_clean_test.go index 4d1848ff9e7b..5e09cfcaf447 100644 --- a/pkg/ccl/testccl/sqlccl/temp_table_clean_test.go +++ b/pkg/ccl/testccl/sqlccl/temp_table_clean_test.go @@ -119,7 +119,6 @@ func TestTenantTempTableCleanup(t *testing.T) { _, tenantSecondDB := serverutils.StartTenant(t, tc.Server(1), base.TestTenantArgs{ - Existing: true, TenantID: serverutils.TestTenantID(), Settings: settings, Stopper: tenantStoppers[1], @@ -161,7 +160,6 @@ func TestTenantTempTableCleanup(t *testing.T) { tenantStoppers[0] = stop.NewStopper() _, tenantPrimaryDB = serverutils.StartTenant(t, tc.Server(0), base.TestTenantArgs{ - Existing: true, TenantID: serverutils.TestTenantID(), Settings: settings, TestingKnobs: tenantTempKnobSettings, diff --git a/pkg/ccl/workloadccl/allccl/all_test.go b/pkg/ccl/workloadccl/allccl/all_test.go index 84c1032f02f4..c7471d87dc87 100644 --- a/pkg/ccl/workloadccl/allccl/all_test.go +++ b/pkg/ccl/workloadccl/allccl/all_test.go @@ -88,8 +88,11 @@ func TestAllRegisteredImportFixture(t *testing.T) { ctx := context.Background() s, db, _ := serverutils.StartServer(t, base.TestServerArgs{ - UseDatabase: "d", - SQLMemoryPoolSize: sqlMemoryPoolSize, + // The SQL server needs to be disabled for this test until + // we address #75449. + DisableDefaultSQLServer: true, + UseDatabase: "d", + SQLMemoryPoolSize: sqlMemoryPoolSize, }) defer s.Stopper().Stop(ctx) sqlutils.MakeSQLRunner(db).Exec(t, `CREATE DATABASE d`) @@ -146,7 +149,10 @@ func TestAllRegisteredSetup(t *testing.T) { defer log.Scope(t).Close(t) ctx := context.Background() s, db, _ := serverutils.StartServer(t, base.TestServerArgs{ - UseDatabase: "d", + // Need to disable the SQL server here until we resolve + // #75449 as this test makes use of import through a fixture. + DisableDefaultSQLServer: true, + UseDatabase: "d", }) defer s.Stopper().Stop(ctx) sqlutils.MakeSQLRunner(db).Exec(t, `CREATE DATABASE d`) diff --git a/pkg/ccl/workloadccl/fixture_test.go b/pkg/ccl/workloadccl/fixture_test.go index 25e7f0f0c481..a2f22648a267 100644 --- a/pkg/ccl/workloadccl/fixture_test.go +++ b/pkg/ccl/workloadccl/fixture_test.go @@ -174,7 +174,12 @@ func TestImportFixture(t *testing.T) { stats.DefaultRefreshInterval = time.Millisecond stats.DefaultAsOfTime = 10 * time.Millisecond - s, db, _ := serverutils.StartServer(t, base.TestServerArgs{}) + s, db, _ := serverutils.StartServer(t, + // Need to disable the SQL server until we have a fix for #75449. + base.TestServerArgs{ + DisableDefaultSQLServer: true, + }, + ) defer s.Stopper().Stop(ctx) sqlDB := sqlutils.MakeSQLRunner(db) @@ -214,7 +219,13 @@ func TestImportFixtureCSVServer(t *testing.T) { ts := httptest.NewServer(workload.CSVMux(workload.Registered())) defer ts.Close() - s, db, _ := serverutils.StartServer(t, base.TestServerArgs{UseDatabase: `d`}) + s, db, _ := serverutils.StartServer(t, + base.TestServerArgs{ + UseDatabase: `d`, + // Test fails with SQL server due to #75449. + DisableDefaultSQLServer: true, + }, + ) defer s.Stopper().Stop(ctx) sqlDB := sqlutils.MakeSQLRunner(db) diff --git a/pkg/cli/BUILD.bazel b/pkg/cli/BUILD.bazel index dc2d79f51d81..cf94d38de5eb 100644 --- a/pkg/cli/BUILD.bazel +++ b/pkg/cli/BUILD.bazel @@ -165,6 +165,7 @@ go_library( "//pkg/storage/enginepb", "//pkg/storage/fs", "//pkg/testutils/serverutils", + "//pkg/testutils/skip", "//pkg/ts", "//pkg/ts/tspb", "//pkg/util", diff --git a/pkg/cli/democluster/demo_cluster.go b/pkg/cli/democluster/demo_cluster.go index 8ed66444d118..e14405e2bd15 100644 --- a/pkg/cli/democluster/demo_cluster.go +++ b/pkg/cli/democluster/demo_cluster.go @@ -748,6 +748,9 @@ func (demoCtx *Context) testServerArgsForTransientCluster( CacheSize: demoCtx.CacheSize, NoAutoInitializeCluster: true, EnableDemoLoginEndpoint: true, + // Demo clusters by default will create their own SQL servers, so we + // don't need to create them here. + DisableDefaultSQLServer: true, // This disables the tenant server. We could enable it but would have to // generate the suitable certs at the caller who wishes to do so. TenantAddr: new(string), diff --git a/pkg/cli/democluster/demo_cluster_test.go b/pkg/cli/democluster/demo_cluster_test.go index 364e42adfca1..acc9b540c5eb 100644 --- a/pkg/cli/democluster/demo_cluster_test.go +++ b/pkg/cli/democluster/demo_cluster_test.go @@ -67,6 +67,7 @@ func TestTestServerArgsForTransientCluster(t *testing.T) { sqlPoolMemorySize: 2 << 10, cacheSize: 1 << 10, expected: base.TestServerArgs{ + DisableDefaultSQLServer: true, PartOfCluster: true, JoinAddr: "127.0.0.1", DisableTLSForHTTP: true, @@ -90,6 +91,7 @@ func TestTestServerArgsForTransientCluster(t *testing.T) { sqlPoolMemorySize: 4 << 10, cacheSize: 4 << 10, expected: base.TestServerArgs{ + DisableDefaultSQLServer: true, PartOfCluster: true, JoinAddr: "127.0.0.1", SQLAddr: ":1236", diff --git a/pkg/cli/testutils.go b/pkg/cli/testutils.go index 774a142f4fbf..fcfeb56af9bf 100644 --- a/pkg/cli/testutils.go +++ b/pkg/cli/testutils.go @@ -35,6 +35,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/server" "github.com/cockroachdb/cockroach/pkg/sql/sqlstats" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" + "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/errors" "github.com/kr/pretty" @@ -97,9 +98,15 @@ const testTempFilePrefix = "test-temp-prefix-" // from the uniquely generated (temp directory) file path. const testUserfileUploadTempDirPrefix = "test-userfile-upload-temp-dir-" -func (c *TestCLI) fail(err interface{}) { +func (c *TestCLI) fail(err error) { if c.t != nil { defer c.logScope.Close(c.t) + if strings.Contains(err.Error(), "requires a CCL binary") { + if c.TestServer != nil { + c.TestServer.Stopper().Stop(context.Background()) + } + skip.IgnoreLint(c.t, "skipping due to lack of CCL binary") + } c.t.Fatal(err) } else { panic(err) diff --git a/pkg/gen/excluded.bzl b/pkg/gen/excluded.bzl index 3f28d2c2df64..b19a13cc7179 100644 --- a/pkg/gen/excluded.bzl +++ b/pkg/gen/excluded.bzl @@ -346,6 +346,8 @@ EXCLUDED_SRCS = [ "//pkg/ui/workspaces/cluster-ui:dist/types/sessions/sessionsTableContent.d.ts", "//pkg/ui/workspaces/cluster-ui:dist/types/sessions/terminateQueryModal.d.ts", "//pkg/ui/workspaces/cluster-ui:dist/types/sessions/terminateSessionModal.d.ts", + "//pkg/ui/workspaces/cluster-ui:dist/types/settings/booleanSetting.d.ts", + "//pkg/ui/workspaces/cluster-ui:dist/types/settings/index.d.ts", "//pkg/ui/workspaces/cluster-ui:dist/types/sortedtable/index.d.ts", "//pkg/ui/workspaces/cluster-ui:dist/types/sortedtable/sortedtable.d.ts", "//pkg/ui/workspaces/cluster-ui:dist/types/sortedtable/tableHead/index.d.ts", diff --git a/pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher.go b/pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher.go index f2383a3b6a1d..fd8bb9b6d387 100644 --- a/pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher.go +++ b/pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher.go @@ -486,10 +486,12 @@ func newRetryErrorOnFailedPreemptiveRefresh( } msg := "failed preemptive refresh" if refreshErr != nil { + backupMsg := refreshErr.String() if refreshErr, ok := refreshErr.GetDetail().(*roachpb.RefreshFailedError); ok { msg = fmt.Sprintf("%s due to a conflict: %s on key %s", msg, refreshErr.FailureReason(), refreshErr.Key) } else { - msg = fmt.Sprintf("%s - unknown error: %s", msg, refreshErr) + // We can't get details, so return the refreshErr string + msg = fmt.Sprintf("%s - unknown error %s", msg, backupMsg) } } retryErr := roachpb.NewTransactionRetryError(reason, msg) diff --git a/pkg/kv/kvclient/kvtenant/BUILD.bazel b/pkg/kv/kvclient/kvtenant/BUILD.bazel index 21b7fa616306..8af4ca469470 100644 --- a/pkg/kv/kvclient/kvtenant/BUILD.bazel +++ b/pkg/kv/kvclient/kvtenant/BUILD.bazel @@ -17,6 +17,8 @@ go_library( "//pkg/server/serverpb", "//pkg/server/settingswatcher", "//pkg/spanconfig", + "//pkg/sql/pgwire/pgcode", + "//pkg/sql/pgwire/pgerror", "//pkg/util/log", "//pkg/util/retry", "@com_github_cockroachdb_errors//:errors", diff --git a/pkg/kv/kvclient/kvtenant/connector.go b/pkg/kv/kvclient/kvtenant/connector.go index 84180bf5cb52..9cc9a7ac433b 100644 --- a/pkg/kv/kvclient/kvtenant/connector.go +++ b/pkg/kv/kvclient/kvtenant/connector.go @@ -27,6 +27,8 @@ import ( "github.com/cockroachdb/cockroach/pkg/server/serverpb" "github.com/cockroachdb/cockroach/pkg/server/settingswatcher" "github.com/cockroachdb/cockroach/pkg/spanconfig" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/retry" "github.com/cockroachdb/errors" @@ -110,7 +112,9 @@ var Factory ConnectorFactory = requiresCCLBinaryFactory{} type requiresCCLBinaryFactory struct{} func (requiresCCLBinaryFactory) NewConnector(_ ConnectorConfig, _ []string) (Connector, error) { - return nil, errors.Errorf(`tenant connector requires a CCL binary`) + return nil, pgerror.WithCandidateCode( + errors.New(`tenant connector requires a CCL binary`), + pgcode.CCLRequired) } // AddressResolver wraps a NodeDescStore interface in an adapter that allows it diff --git a/pkg/kv/kvserver/replicate_queue_test.go b/pkg/kv/kvserver/replicate_queue_test.go index 5bb943a6e62e..f711278b029f 100644 --- a/pkg/kv/kvserver/replicate_queue_test.go +++ b/pkg/kv/kvserver/replicate_queue_test.go @@ -335,6 +335,7 @@ func TestReplicateQueueUpAndDownReplicateNonVoters(t *testing.T) { base.TestClusterArgs{ ReplicationMode: base.ReplicationAuto, ServerArgs: base.TestServerArgs{ + DisableDefaultSQLServer: true, Knobs: base.TestingKnobs{ SpanConfig: &spanconfig.TestingKnobs{ ConfigureScratchRange: true, diff --git a/pkg/server/admin.go b/pkg/server/admin.go index a384e0f66d13..d66b2d5d62c0 100644 --- a/pkg/server/admin.go +++ b/pkg/server/admin.go @@ -868,6 +868,25 @@ func (s *adminServer) tableDetailsHelper( resp.CreateTableStatement = createStmt } + // Marshal SHOW STATISTICS result. + row, cols, err = s.server.sqlServer.internalExecutor.QueryRowExWithCols( + ctx, "admin-show-statistics", nil, /* txn */ + sessiondata.InternalExecutorOverride{User: userName}, + fmt.Sprintf("SELECT max(created) AS created FROM [SHOW STATISTICS FOR TABLE %s]", escQualTable), + ) + if err != nil { + return nil, err + } + if row != nil { + scanner := makeResultScanner(cols) + const createdCol = "created" + var createdTs *time.Time + if err := scanner.Scan(row, createdCol, &createdTs); err != nil { + return nil, err + } + resp.StatsLastCreatedAt = createdTs + } + // Marshal SHOW ZONE CONFIGURATION result. row, cols, err = s.server.sqlServer.internalExecutor.QueryRowExWithCols( ctx, "admin-show-zone-config", nil, /* txn */ diff --git a/pkg/server/admin_test.go b/pkg/server/admin_test.go index f3819e377c9e..bf45c657c2ba 100644 --- a/pkg/server/admin_test.go +++ b/pkg/server/admin_test.go @@ -673,6 +673,7 @@ func TestAdminAPITableDetails(t *testing.T) { "CREATE USER app", fmt.Sprintf("GRANT SELECT ON %s.%s TO readonly", escDBName, tblName), fmt.Sprintf("GRANT SELECT,UPDATE,DELETE ON %s.%s TO app", escDBName, tblName), + fmt.Sprintf("CREATE STATISTICS test_stats FROM %s.%s", escDBName, tblName), } pgURL, cleanupGoDB := sqlutils.PGUrl( t, s.ServingSQLAddr(), "StartServer" /* prefix */, url.User(security.RootUser)) @@ -779,6 +780,22 @@ func TestAdminAPITableDetails(t *testing.T) { } } + // Verify statistics last updated. + { + + showStatisticsForTableQuery := fmt.Sprintf("SELECT max(created) AS created FROM [SHOW STATISTICS FOR TABLE %s.%s]", escDBName, tblName) + + row := db.QueryRow(showStatisticsForTableQuery) + var createdTs time.Time + if err := row.Scan(&createdTs); err != nil { + t.Fatal(err) + } + + if a, e := resp.StatsLastCreatedAt, createdTs; reflect.DeepEqual(a, e) { + t.Fatalf("mismatched statistics creation timestamp; expected %s, got %s", e, a) + } + } + // Verify Descriptor ID. tableID, err := ts.admin.queryTableID(ctx, security.RootUserName(), tc.dbName, tc.tblName) if err != nil { diff --git a/pkg/server/config.go b/pkg/server/config.go index ce427f3f2f4b..ac72c0d74c6a 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -179,6 +179,9 @@ type BaseConfig struct { // Environment Variable: COCKROACH_DISABLE_SPAN_CONFIGS SpanConfigsDisabled bool + // Disables the default SQL server. + DisableDefaultSQLServer bool + // TestingKnobs is used for internal test controls only. TestingKnobs base.TestingKnobs diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index f131d4d0e353..3f9df3d140b0 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -136,10 +136,10 @@ func TestHealthCheck(t *testing.T) { }, }) + defer s.Stopper().Stop(context.Background()) if err != nil { t.Fatal(err) } - defer s.Stopper().Stop(context.Background()) ctx := context.Background() diff --git a/pkg/server/serverpb/admin.proto b/pkg/server/serverpb/admin.proto index aeb4f11215e8..9c8a0c537c77 100644 --- a/pkg/server/serverpb/admin.proto +++ b/pkg/server/serverpb/admin.proto @@ -210,6 +210,8 @@ message TableDetailsResponse { // for this table. It is a SQL statement that would re-configure the table's current // zone if executed. string configure_zone_statement = 9; + // stats_last_created_at is the time at which statistics were last created. + google.protobuf.Timestamp stats_last_created_at = 10 [(gogoproto.stdtime) = true]; } // TableStatsRequest is a request for detailed, computationally expensive diff --git a/pkg/server/systemconfigwatcher/systemconfigwatchertest/test_system_config_watcher.go b/pkg/server/systemconfigwatcher/systemconfigwatchertest/test_system_config_watcher.go index 0935811c5c5f..d1058c36c5c6 100644 --- a/pkg/server/systemconfigwatcher/systemconfigwatchertest/test_system_config_watcher.go +++ b/pkg/server/systemconfigwatcher/systemconfigwatchertest/test_system_config_watcher.go @@ -40,7 +40,13 @@ func TestSystemConfigWatcher(t *testing.T, skipSecondary bool) { defer log.Scope(t).Close(t) ctx := context.Background() - s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{}) + s, sqlDB, _ := serverutils.StartServer(t, + base.TestServerArgs{ + // Test runs against SQL server, so no need to create the default + // SQL server. + DisableDefaultSQLServer: true, + }, + ) defer s.Stopper().Stop(ctx) tdb := sqlutils.MakeSQLRunner(sqlDB) // Shorten the closed timestamp duration as a cheeky way to check the diff --git a/pkg/server/tenant.go b/pkg/server/tenant.go index 9efba6a3bf29..0357eee74054 100644 --- a/pkg/server/tenant.go +++ b/pkg/server/tenant.go @@ -214,11 +214,10 @@ func startTenantInternal( args.sqlStatusServer = tenantStatusServer s, err := newSQLServer(ctx, args) - tenantStatusServer.sqlServer = s - if err != nil { return nil, nil, nil, "", "", err } + tenantStatusServer.sqlServer = s drainServer = newDrainServer(baseCfg, args.stopper, args.grpc, s) diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go index 6ad863ea2733..03e0ea43554f 100644 --- a/pkg/server/testserver.go +++ b/pkg/server/testserver.go @@ -24,6 +24,7 @@ import ( "github.com/cenkalti/backoff" circuit "github.com/cockroachdb/circuitbreaker" "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/blobs" "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/config" "github.com/cockroachdb/cockroach/pkg/config/zonepb" @@ -46,6 +47,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/physicalplan" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" + "github.com/cockroachdb/cockroach/pkg/sql/sqlstats" "github.com/cockroachdb/cockroach/pkg/storage" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/ts" @@ -269,6 +271,8 @@ func makeTestConfigFromParams(params base.TestServerArgs) Config { cfg.TempStorageConfig = params.TempStorageConfig } + cfg.DisableDefaultSQLServer = params.DisableDefaultSQLServer + if cfg.TestingKnobs.Store == nil { cfg.TestingKnobs.Store = &kvserver.StoreTestingKnobs{} } @@ -301,6 +305,10 @@ type TestServer struct { *Server // httpTestServer provides the HTTP APIs of TestTenantInterface. *httpTestServer + // The SQLServers associated with this server. Currently, there is only one + // SQL server created by default, but longer term we may allow for creation + // of multiple SQLServers for more advanced testing. + SQLServers []serverutils.TestTenantInterface } var _ serverutils.TestServerInterface = &TestServer{} @@ -477,6 +485,63 @@ func (ts *TestServer) TestingKnobs() *base.TestingKnobs { return nil } +// maybeStartTestSQLServer might start a test SQL server. This can then be used +// for multi-tenant testing, where the default SQL connection will be made to +// this SQL server instead of to the system SQL server. Note that we will +// currently only attempt to start a SQL server if we're running in an +// enterprise enabled build. This is due to licensing restrictions on the MT +// capabilities. +func (ts *TestServer) maybeStartTestSQLServer(ctx context.Context) error { + org := sql.ClusterOrganization.Get(&ts.st.SV) + if err := base.CheckEnterpriseEnabled(ts.st, ts.ClusterID(), org, "SQL servers"); err != nil { + // If not enterprise enabled, we won't be able to use SQL Servers so eat + // the error and return without creating/starting a SQL server. + return nil // nolint:returnerrcheck + } + + if ts.params.DisableDefaultSQLServer || ts.cfg.DisableDefaultSQLServer { + return nil + } + + tempStorageConfig := base.DefaultTestTempStorageConfig(cluster.MakeTestingClusterSettings()) + params := base.TestTenantArgs{ + // Currently, all the servers leverage the same tenant ID. We may + // want to change this down the road, for more elaborate testing. + TenantID: serverutils.TestTenantID(), + MemoryPoolSize: ts.params.SQLMemoryPoolSize, + TempStorageConfig: &tempStorageConfig, + Locality: ts.params.Locality, + ExternalIODir: ts.params.ExternalIODir, + ExternalIODirConfig: ts.params.ExternalIODirConfig, + ForceInsecure: ts.Insecure(), + UseDatabase: ts.params.UseDatabase, + SSLCertsDir: ts.params.SSLCertsDir, + AllowSettingClusterSettings: true, + // These settings are inherited from the SQL server creation in + // logicTest.newCluster, and are required to run the logic test suite + // successfully. + TestingKnobs: base.TestingKnobs{ + SQLExecutor: &sql.ExecutorTestingKnobs{ + DeterministicExplain: true, + }, + SQLStatsKnobs: &sqlstats.TestingKnobs{ + AOSTClause: "AS OF SYSTEM TIME '-1us'", + }, + }, + } + + tenant, err := ts.StartTenant(ctx, params) + if err != nil { + return err + } + + if len(ts.SQLServers) == 0 { + ts.SQLServers = make([]serverutils.TestTenantInterface, 1) + } + ts.SQLServers[0] = tenant + return nil +} + // Start starts the TestServer by bootstrapping an in-memory store // (defaults to maximum of 100M). The server is started, launching the // node RPC server and all HTTP endpoints. Use the value of @@ -484,7 +549,11 @@ func (ts *TestServer) TestingKnobs() *base.TestingKnobs { // Use TestServer.Stopper().Stop() to shutdown the server after the test // completes. func (ts *TestServer) Start(ctx context.Context) error { - return ts.Server.Start(ctx) + err := ts.Server.Start(ctx) + if err != nil { + return err + } + return ts.maybeStartTestSQLServer(ctx) } type tenantProtectedTSProvider struct { @@ -635,21 +704,30 @@ func (t *TestTenant) DrainClients(ctx context.Context) error { func (ts *TestServer) StartTenant( ctx context.Context, params base.TestTenantArgs, ) (serverutils.TestTenantInterface, error) { - if !params.Existing { - if _, err := ts.InternalExecutor().(*sql.InternalExecutor).Exec( - ctx, "testserver-create-tenant", nil /* txn */, "SELECT crdb_internal.create_tenant($1)", params.TenantID.ToUint64(), - ); err != nil { + // Determine if we need to create the tenant before starting it. + if !params.DisableCreateTenant { + rowCount, err := ts.InternalExecutor().(*sql.InternalExecutor).Exec( + ctx, "testserver-check-tenant-active", nil, + "SELECT 1 FROM system.tenants WHERE id=$1 AND active=true", + params.TenantID.ToUint64(), + ) + if err != nil { return nil, err } - } - - if !params.SkipTenantCheck { + if rowCount == 0 { + // Tenant doesn't exist. Create it. + if _, err := ts.InternalExecutor().(*sql.InternalExecutor).Exec( + ctx, "testserver-create-tenant", nil /* txn */, "SELECT crdb_internal.create_tenant($1)", params.TenantID.ToUint64(), + ); err != nil { + return nil, err + } + } + } else if !params.SkipTenantCheck { rowCount, err := ts.InternalExecutor().(*sql.InternalExecutor).Exec( ctx, "testserver-check-tenant-active", nil, "SELECT 1 FROM system.tenants WHERE id=$1 AND active=true", params.TenantID.ToUint64(), ) - if err != nil { return nil, err } @@ -657,6 +735,7 @@ func (ts *TestServer) StartTenant( return nil, errors.New("not found") } } + st := params.Settings if st == nil { st = cluster.MakeTestingClusterSettings() @@ -690,6 +769,17 @@ func (ts *TestServer) StartTenant( baseCfg.TestingKnobs = params.TestingKnobs baseCfg.Insecure = params.ForceInsecure baseCfg.Locality = params.Locality + + tk := &baseCfg.TestingKnobs + blobClientFactory := blobs.NewLocalOnlyBlobClientFactory(params.ExternalIODir) + if serverKnobs, ok := tk.Server.(*TestingKnobs); ok { + serverKnobs.BlobClientFactory = blobClientFactory + } else { + tk.Server = &TestingKnobs{ + BlobClientFactory: blobClientFactory, + } + } + if params.SSLCertsDir != "" { baseCfg.SSLCertsDir = params.SSLCertsDir } @@ -799,8 +889,12 @@ func (ts *TestServer) ServingRPCAddr() string { } // ServingSQLAddr returns the server's SQL address. Should be used by clients. +// If a SQL server is started, return the first SQL server's address. func (ts *TestServer) ServingSQLAddr() string { - return ts.cfg.SQLAdvertiseAddr + if len(ts.SQLServers) == 0 { + return ts.cfg.SQLAdvertiseAddr + } + return ts.SQLServers[0].SQLAddr() } // HTTPAddr returns the server's HTTP address. Should be used by clients. diff --git a/pkg/sql/flowinfra/cluster_test.go b/pkg/sql/flowinfra/cluster_test.go index da972f772528..81a6dec265af 100644 --- a/pkg/sql/flowinfra/cluster_test.go +++ b/pkg/sql/flowinfra/cluster_test.go @@ -337,7 +337,6 @@ func TestTenantClusterFlow(t *testing.T) { for i := 0; i < numPods; i++ { pods[i], podConns[i] = serverutils.StartTenant(t, tci.Server(0), base.TestTenantArgs{ TenantID: tenantID, - Existing: i != 0, TestingKnobs: testingKnobs, }) defer podConns[i].Close() diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go index 278c7b52187e..b94c960fe909 100644 --- a/pkg/sql/logictest/logic.go +++ b/pkg/sql/logictest/logic.go @@ -501,6 +501,8 @@ type testClusterConfig struct { // If true, a sql tenant server will be started and pointed at a node in the // cluster. Connections on behalf of the logic test will go to that tenant. useTenant bool + // Disable the default SQL server in the test server + disableDefaultSQLServer bool // isCCLConfig should be true for any config that can only be run with a CCL // binary. isCCLConfig bool @@ -583,6 +585,10 @@ var logicTestConfigs = []testClusterConfig{ numNodes: 1, overrideDistSQLMode: "off", overrideAutoStats: "false", + // Need to disable the default SQL server due because of interactions + // with tenant_usage and privileged_zone_test. More investigation is + // required here. + disableDefaultSQLServer: true, }, { name: "local-declarative-schema", @@ -660,6 +666,11 @@ var logicTestConfigs = []testClusterConfig{ numNodes: 5, overrideDistSQLMode: "on", overrideAutoStats: "false", + // Have to disable the default SQL server here as there are test run in + // this mode which try to modify zone configurations and we're more + // restrictive in the way we allow zone configs to be modified by + // secondary tenants. See #75569 for more info. + disableDefaultSQLServer: true, }, { name: "5node-metadata", @@ -752,6 +763,9 @@ var logicTestConfigs = []testClusterConfig{ numNodes: 9, overrideAutoStats: "false", localities: multiregion9node3region3azsLocalities, + // Need to disable the default SQL server here until we have the + // locality optimized search working in multi-tenant configurations. + disableDefaultSQLServer: true, }, { name: "multiregion-9node-3region-3azs-tenant", @@ -1439,8 +1453,9 @@ func (t *logicTest) newCluster(serverArgs TestServerArgs, opts []clusterOpt) { params := base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - SQLMemoryPoolSize: serverArgs.maxSQLMemoryLimit, - TempStorageConfig: tempStorageConfig, + SQLMemoryPoolSize: serverArgs.maxSQLMemoryLimit, + TempStorageConfig: tempStorageConfig, + DisableDefaultSQLServer: t.cfg.useTenant || t.cfg.disableDefaultSQLServer, Knobs: base.TestingKnobs{ Store: &kvserver.StoreTestingKnobs{ // The consistency queue makes a lot of noisy logs during logic tests. @@ -1577,7 +1592,6 @@ func (t *logicTest) newCluster(serverArgs TestServerArgs, opts []clusterOpt) { MemoryPoolSize: params.ServerArgs.SQLMemoryPoolSize, TempStorageConfig: ¶ms.ServerArgs.TempStorageConfig, Locality: paramsPerNode[i].Locality, - Existing: i > 0, TracingDefault: params.ServerArgs.TracingDefault, } diff --git a/pkg/sql/logictest/testdata/logic_test/event_log b/pkg/sql/logictest/testdata/logic_test/event_log index 2503f39171a3..71c3fe321e67 100644 --- a/pkg/sql/logictest/testdata/logic_test/event_log +++ b/pkg/sql/logictest/testdata/logic_test/event_log @@ -454,6 +454,9 @@ EXECUTE set_setting('some string') # # The result differs in 3node-tenant because the # kv.range_merge.queue_enabled cluster setting is not set. +# In all other configurations, and with probabilistic SQL server creation, +# we need to filter out kv.range_merge.queue_enabled, since it will only +# be set if a SQL server is not created. skipif config 3node-tenant query IIT SELECT "targetID", "reportingID", "info"::JSONB - 'Timestamp' - 'DescriptorID' @@ -465,10 +468,10 @@ AND info NOT LIKE '%sql.defaults.vectorize%' AND info NOT LIKE '%sql.testing%' AND info NOT LIKE '%sql.defaults.experimental_distsql_planning%' AND info NOT LIKE '%sql.defaults.experimental_new_schema_changer.enabled%' +AND info NOT LIKE '%kv.range_merge.queue_enabled%' ORDER BY "timestamp", info ---- 0 1 {"ApplicationName": "$ internal-optInToDiagnosticsStatReporting", "EventType": "set_cluster_setting", "SettingName": "diagnostics.reporting.enabled", "Statement": "SET CLUSTER SETTING \"diagnostics.reporting.enabled\" = true", "Tag": "SET CLUSTER SETTING", "User": "root", "Value": "true"} -0 1 {"EventType": "set_cluster_setting", "SettingName": "kv.range_merge.queue_enabled", "Statement": "SET CLUSTER SETTING \"kv.range_merge.queue_enabled\" = false", "Tag": "SET CLUSTER SETTING", "User": "root", "Value": "false"} 0 1 {"EventType": "set_cluster_setting", "PlaceholderValues": ["5"], "SettingName": "sql.stats.automatic_collection.min_stale_rows", "Statement": "SET CLUSTER SETTING \"sql.stats.automatic_collection.min_stale_rows\" = $1::INT8", "Tag": "SET CLUSTER SETTING", "User": "root", "Value": "5"} 0 1 {"EventType": "set_cluster_setting", "SettingName": "sql.crdb_internal.table_row_statistics.as_of_time", "Statement": "SET CLUSTER SETTING \"sql.crdb_internal.table_row_statistics.as_of_time\" = e'-1\\u00B5s'", "Tag": "SET CLUSTER SETTING", "User": "root", "Value": "-00:00:00.000001"} 0 1 {"EventType": "set_cluster_setting", "SettingName": "kv.allocator.load_based_lease_rebalancing.enabled", "Statement": "SET CLUSTER SETTING \"kv.allocator.load_based_lease_rebalancing.enabled\" = false", "Tag": "SET CLUSTER SETTING", "User": "root", "Value": "false"} diff --git a/pkg/sql/logictest/testdata/logic_test/system b/pkg/sql/logictest/testdata/logic_test/system index 3b1f95a6c0d1..ed9747156e9e 100644 --- a/pkg/sql/logictest/testdata/logic_test/system +++ b/pkg/sql/logictest/testdata/logic_test/system @@ -1055,9 +1055,9 @@ GRANT ALL ON system.lease TO testuser # NB: the "order by" is necessary or this test is flaky under DistSQL. # This is somewhat surprising. -# The expectations are different on tenants because of the -# kv.range_merge_queue_enabled setting. -skipif config 3node-tenant +# With probabilistic SQL server creation, we have to filter out +# kv.range_merge.queue_enabled, since it will only be set in cases +# where a SQL server is not allocated. query T SELECT name FROM system.settings @@ -1067,11 +1067,11 @@ AND name NOT LIKE '%sql.defaults.vectorize%' AND name NOT LIKE '%sql.testing%' AND name NOT LIKE '%sql.defaults.experimental_distsql_planning%' AND name != 'sql.defaults.experimental_new_schema_changer.enabled' +AND name != 'kv.range_merge.queue_enabled' ORDER BY name ---- cluster.secret diagnostics.reporting.enabled -kv.range_merge.queue_enabled sql.crdb_internal.table_row_statistics.as_of_time sql.stats.automatic_collection.min_stale_rows version @@ -1097,20 +1097,19 @@ version statement ok INSERT INTO system.settings (name, value) VALUES ('somesetting', 'somevalue') -# The expectations are different on tenants because of the -# kv.range_merge_queue_enabled setting. -skipif config 3node-tenant +# Have to exclude kv.range_merge.queue_enabled as it is not accessible +# to SQL servers. query TT SELECT name, value FROM system.settings WHERE name NOT IN ('version', 'sql.defaults.distsql', 'cluster.secret', 'sql.stats.automatic_collection.enabled', 'sql.defaults.vectorize', 'sql.defaults.experimental_distsql_planning', - 'sql.defaults.experimental_new_schema_changer.enabled') + 'sql.defaults.experimental_new_schema_changer.enabled', + 'kv.range_merge.queue_enabled') ORDER BY name ---- diagnostics.reporting.enabled true -kv.range_merge.queue_enabled false somesetting somevalue sql.crdb_internal.table_row_statistics.as_of_time -1µs sql.stats.automatic_collection.min_stale_rows 5 diff --git a/pkg/sql/pgwire/pgerror/errors.go b/pkg/sql/pgwire/pgerror/errors.go index a98ee7a4a317..cb87f0a60c88 100644 --- a/pkg/sql/pgwire/pgerror/errors.go +++ b/pkg/sql/pgwire/pgerror/errors.go @@ -37,6 +37,9 @@ func (pg *Error) ErrorDetail() string { return pg.Detail } // FullError can be used when the hint and/or detail are to be tested. func FullError(err error) string { var errString string + if err == nil { + return "nil" + } if pqErr := (*pq.Error)(nil); errors.As(err, &pqErr) { errString = formatMsgHintDetail("pq", pqErr.Message, pqErr.Hint, pqErr.Detail) } else { diff --git a/pkg/sql/sqltestutils/telemetry.go b/pkg/sql/sqltestutils/telemetry.go index ee80a4380e86..a721ae40e4ff 100644 --- a/pkg/sql/sqltestutils/telemetry.go +++ b/pkg/sql/sqltestutils/telemetry.go @@ -88,6 +88,13 @@ func TelemetryTest(t *testing.T, serverArgs []base.TestServerArgs, testTenant bo defer cloudinfo.Disable()() var test telemetryTest + + // Disable the default SQL server as this test is validating that we're + // getting a locality optimized search plan, which is not currently + // supported in tenants. Tracked with #76378. + for i := 0; i < len(serverArgs); i++ { + serverArgs[i].DisableDefaultSQLServer = true + } test.Start(t, serverArgs) defer test.Close() diff --git a/pkg/testutils/serverutils/BUILD.bazel b/pkg/testutils/serverutils/BUILD.bazel index 51d0bb6e8d66..f4b94ce56a9d 100644 --- a/pkg/testutils/serverutils/BUILD.bazel +++ b/pkg/testutils/serverutils/BUILD.bazel @@ -19,6 +19,7 @@ go_library( "//pkg/server/status", "//pkg/settings/cluster", "//pkg/storage", + "//pkg/testutils/skip", "//pkg/testutils/sqlutils", "//pkg/util/hlc", "//pkg/util/httputil", diff --git a/pkg/testutils/serverutils/test_server_shim.go b/pkg/testutils/serverutils/test_server_shim.go index fe3dfef5614e..dcf8a0125710 100644 --- a/pkg/testutils/serverutils/test_server_shim.go +++ b/pkg/testutils/serverutils/test_server_shim.go @@ -20,7 +20,9 @@ package serverutils import ( "context" gosql "database/sql" + "math/rand" "net/url" + "strings" "testing" "github.com/cockroachdb/cockroach/pkg/base" @@ -30,6 +32,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/server/status" "github.com/cockroachdb/cockroach/pkg/storage" + "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/httputil" @@ -224,7 +227,21 @@ func StartServer( if err != nil { t.Fatalf("%+v", err) } + + // Determine if we should probabilistically start tenants for the cluster. + const probabilityOfStartingTestTenant = 0.5 + // If we haven't been asked to explicitly disable the test tenant, but we + // determine that we shouldn't be probabilistically starting the test + // tenant, then disable it explicitly here. + if !params.DisableDefaultSQLServer && rand.Float32() > probabilityOfStartingTestTenant { + params.DisableDefaultSQLServer = true + } + if err := server.Start(context.Background()); err != nil { + if strings.Contains(err.Error(), "requires a CCL binary") { + server.Stopper().Stop(context.Background()) + skip.IgnoreLint(t, "skipping due to lack of CCL binary") + } t.Fatalf("%+v", err) } goDB := OpenDBConn( @@ -293,6 +310,7 @@ func StartServerRaw(args base.TestServerArgs) (TestServerInterface, error) { return nil, err } if err := server.Start(context.Background()); err != nil { + server.Stopper().Stop(context.Background()) return nil, err } return server, nil diff --git a/pkg/testutils/testcluster/BUILD.bazel b/pkg/testutils/testcluster/BUILD.bazel index e6723a05e315..05a72e10df8e 100644 --- a/pkg/testutils/testcluster/BUILD.bazel +++ b/pkg/testutils/testcluster/BUILD.bazel @@ -19,6 +19,7 @@ go_library( "//pkg/storage", "//pkg/testutils", "//pkg/testutils/serverutils", + "//pkg/testutils/skip", "//pkg/util/contextutil", "//pkg/util/hlc", "//pkg/util/log", diff --git a/pkg/testutils/testcluster/testcluster.go b/pkg/testutils/testcluster/testcluster.go index cef34c0457c4..39cd6c7527b2 100644 --- a/pkg/testutils/testcluster/testcluster.go +++ b/pkg/testutils/testcluster/testcluster.go @@ -14,6 +14,7 @@ import ( "context" gosql "database/sql" "fmt" + "math/rand" "net" "reflect" "runtime" @@ -35,6 +36,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/storage" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" + "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/util/contextutil" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/log" @@ -301,6 +303,15 @@ func (tc *TestCluster) Start(t testing.TB) { errCh = make(chan error, nodes) } + // Determine if we should probabilistically start a SQL server for the + // cluster. + const probabilityOfStartingTestTenant = 0.5 + probabilisticallyStartTestSQLServer := true + startedTestSQLServer := true + if rand.Float32() > probabilityOfStartingTestTenant { + probabilisticallyStartTestSQLServer = false + } + disableLBS := false for i := 0; i < nodes; i++ { // Disable LBS if any server has a very low scan interval. @@ -308,12 +319,38 @@ func (tc *TestCluster) Start(t testing.TB) { disableLBS = true } + // If we're not probabilistically starting the test SQL server, disable + // its start and set the "started" flag accordingly. We need to do this + // with two separate if checks because the DisableDefaultSQLServer flag + // could have been set coming into this function by the caller. + if !probabilisticallyStartTestSQLServer { + tc.Servers[i].Cfg.DisableDefaultSQLServer = true + } + if tc.Servers[i].Cfg.DisableDefaultSQLServer { + if startedTestSQLServer && i > 0 { + t.Fatal(errors.Newf("starting only some nodes with a test SQL server is not"+ + "currently supported - attempted to disable SQL sever on node %d", i)) + } + startedTestSQLServer = false + } + if tc.clusterArgs.ParallelStart { go func(i int) { errCh <- tc.startServer(i, tc.serverArgs[i]) }(i) } else { if err := tc.startServer(i, tc.serverArgs[i]); err != nil { + if strings.Contains(err.Error(), "requires a CCL binary") { + if i != 0 { + t.Fatal(errors.Newf("failed to start server on node %d due to lack of "+ + "CCL binary after server started successfully on previous nodes", i)) + } + for j := 0; j < nodes; j++ { + tc.Servers[j].Stopper().Stop(context.TODO()) + } + tc.Stopper().Stop(context.TODO()) + skip.IgnoreLint(t, "skipping due to lack of CCL binary") + } t.Fatal(err) } // We want to wait for stores for each server in order to have predictable @@ -326,6 +363,13 @@ func (tc *TestCluster) Start(t testing.TB) { if tc.clusterArgs.ParallelStart { for i := 0; i < nodes; i++ { if err := <-errCh; err != nil { + if strings.Contains(err.Error(), "requires a CCL binary") { + for j := 0; j < nodes; j++ { + tc.Servers[j].Stopper().Stop(context.TODO()) + } + tc.Stopper().Stop(context.TODO()) + skip.IgnoreLint(t, "skipping due to lack of CCL binary") + } t.Fatal(err) } } @@ -333,7 +377,10 @@ func (tc *TestCluster) Start(t testing.TB) { tc.WaitForNStores(t, tc.NumServers(), tc.Servers[0].Gossip()) } - if tc.clusterArgs.ReplicationMode == base.ReplicationManual { + // No need to disable the merge queue for SQL servers, as they don't have + // access to that cluster setting (and ALTER TABLE ... SPLIT AT is not + // supported in SQL servers either). + if !startedTestSQLServer && tc.clusterArgs.ReplicationMode == base.ReplicationManual { // We've already disabled the merge queue via testing knobs above, but ALTER // TABLE ... SPLIT AT will throw an error unless we also disable merges via // the cluster setting. @@ -414,6 +461,10 @@ func (tc *TestCluster) AddAndStartServer(t testing.TB, serverArgs base.TestServe } if err := tc.startServer(len(tc.Servers)-1, serverArgs); err != nil { + if strings.Contains(err.Error(), "requires a CCL binary") { + tc.Stopper().Stop(context.TODO()) + skip.IgnoreLint(t, "skipping due to lack of CCL binary") + } t.Fatal(err) } } diff --git a/pkg/ui/workspaces/cluster-ui/src/core/colors.module.scss b/pkg/ui/workspaces/cluster-ui/src/core/colors.module.scss index 9f2277fd1814..8eef5360c24d 100644 --- a/pkg/ui/workspaces/cluster-ui/src/core/colors.module.scss +++ b/pkg/ui/workspaces/cluster-ui/src/core/colors.module.scss @@ -59,6 +59,7 @@ $colors--functional-orange-5: #764205; $colors--title: $colors--neutral-8; $colors--primary-text: $colors--neutral-7; $colors--secondary-text: $colors--neutral-6; +$colors--success: $colors--primary-green-3; $colors--disabled: $colors--neutral-5; $colors--link: $colors--primary-blue-3; $colors--white: $colors--neutral-0; diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.stories.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.stories.tsx index 738b9b5dadb8..c00140b4b5f9 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.stories.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.stories.tsx @@ -25,6 +25,7 @@ import { } from "./databaseDetailsPage"; import * as H from "history"; +import moment from "moment"; const history = H.createHashHistory(); const withLoadingIndicator: DatabaseDetailsPageProps = { @@ -107,6 +108,7 @@ const withData: DatabaseDetailsPageProps = { userCount: roles.length, roles: roles, grants: grants, + statsLastUpdated: moment("0001-01-01T00:00:00Z"), }, showNodeRegionsColumn: true, stats: { diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx index ecb01d305275..1ddde24a6c83 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx @@ -36,6 +36,8 @@ import { baseHeadingClasses, statisticsClasses, } from "src/transactionsPage/transactionsPageClasses"; +import { Moment } from "moment"; +import { formatDate } from "antd/es/date-picker/utils"; const cx = classNames.bind(styles); const sortableTableCx = classNames.bind(sortableTableStyles); @@ -101,6 +103,7 @@ export interface DatabaseDetailsPageDataTableDetails { userCount: number; roles: string[]; grants: string[]; + statsLastUpdated?: Moment; } export interface DatabaseDetailsPageDataTableStats { @@ -351,6 +354,26 @@ export class DatabaseDetailsPage extends React.Component< showByDefault: this.props.showNodeRegionsColumn, hideIfTenant: true, }, + { + title: ( + + Table Stats Last Updated (UTC) + + ), + cell: table => + !table.details.statsLastUpdated + ? "No table statistics found" + : formatDate( + table.details.statsLastUpdated, + "MMM DD, YYYY [at] h:mm A", + ), + sort: table => table.details.statsLastUpdated, + className: cx("database-table__col--table-stats"), + name: "tableStatsUpdated", + }, ]; } diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.stories.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.stories.tsx index d244bf4f8747..bb444ba90f9d 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.stories.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.stories.tsx @@ -26,6 +26,7 @@ const history = H.createHashHistory(); const withLoadingIndicator: DatabaseTablePageProps = { databaseName: randomName(), name: randomName(), + automaticStatsCollectionEnabled: true, details: { loading: true, loaded: false, @@ -33,6 +34,7 @@ const withLoadingIndicator: DatabaseTablePageProps = { replicaCount: 0, indexNames: [], grants: [], + statsLastUpdated: moment("0001-01-01T00:00:00Z"), }, stats: { loading: true, @@ -58,6 +60,7 @@ const withLoadingIndicator: DatabaseTablePageProps = { refreshTableStats: () => {}, refreshIndexStats: () => {}, resetIndexUsageStats: () => {}, + refreshSettings: () => {}, }; const name = randomName(); @@ -65,6 +68,7 @@ const name = randomName(); const withData: DatabaseTablePageProps = { databaseName: randomName(), name: name, + automaticStatsCollectionEnabled: true, details: { loading: false, loaded: true, @@ -89,6 +93,7 @@ const withData: DatabaseTablePageProps = { }; }), ), + statsLastUpdated: moment("0001-01-01T00:00:00Z"), }, showNodeRegionsSection: true, stats: { @@ -136,6 +141,7 @@ const withData: DatabaseTablePageProps = { refreshTableStats: () => {}, refreshIndexStats: () => {}, resetIndexUsageStats: () => {}, + refreshSettings: () => {}, }; storiesOf("Database Table Page", module) diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx index f1f797d3bfb1..705726c7a70b 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx @@ -16,14 +16,19 @@ import _ from "lodash"; import { Tooltip } from "antd"; import { Heading } from "@cockroachlabs/ui-components"; +import { Anchor } from "src/anchor"; import { Breadcrumbs } from "src/breadcrumbs"; import { CaretRight } from "src/icon/caretRight"; import { StackIcon } from "src/icon/stackIcon"; import { SqlBox } from "src/sql"; import { ColumnDescriptor, SortSetting, SortedTable } from "src/sortedtable"; -import { SummaryCard, SummaryCardItem } from "src/summaryCard"; +import { + SummaryCard, + SummaryCardItem, + SummaryCardItemBoolSetting, +} from "src/summaryCard"; import * as format from "src/util/format"; -import { syncHistory } from "src/util"; +import { syncHistory, tableStatsClusterSetting } from "src/util"; import styles from "./databaseTablePage.module.scss"; import { commonStyles } from "src/common"; @@ -32,7 +37,10 @@ import moment, { Moment } from "moment"; import { Search as IndexIcon } from "@cockroachlabs/icons"; import { formatDate } from "antd/es/date-picker/utils"; import { Link } from "react-router-dom"; +import classnames from "classnames/bind"; +import booleanSettingStyles from "../settings/booleanSetting.module.scss"; const cx = classNames.bind(styles); +const booleanSettingCx = classnames.bind(booleanSettingStyles); const { TabPane } = Tabs; @@ -84,6 +92,7 @@ export interface DatabaseTablePageData { stats: DatabaseTablePageDataStats; indexStats: DatabaseTablePageIndexStats; showNodeRegionsSection?: boolean; + automaticStatsCollectionEnabled: boolean; } export interface DatabaseTablePageDataDetails { @@ -93,6 +102,7 @@ export interface DatabaseTablePageDataDetails { replicaCount: number; indexNames: string[]; grants: Grant[]; + statsLastUpdated?: Moment; } export interface DatabaseTablePageIndexStats { @@ -125,6 +135,7 @@ export interface DatabaseTablePageDataStats { export interface DatabaseTablePageActions { refreshTableDetails: (database: string, table: string) => void; refreshTableStats: (database: string, table: string) => void; + refreshSettings: () => void; refreshIndexStats?: (database: string, table: string) => void; resetIndexUsageStats?: (database: string, table: string) => void; refreshNodes?: () => void; @@ -203,6 +214,10 @@ export class DatabaseTablePage extends React.Component< this.props.name, ); } + + if (this.props.refreshSettings != null) { + this.props.refreshSettings(); + } } minDate = moment.utc("0001-01-01"); // minimum value as per UTC @@ -359,6 +374,36 @@ export class DatabaseTablePage extends React.Component< label="Ranges" value={this.props.stats.rangeCount} /> + {this.props.details.statsLastUpdated && ( + + )} + + {" "} + Automatic statistics can help improve query + performance. Learn how to{" "} + + manage statistics collection + + . + + } + /> diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.stories.tsx b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.stories.tsx index d7514ed43df8..5107bbb00052 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.stories.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.stories.tsx @@ -22,6 +22,7 @@ const history = H.createHashHistory(); const withLoadingIndicator: DatabasesPageProps = { loading: true, loaded: false, + automaticStatsCollectionEnabled: true, databases: [], sortSetting: { ascending: false, @@ -29,6 +30,7 @@ const withLoadingIndicator: DatabasesPageProps = { }, onSortingChange: () => {}, refreshDatabases: () => {}, + refreshSettings: () => {}, refreshDatabaseDetails: () => {}, refreshTableStats: () => {}, location: history.location, @@ -44,6 +46,7 @@ const withLoadingIndicator: DatabasesPageProps = { const withoutData: DatabasesPageProps = { loading: false, loaded: true, + automaticStatsCollectionEnabled: true, databases: [], sortSetting: { ascending: false, @@ -51,6 +54,7 @@ const withoutData: DatabasesPageProps = { }, onSortingChange: () => {}, refreshDatabases: () => {}, + refreshSettings: () => {}, refreshDatabaseDetails: () => {}, refreshTableStats: () => {}, location: history.location, @@ -67,6 +71,7 @@ const withData: DatabasesPageProps = { loading: false, loaded: true, showNodeRegionsColumn: true, + automaticStatsCollectionEnabled: true, sortSetting: { ascending: false, columnTitle: "name", @@ -86,6 +91,7 @@ const withData: DatabasesPageProps = { }), onSortingChange: () => {}, refreshDatabases: () => {}, + refreshSettings: () => {}, refreshDatabaseDetails: () => {}, refreshTableStats: () => {}, location: history.location, diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx index be5443227fe0..ba9ee87f54a0 100644 --- a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx @@ -14,8 +14,10 @@ import { Tooltip } from "antd"; import classNames from "classnames/bind"; import _ from "lodash"; +import { Anchor } from "src/anchor"; import { StackIcon } from "src/icon/stackIcon"; import { Pagination, ResultsPerPageLabel } from "src/pagination"; +import { BooleanSetting } from "src/settings/booleanSetting"; import { ColumnDescriptor, ISortedTablePagination, @@ -30,10 +32,13 @@ import { baseHeadingClasses, statisticsClasses, } from "src/transactionsPage/transactionsPageClasses"; -import { syncHistory } from "../util"; +import { syncHistory, tableStatsClusterSetting } from "src/util"; +import classnames from "classnames/bind"; +import booleanSettingStyles from "../settings/booleanSetting.module.scss"; const cx = classNames.bind(styles); const sortableTableCx = classNames.bind(sortableTableStyles); +const booleanSettingCx = classnames.bind(booleanSettingStyles); // We break out separate interfaces for some of the nested objects in our data // both so that they can be available as SortedTable rows and for making @@ -67,6 +72,7 @@ export interface DatabasesPageData { loaded: boolean; databases: DatabasesPageDataDatabase[]; sortSetting: SortSetting; + automaticStatsCollectionEnabled: boolean; showNodeRegionsColumn?: boolean; } @@ -99,6 +105,7 @@ export interface DatabasesPageActions { refreshDatabases: () => void; refreshDatabaseDetails: (database: string) => void; refreshTableStats: (database: string, table: string) => void; + refreshSettings: () => void; refreshNodes?: () => void; onSortingChange?: ( name: string, @@ -160,6 +167,10 @@ export class DatabasesPage extends React.Component< this.props.refreshNodes(); } + if (this.props.refreshSettings != null) { + this.props.refreshSettings(); + } + if (!this.props.loaded && !this.props.loading) { return this.props.refreshDatabases(); } @@ -282,7 +293,28 @@ export class DatabasesPage extends React.Component< ); return (
-

Databases

+
+

Databases

+ + {" "} + Automatic statistics can help improve query performance. Learn + how to{" "} + + manage statistics collection + + . + + } + /> +

diff --git a/pkg/ui/workspaces/cluster-ui/src/settings/booleanSetting.module.scss b/pkg/ui/workspaces/cluster-ui/src/settings/booleanSetting.module.scss new file mode 100644 index 000000000000..e0f3aa935a80 --- /dev/null +++ b/pkg/ui/workspaces/cluster-ui/src/settings/booleanSetting.module.scss @@ -0,0 +1,36 @@ +@import "src/core/index.module"; + +.crl-hover-text { + font-size: 12px; + + &__dashed-underline { + color: inherit; + border-bottom-width: 1px; + border-bottom-style: dashed; + border-bottom-color: inherit; + cursor: default; + } + + &__link-text { + color: inherit; + font-size: inherit; + } +} + +.bool-setting-icon { + + &__enabled { + fill: $colors--success; + margin-right: 8px; + height: 8px; + width: 8px; + } + + &__disabled { + fill: $colors--disabled; + margin-right: 8px; + height: 8px; + width: 8px; + } +} + diff --git a/pkg/ui/workspaces/cluster-ui/src/settings/booleanSetting.tsx b/pkg/ui/workspaces/cluster-ui/src/settings/booleanSetting.tsx new file mode 100644 index 000000000000..6edf739f617a --- /dev/null +++ b/pkg/ui/workspaces/cluster-ui/src/settings/booleanSetting.tsx @@ -0,0 +1,43 @@ +// 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. + +import * as React from "react"; +import { CircleFilled } from "src/icon"; +import { Tooltip } from "antd"; +import classNames from "classnames/bind"; +import styles from "./booleanSetting.module.scss"; + +const cx = classNames.bind(styles); + +export interface BooleanSettingProps { + text: string; + enabled: boolean; + tooltipText: JSX.Element; +} + +export function BooleanSetting(props: BooleanSettingProps) { + const { text, enabled, tooltipText } = props; + const label = enabled ? "enabled" : "disabled"; + const boolClass = enabled + ? "bool-setting-icon__enabled" + : "bool-setting-icon__disabled"; + return ( +
+ + + {text} - {label} + +
+ ); +} diff --git a/pkg/ui/workspaces/cluster-ui/src/settings/index.ts b/pkg/ui/workspaces/cluster-ui/src/settings/index.ts new file mode 100644 index 000000000000..7cbedc8da889 --- /dev/null +++ b/pkg/ui/workspaces/cluster-ui/src/settings/index.ts @@ -0,0 +1,11 @@ +// 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. + +export * from "./booleanSetting"; diff --git a/pkg/ui/workspaces/cluster-ui/src/summaryCard/index.tsx b/pkg/ui/workspaces/cluster-ui/src/summaryCard/index.tsx index ac396f426944..0ee386e18416 100644 --- a/pkg/ui/workspaces/cluster-ui/src/summaryCard/index.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/summaryCard/index.tsx @@ -11,6 +11,9 @@ import React from "react"; import classnames from "classnames/bind"; import styles from "./summaryCard.module.scss"; +import booleanSettingStyles from "../settings/booleanSetting.module.scss"; +import { CircleFilled } from "src/icon"; +import { Tooltip } from "antd"; interface ISummaryCardProps { children: React.ReactNode; @@ -18,6 +21,7 @@ interface ISummaryCardProps { } const cx = classnames.bind(styles); +const booleanSettingCx = classnames.bind(booleanSettingStyles); // tslint:disable-next-line: variable-name export const SummaryCard: React.FC = ({ @@ -31,6 +35,10 @@ interface ISummaryCardItemProps { className?: string; } +interface ISummaryCardItemBoolSettingProps extends ISummaryCardItemProps { + toolTipText: JSX.Element; +} + export const SummaryCardItem: React.FC = ({ label, value, @@ -41,3 +49,31 @@ export const SummaryCardItem: React.FC = ({

{value}

); + +export const SummaryCardItemBoolSetting: React.FC = ({ + label, + value, + toolTipText, + className, +}) => { + const boolValue = value ? "Enabled" : "Disabled"; + const boolClass = value + ? "bool-setting-icon__enabled" + : "bool-setting-icon__disabled"; + + return ( +
+

{label}

+

+ + + {boolValue} + +

+
+ ); +}; diff --git a/pkg/ui/workspaces/cluster-ui/src/util/docs.ts b/pkg/ui/workspaces/cluster-ui/src/util/docs.ts index ae781ce20ebd..258083b4083e 100644 --- a/pkg/ui/workspaces/cluster-ui/src/util/docs.ts +++ b/pkg/ui/workspaces/cluster-ui/src/util/docs.ts @@ -84,6 +84,9 @@ export const reviewOfCockroachTerminology = docsURL( "ui-replication-dashboard.html#review-of-cockroachdb-terminology", ); export const sessionsTable = docsURL("ui-sessions-page.html"); +export const tableStatsClusterSetting = docsURL( + "cost-based-optimizer.html#control-automatic-statistics", +); // Note that these explicitly don't use the current version, since we want to // link to the most up-to-date documentation available. export const upgradeCockroachVersion = diff --git a/pkg/ui/workspaces/db-console/src/redux/clusterSettings/clusterSettings.selectors.ts b/pkg/ui/workspaces/db-console/src/redux/clusterSettings/clusterSettings.selectors.ts index 1d9908649bba..62d0e3601e27 100644 --- a/pkg/ui/workspaces/db-console/src/redux/clusterSettings/clusterSettings.selectors.ts +++ b/pkg/ui/workspaces/db-console/src/redux/clusterSettings/clusterSettings.selectors.ts @@ -41,3 +41,14 @@ export const selectResolution30mStorageTTL = createSelector( return util.durationFromISO8601String(value); }, ); + +export const selectAutomaticStatsCollectionEnabled = createSelector( + selectClusterSettings, + (settings): boolean | undefined => { + if (!settings) { + return undefined; + } + const value = settings["sql.stats.automatic_collection.enabled"]?.value; + return value === "true"; + }, +); diff --git a/pkg/ui/workspaces/db-console/src/test-utils/assertDeepStrictEqual.ts b/pkg/ui/workspaces/db-console/src/test-utils/assertDeepStrictEqual.ts new file mode 100644 index 000000000000..85a052c48ac6 --- /dev/null +++ b/pkg/ui/workspaces/db-console/src/test-utils/assertDeepStrictEqual.ts @@ -0,0 +1,21 @@ +// 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. + +import assert from "assert"; + +export function assertDeepStrictEqual(expected: T, actual: T) { + assert.deepStrictEqual(actual, expected, errorMessage(expected, actual)); +} + +function errorMessage(expected: any, actual: any): string { + return `expected: ${JSON.stringify(expected)} but was ${JSON.stringify( + actual, + )}`; +} diff --git a/pkg/ui/workspaces/db-console/src/test-utils/index.ts b/pkg/ui/workspaces/db-console/src/test-utils/index.ts index 5058ecfa6db4..ac335940131a 100644 --- a/pkg/ui/workspaces/db-console/src/test-utils/index.ts +++ b/pkg/ui/workspaces/db-console/src/test-utils/index.ts @@ -9,3 +9,4 @@ // licenses/APL.txt. export * from "./connectedMount"; +export * from "./assertDeepStrictEqual"; diff --git a/pkg/ui/workspaces/db-console/src/util/fakeApi.ts b/pkg/ui/workspaces/db-console/src/util/fakeApi.ts index b6d29d961000..f3cfe69e5eb7 100644 --- a/pkg/ui/workspaces/db-console/src/util/fakeApi.ts +++ b/pkg/ui/workspaces/db-console/src/util/fakeApi.ts @@ -17,6 +17,7 @@ import fetchMock from "src/util/fetch-mock"; const { DatabasesResponse, DatabaseDetailsResponse, + SettingsResponse, TableDetailsResponse, TableStatsResponse, TableIndexStatsResponse, @@ -54,6 +55,16 @@ export function restore() { fetchMock.restore(); } +export function stubClusterSettings( + response: cockroach.server.serverpb.ISettingsResponse, +) { + stubGet( + "/settings?unredacted_values=true", + SettingsResponse.encode(response), + API_PREFIX, + ); +} + export function stubDatabases( response: cockroach.server.serverpb.IDatabasesResponse, ) { diff --git a/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.spec.ts b/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.spec.ts index 89624b130c62..75d43dd9a2e4 100644 --- a/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.spec.ts +++ b/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.spec.ts @@ -8,7 +8,6 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -import assert from "assert"; import { createMemoryHistory } from "history"; import _ from "lodash"; import Long from "long"; @@ -19,6 +18,7 @@ import { DatabaseDetailsPageData, DatabaseDetailsPageDataTableDetails, DatabaseDetailsPageDataTableStats, + util, ViewMode, } from "@cockroachlabs/cluster-ui"; @@ -26,6 +26,8 @@ import { AdminUIState, createAdminUIStore } from "src/redux/state"; import { databaseNameAttr } from "src/util/constants"; import * as fakeApi from "src/util/fakeApi"; import { mapStateToProps, mapDispatchToProps } from "./redux"; +import { makeTimestamp } from "src/views/databases/utils"; +import { assertDeepStrictEqual } from "src/test-utils"; function fakeRouteComponentProps( key: string, @@ -67,26 +69,26 @@ class TestDriver { } assertProperties(expected: DatabaseDetailsPageData) { - assert.deepEqual(this.properties(), expected); + assertDeepStrictEqual(expected, this.properties()); } assertTableDetails( name: string, expected: DatabaseDetailsPageDataTableDetails, ) { - assert.deepEqual(this.findTable(name).details, expected); + assertDeepStrictEqual(expected, this.findTable(name).details); } assertTableRoles(name: string, expected: string[]) { - assert.deepEqual(this.findTable(name).details.roles, expected); + assertDeepStrictEqual(expected, this.findTable(name).details.roles); } assertTableGrants(name: string, expected: string[]) { - assert.deepEqual(this.findTable(name).details.grants, expected); + assertDeepStrictEqual(expected, this.findTable(name).details.grants); } assertTableStats(name: string, expected: DatabaseDetailsPageDataTableStats) { - assert.deepEqual(this.findTable(name).stats, expected); + assertDeepStrictEqual(expected, this.findTable(name).stats); } async refreshDatabaseDetails() { @@ -159,6 +161,7 @@ describe("Database Details Page", function() { userCount: 0, roles: [], grants: [], + statsLastUpdated: null, }, stats: { loading: false, @@ -178,6 +181,7 @@ describe("Database Details Page", function() { userCount: 0, roles: [], grants: [], + statsLastUpdated: null, }, stats: { loading: false, @@ -250,6 +254,7 @@ describe("Database Details Page", function() { implicit: false, }, ], + stats_last_created_at: makeTimestamp("0001-01-01T00:00:00Z"), }); fakeApi.stubTableDetails("things", "bar", { @@ -298,6 +303,7 @@ describe("Database Details Page", function() { implicit: false, }, ], + stats_last_created_at: makeTimestamp("0001-01-01T00:00:00Z"), }); await driver.refreshDatabaseDetails(); @@ -312,6 +318,9 @@ describe("Database Details Page", function() { userCount: 2, roles: ["admin", "public"], grants: ["CREATE", "SELECT"], + statsLastUpdated: util.TimestampToMoment( + makeTimestamp("0001-01-01T00:00:00Z"), + ), }); driver.assertTableDetails("bar", { @@ -322,6 +331,9 @@ describe("Database Details Page", function() { userCount: 3, roles: ["root", "app", "data"], grants: ["ALL", "SELECT", "INSERT"], + statsLastUpdated: util.TimestampToMoment( + makeTimestamp("0001-01-01T00:00:00Z"), + ), }); }); diff --git a/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.ts b/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.ts index 96052d4a1a5a..bcad3caf069e 100644 --- a/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.ts +++ b/pkg/ui/workspaces/db-console/src/views/databases/databaseDetailsPage/redux.ts @@ -12,7 +12,11 @@ import { RouteComponentProps } from "react-router"; import { createSelector } from "reselect"; import { LocalSetting } from "src/redux/localsettings"; import _ from "lodash"; -import { DatabaseDetailsPageData, ViewMode } from "@cockroachlabs/cluster-ui"; +import { + DatabaseDetailsPageData, + util, + ViewMode, +} from "@cockroachlabs/cluster-ui"; import { cockroach } from "src/js/protos"; import { @@ -30,6 +34,7 @@ import { selectIsMoreThanOneNode, } from "src/redux/nodes"; import { getNodesByRegionString } from "../utils"; + const { DatabaseDetailsRequest, TableDetailsRequest, @@ -132,7 +137,6 @@ export const mapStateToProps = createSelector( const numIndexes = _.uniq( _.map(details?.data?.indexes, index => index.name), ).length; - return { name: table, details: { @@ -143,6 +147,9 @@ export const mapStateToProps = createSelector( userCount: roles.length, roles: roles, grants: grants, + statsLastUpdated: details?.data?.stats_last_created_at + ? util.TimestampToMoment(details?.data?.stats_last_created_at) + : null, }, stats: { loading: !!stats?.inFlight, diff --git a/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.spec.ts b/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.spec.ts index ef1a4e859d78..42ab5bf7686f 100644 --- a/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.spec.ts +++ b/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.spec.ts @@ -8,7 +8,6 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -import assert from "assert"; import { createMemoryHistory } from "history"; import Long from "long"; import { RouteComponentProps } from "react-router-dom"; @@ -26,8 +25,8 @@ import { AdminUIState, createAdminUIStore } from "src/redux/state"; import { databaseNameAttr, tableNameAttr } from "src/util/constants"; import * as fakeApi from "src/util/fakeApi"; import { mapStateToProps, mapDispatchToProps } from "./redux"; -import moment from "moment"; import { makeTimestamp } from "src/views/databases/utils"; +import { assertDeepStrictEqual } from "src/test-utils"; function fakeRouteComponentProps( k1: string, @@ -86,21 +85,22 @@ class TestDriver { ) { // Assert moments are equal if not in pre-loading state. if (compareTimestamps) { - this.properties().indexStats.lastReset.isSame( + assertDeepStrictEqual( expected.indexStats.lastReset, + this.properties().indexStats.lastReset, ); } delete this.properties().indexStats.lastReset; delete expected.indexStats.lastReset; - assert.deepStrictEqual(this.properties(), expected); + assertDeepStrictEqual(expected, this.properties()); } assertTableDetails(expected: DatabaseTablePageDataDetails) { - assert.deepStrictEqual(this.properties().details, expected); + assertDeepStrictEqual(expected, this.properties().details); } assertTableStats(expected: DatabaseTablePageDataStats) { - assert.deepStrictEqual(this.properties().stats, expected); + assertDeepStrictEqual(expected, this.properties().stats); } assertIndexStats( @@ -109,22 +109,27 @@ class TestDriver { ) { // Assert moments are equal if not in pre-loading state. if (compareTimestamps) { - assert( - this.properties().indexStats.stats[0].lastUsed.isSame( - expected.stats[0].lastUsed, - ), + assertDeepStrictEqual( + expected.stats[0].lastUsed, + this.properties().indexStats.stats[0].lastUsed, ); } delete this.properties().indexStats.stats[0].lastUsed; delete expected.stats[0].lastUsed; - assert(this.properties().indexStats.lastReset.isSame(expected.lastReset)); + assertDeepStrictEqual( + expected.lastReset, + this.properties().indexStats.lastReset, + ); delete this.properties().indexStats.lastReset; delete expected.lastReset; // Assert objects without moments are equal. - assert.deepStrictEqual(this.properties().indexStats, expected); + assertDeepStrictEqual(expected, this.properties().indexStats); } + async refreshSettings() { + return this.actions.refreshSettings(); + } async refreshTableDetails() { return this.actions.refreshTableDetails(this.database, this.table); } @@ -153,7 +158,15 @@ describe("Database Table Page", function() { fakeApi.restore(); }); - it("starts in a pre-loading state", function() { + it("starts in a pre-loading state", async function() { + fakeApi.stubClusterSettings({ + key_values: { + "sql.stats.automatic_collection.enabled": { value: "true" }, + }, + }); + + await driver.refreshSettings(); + driver.assertProperties( { databaseName: "DATABASE", @@ -166,7 +179,9 @@ describe("Database Table Page", function() { replicaCount: 0, indexNames: [], grants: [], + statsLastUpdated: null, }, + automaticStatsCollectionEnabled: true, stats: { loading: false, loaded: false, @@ -178,7 +193,7 @@ describe("Database Table Page", function() { loading: false, loaded: false, stats: [], - lastReset: moment(), + lastReset: null, }, }, false, @@ -200,6 +215,7 @@ describe("Database Table Page", function() { zone_config: { num_replicas: 5, }, + stats_last_created_at: makeTimestamp("0001-01-01T00:00:00Z"), }); await driver.refreshTableDetails(); @@ -215,6 +231,9 @@ describe("Database Table Page", function() { { user: "admin", privilege: "DROP" }, { user: "public", privilege: "SELECT" }, ], + statsLastUpdated: util.TimestampToMoment( + makeTimestamp("0001-01-01T00:00:00Z"), + ), }); }); diff --git a/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.ts b/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.ts index f68857437327..9b7de1ecee7f 100644 --- a/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.ts +++ b/pkg/ui/workspaces/db-console/src/views/databases/databaseTablePage/redux.ts @@ -20,6 +20,7 @@ import { refreshTableStats, refreshNodes, refreshIndexStats, + refreshSettings, } from "src/redux/apiReducers"; import { AdminUIState } from "src/redux/state"; import { databaseNameAttr, tableNameAttr } from "src/util/constants"; @@ -31,6 +32,7 @@ import { } from "src/redux/nodes"; import { getNodesByRegionString } from "../utils"; import { resetIndexUsageStatsAction } from "src/redux/indexUsageStats"; +import { selectAutomaticStatsCollectionEnabled } from "src/redux/clusterSettings"; const { TableDetailsRequest, @@ -49,6 +51,7 @@ export const mapStateToProps = createSelector( state => state.cachedData.indexStats, state => nodeRegionsByIDSelector(state), state => selectIsMoreThanOneNode(state), + state => selectAutomaticStatsCollectionEnabled(state), ( database, @@ -58,6 +61,7 @@ export const mapStateToProps = createSelector( indexUsageStats, nodeRegions, showNodeRegionsSection, + automaticStatsCollectionEnabled, ): DatabaseTablePageData => { const details = tableDetails[generateTableID(database, table)]; const stats = tableStats[generateTableID(database, table)]; @@ -102,8 +106,12 @@ export const mapStateToProps = createSelector( replicaCount: details?.data?.zone_config?.num_replicas || 0, indexNames: _.uniq(_.map(details?.data?.indexes, index => index.name)), grants: grants, + statsLastUpdated: details?.data?.stats_last_created_at + ? util.TimestampToMoment(details?.data?.stats_last_created_at) + : null, }, showNodeRegionsSection, + automaticStatsCollectionEnabled, stats: { loading: !!stats?.inFlight, loaded: !!stats?.valid, @@ -139,4 +147,6 @@ export const mapDispatchToProps = { resetIndexUsageStats: resetIndexUsageStatsAction, refreshNodes, + + refreshSettings, }; diff --git a/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.spec.ts b/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.spec.ts index 03765a96aaff..fe1540f68320 100644 --- a/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.spec.ts +++ b/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.spec.ts @@ -8,7 +8,6 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -import assert from "assert"; import { createMemoryHistory } from "history"; import _ from "lodash"; import Long from "long"; @@ -22,7 +21,8 @@ import { import { AdminUIState, createAdminUIStore } from "src/redux/state"; import * as fakeApi from "src/util/fakeApi"; -import { mapStateToProps, mapDispatchToProps } from "./redux"; +import { mapDispatchToProps, mapStateToProps } from "./redux"; +import { assertDeepStrictEqual } from "src/test-utils"; class TestDriver { private readonly actions: DatabasesPageActions; @@ -48,15 +48,19 @@ class TestDriver { return this.actions.refreshTableStats(database, table); } + async refreshSettings() { + return this.actions.refreshSettings(); + } + assertProperties(expected: DatabasesPageData) { - assert.deepEqual(this.properties(), expected); + assertDeepStrictEqual(expected, this.properties()); } assertDatabaseProperties( database: string, expected: DatabasesPageDataDatabase, ) { - assert.deepEqual(this.findDatabase(database), expected); + assertDeepStrictEqual(expected, this.findDatabase(database)); } assertMissingTableProperties( @@ -64,9 +68,9 @@ class TestDriver { table: string, expected: DatabasesPageDataMissingTable, ) { - assert.deepEqual( - this.findMissingTable(this.findDatabase(database), table), + assertDeepStrictEqual( expected, + this.findMissingTable(this.findDatabase(database), table), ); } @@ -91,11 +95,20 @@ describe("Databases Page", function() { }); it("starts in a pre-loading state", async function() { + fakeApi.stubClusterSettings({ + key_values: { + "sql.stats.automatic_collection.enabled": { value: "true" }, + }, + }); + + await driver.refreshSettings(); + driver.assertProperties({ loading: false, loaded: false, databases: [], sortSetting: { ascending: true, columnTitle: "name" }, + automaticStatsCollectionEnabled: true, showNodeRegionsColumn: false, }); }); @@ -104,8 +117,14 @@ describe("Databases Page", function() { fakeApi.stubDatabases({ databases: ["system", "test"], }); + fakeApi.stubClusterSettings({ + key_values: { + "sql.stats.automatic_collection.enabled": { value: "true" }, + }, + }); await driver.refreshDatabases(); + await driver.refreshSettings(); driver.assertProperties({ loading: false, @@ -134,6 +153,7 @@ describe("Databases Page", function() { ], sortSetting: { ascending: true, columnTitle: "name" }, showNodeRegionsColumn: false, + automaticStatsCollectionEnabled: true, }); }); diff --git a/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.ts b/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.ts index 3034237e2d0d..7879a2a778d8 100644 --- a/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.ts +++ b/pkg/ui/workspaces/db-console/src/views/databases/databasesPage/redux.ts @@ -23,6 +23,7 @@ import { refreshDatabaseDetails, refreshTableStats, refreshNodes, + refreshSettings, } from "src/redux/apiReducers"; import { AdminUIState } from "src/redux/state"; import { FixLong } from "src/util/fixLong"; @@ -31,6 +32,7 @@ import { selectIsMoreThanOneNode, } from "src/redux/nodes"; import { getNodesByRegionString } from "../utils"; +import { selectAutomaticStatsCollectionEnabled } from "src/redux/clusterSettings"; const { DatabaseDetailsRequest, TableStatsRequest } = cockroach.server.serverpb; @@ -123,10 +125,12 @@ export const mapStateToProps = (state: AdminUIState): DatabasesPageData => ({ loaded: selectLoaded(state), databases: selectDatabases(state), sortSetting: sortSettingLocalSetting.selector(state), + automaticStatsCollectionEnabled: selectAutomaticStatsCollectionEnabled(state), showNodeRegionsColumn: selectIsMoreThanOneNode(state), }); export const mapDispatchToProps = { + refreshSettings, refreshDatabases, refreshDatabaseDetails: (database: string) => { return refreshDatabaseDetails(