From bec90b3ec8730d18000acfecf9c90a07fb80903f Mon Sep 17 00:00:00 2001 From: Adam Storm Date: Tue, 23 Nov 2021 13:02:01 -0500 Subject: [PATCH] multitenant: randomly run tests with tenants Previously, testServer would run by default in single-tenant mode. This PR changes testServer to run by default in multi-tenant mode, and runs all statements through the default test tenant. Since we want the bulk of our testing to remain in single-tenant mode, we only probabilistically run with tenants (with probability 0.25). This commit also contains a few pieces of cleanup which were necessary to make the testing changes (and were difficult to split into a separate commit): - Change the "Existing" flag on tenant start to "DisableCreateTenant" and make the tenant start code more tolerant of existing tenants by checking to see if the tenant exists before trying to create it. The DisableCreateTenant flag is required for the cases where we want a test to fail due to the lack of a created tenant. - Changed the multi-region backup tests to use node 0 instead of node 1, to allow them to work within a tenant. Tenants aren't able to access remote nodes of nodelocal stoarge. - Cleaned up a couple of cases where our error handling was passing nil structs to be printed out. - Had to special case the setting of kv.range_merge.queue_enabled because it is only available to the system SQL server. As a result, all tests which check for its setting had to be modified to no longer check for it (since it will only be set in the case where we have no SQL server). Additionally, we've created a tracking issue for all tests which currently don't work under the default test tenant (#76378). Release note: None --- pkg/base/test_server_args.go | 12 +- pkg/ccl/backupccl/backup_test.go | 78 +++++++++--- .../backupccl/create_scheduled_backup_test.go | 3 + .../full_cluster_backup_restore_test.go | 10 +- pkg/ccl/backupccl/helpers_test.go | 17 +++ ...lic_schema_namespace_entry_restore_test.go | 5 +- .../restore_mid_schema_change_test.go | 7 +- .../backupccl/restore_old_versions_test.go | 24 +++- pkg/ccl/backupccl/system_schema_test.go | 8 +- pkg/ccl/changefeedccl/changefeed_test.go | 11 +- pkg/ccl/changefeedccl/helpers_test.go | 11 +- pkg/ccl/cliccl/debug_backup_test.go | 16 ++- pkg/ccl/importccl/exportcsv_test.go | 22 ++-- pkg/ccl/importccl/import_into_test.go | 7 +- pkg/ccl/importccl/import_processor_test.go | 6 + pkg/ccl/importccl/import_stmt_test.go | 57 +++++++-- pkg/ccl/importccl/read_import_mysql_test.go | 5 +- .../jobs_protected_ts_test.go | 6 + pkg/ccl/kvccl/kvtenantccl/tenant_kv_test.go | 3 + .../kvccl/kvtenantccl/tenant_trace_test.go | 2 + .../kvccl/kvtenantccl/tenant_upgrade_test.go | 18 +-- .../testdata/logic_test/multi_region_backup | 54 ++++---- .../logic_test/multi_region_import_export | 4 +- .../testdata/logic_test/tenant_usage | 4 +- pkg/ccl/multiregionccl/datadriven_test.go | 6 + .../multiregionccltestutils/testutils.go | 6 + .../multiregionccl/regional_by_row_test.go | 5 + pkg/ccl/multiregionccl/unique_test.go | 4 +- .../tenantcostclient/tenant_side_test.go | 5 +- pkg/ccl/serverccl/admin_test.go | 10 +- pkg/ccl/serverccl/role_authentication_test.go | 9 +- pkg/ccl/serverccl/server_sql_test.go | 12 +- .../serverccl/statusccl/tenant_grpc_test.go | 1 - .../serverccl/statusccl/tenant_status_test.go | 4 + .../serverccl/statusccl/tenant_test_utils.go | 6 +- .../tenant_decommissioned_host_test.go | 1 - .../spanconfigcomparedccl/datadriven_test.go | 3 + .../datadriven_test.go | 3 + .../datadriven_test.go | 3 + .../sqlwatcher_test.go | 10 ++ pkg/ccl/sqlproxyccl/proxy_handler_test.go | 61 +++++++-- pkg/ccl/sqlproxyccl/tenant/directory_test.go | 17 ++- ...kroach_sinkless_replication_client_test.go | 9 +- .../partitioned_stream_client_test.go | 3 + .../streamingest/stream_ingestion_job_test.go | 33 +++-- .../streamingest/stream_ingestion_test.go | 7 +- .../streamproducer/producer_job_test.go | 3 + .../streamproducer/replication_stream_test.go | 21 +++- .../testccl/sqlccl/temp_table_clean_test.go | 2 - pkg/ccl/workloadccl/allccl/all_test.go | 12 +- pkg/ccl/workloadccl/fixture_test.go | 15 ++- pkg/cli/BUILD.bazel | 1 + pkg/cli/democluster/demo_cluster.go | 3 + pkg/cli/democluster/demo_cluster_test.go | 2 + pkg/cli/testutils.go | 9 +- .../kvcoord/txn_interceptor_span_refresher.go | 4 +- pkg/kv/kvclient/kvtenant/BUILD.bazel | 2 + pkg/kv/kvclient/kvtenant/connector.go | 6 +- pkg/kv/kvserver/replicate_queue_test.go | 1 + pkg/server/config.go | 3 + pkg/server/server_test.go | 2 +- .../test_system_config_watcher.go | 8 +- pkg/server/tenant.go | 3 +- pkg/server/testserver.go | 118 ++++++++++++++++-- pkg/sql/flowinfra/cluster_test.go | 1 - pkg/sql/logictest/logic.go | 20 ++- .../logictest/testdata/logic_test/event_log | 5 +- pkg/sql/logictest/testdata/logic_test/system | 17 ++- pkg/sql/pgwire/pgerror/errors.go | 3 + pkg/sql/sqltestutils/telemetry.go | 7 ++ pkg/testutils/serverutils/BUILD.bazel | 1 + pkg/testutils/serverutils/test_server_shim.go | 18 +++ pkg/testutils/testcluster/BUILD.bazel | 1 + pkg/testutils/testcluster/testcluster.go | 53 +++++++- 74 files changed, 775 insertions(+), 174 deletions(-) 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) } }