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 e7b2275cbf3b..8fbe0d7e2a1d 100644 --- a/pkg/ccl/backupccl/backup_test.go +++ b/pkg/ccl/backupccl/backup_test.go @@ -169,6 +169,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 { @@ -1179,7 +1183,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]) @@ -5776,7 +5785,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` @@ -6254,6 +6265,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 { @@ -6721,6 +6735,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{ @@ -6772,6 +6789,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) @@ -7177,7 +7197,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) @@ -7198,7 +7221,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) @@ -7240,7 +7263,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) @@ -7266,7 +7289,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]) @@ -7279,7 +7307,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) @@ -7290,7 +7318,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) @@ -7308,7 +7341,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) @@ -7317,7 +7350,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) @@ -7327,7 +7360,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]) @@ -7335,7 +7373,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) @@ -7345,7 +7383,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]) @@ -7353,7 +7396,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) @@ -8262,6 +8305,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{ @@ -9024,6 +9069,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 53e53802d9b8..50c75377cdf5 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 index b8cadf06c715..72ce123dd1d0 100644 --- a/pkg/ccl/backupccl/helpers_test.go +++ b/pkg/ccl/backupccl/helpers_test.go @@ -60,11 +60,20 @@ func backupRestoreTestSetupWithParams( 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 } } @@ -184,11 +193,16 @@ func backupRestoreTestSetupEmptyWithParams( ) (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 } } @@ -212,6 +226,9 @@ func createEmptyCluster( 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]) 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 4d94fdfcccc5..f69e5eeb0880 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 fc2580bdc03c..e8f94d909d6a 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 6b497c9573da..74392ffab35c 100644 --- a/pkg/ccl/importccl/exportcsv_test.go +++ b/pkg/ccl/importccl/exportcsv_test.go @@ -59,9 +59,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, }, }, ) @@ -419,8 +421,11 @@ func TestRandomParquetExports(t *testing.T) { params := base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - UseDatabase: dbName, - ExternalIODir: dir, + // Test fails when run in the SQL server. More investigation is + // required. Tracked with #76378. + DisableDefaultSQLServer: true, + UseDatabase: dbName, + ExternalIODir: dir, }, } ctx := context.Background() @@ -484,8 +489,11 @@ func TestBasicParquetTypes(t *testing.T) { dbName := "baz" params := base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - UseDatabase: dbName, - ExternalIODir: dir, + // Test fails when run in the SQL server. More investigation is + // required. Tracked with #76378. + 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 0e85dc64e406..9cc31eb7105e 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 d18d470cdf65..172a0aacaa43 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 368522e923a4..b48feea4b774 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 f59438e21c8a..eccf3fc8e75e 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/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/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/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..a9668a12852c 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 { @@ -530,7 +599,7 @@ func (t *TestTenant) HTTPAddr() string { // RPCAddr is part of the TestTenantInterface interface. func (t *TestTenant) RPCAddr() string { - // The RPC and SQL functionality for tenants is multiplexed + // The RPC and SQL functionality for SQLServers is multiplexed // on the same address. Having a separate interface to access // for the two addresses makes it easier to distinguish // the use case for which the address is being used. @@ -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.SQLServers 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", + "SELECT 1 FROM system.SQLServers 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 c5d029c22e46..1022fd74eeb4 100644 --- a/pkg/sql/flowinfra/cluster_test.go +++ b/pkg/sql/flowinfra/cluster_test.go @@ -338,7 +338,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) } }