diff --git a/pkg/server/api_v2_sql_schema_test.go b/pkg/server/api_v2_sql_schema_test.go index cb9bdc1978fe..67bd9d13bdf1 100644 --- a/pkg/server/api_v2_sql_schema_test.go +++ b/pkg/server/api_v2_sql_schema_test.go @@ -20,7 +20,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/server/serverpb" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" - "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/stretchr/testify/require" @@ -62,7 +61,6 @@ func TestUsersV2(t *testing.T) { func TestDatabasesTablesV2(t *testing.T) { defer leaktest.AfterTest(t)() - skip.UnderRaceWithIssue(t, 87074, "flaky test") defer log.Scope(t).Close(t) testCluster := serverutils.StartNewTestCluster(t, 3, base.TestClusterArgs{}) diff --git a/pkg/server/server_controller_sql.go b/pkg/server/server_controller_sql.go index 218e7e81a24d..0c1d112494c7 100644 --- a/pkg/server/server_controller_sql.go +++ b/pkg/server/server_controller_sql.go @@ -50,6 +50,10 @@ func (c *serverController) sqlMux( // The concurrent dispatch gives a chance to the succeeding // servers to see and process the cancel at approximately the // same time as every other. + // + // Cancel requests are unauthenticated so run the cancel async to prevent + // the client from deriving any info about the cancel based on how long it + // takes. if err := c.stopper.RunAsyncTask(ctx, "cancel", func(ctx context.Context) { s.handleCancel(ctx, status.CancelKey) }); err != nil { diff --git a/pkg/server/server_sql.go b/pkg/server/server_sql.go index 4b3fbecc1e3d..c00a4f60d728 100644 --- a/pkg/server/server_sql.go +++ b/pkg/server/server_sql.go @@ -910,7 +910,7 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) { // The collector requires nodeliveness to get a list of all the nodes in the // cluster. var getNodes func(ctx context.Context) ([]roachpb.NodeID, error) - if isMixedSQLAndKVNode { + if isMixedSQLAndKVNode && hasNodeLiveness { // TODO(dt): any reason not to just always use the instance reader? And just // pass it directly instead of making a new closure here? getNodes = func(ctx context.Context) ([]roachpb.NodeID, error) { diff --git a/pkg/server/tenant.go b/pkg/server/tenant.go index a42a1dca5a7e..82bd026a9666 100644 --- a/pkg/server/tenant.go +++ b/pkg/server/tenant.go @@ -833,8 +833,12 @@ func (s *SQLServerWrapper) serveConn( pgServer := s.PGServer() switch status.State { case pgwire.PreServeCancel: - pgServer.HandleCancel(ctx, status.CancelKey) - return nil + // Cancel requests are unauthenticated so run the cancel async to prevent + // the client from deriving any info about the cancel based on how long it + // takes. + return s.stopper.RunAsyncTask(ctx, "cancel", func(ctx context.Context) { + pgServer.HandleCancel(ctx, status.CancelKey) + }) case pgwire.PreServeReady: return pgServer.ServeConn(ctx, conn, status) default: diff --git a/pkg/sql/create_as_test.go b/pkg/sql/create_as_test.go index fa24f829081a..cbb77a507be1 100644 --- a/pkg/sql/create_as_test.go +++ b/pkg/sql/create_as_test.go @@ -99,6 +99,221 @@ func TestCreateAsVTable(t *testing.T) { waitForJobsSuccess(t, sqlRunner) } +func TestCreateAsShow(t *testing.T) { + defer leaktest.AfterTest(t)() + + testCases := []struct { + sql string + setup string + skip bool + }{ + { + sql: "SHOW CLUSTER SETTINGS", + }, + { + sql: "SHOW CLUSTER SETTINGS FOR TENANT [2]", + setup: "SELECT crdb_internal.create_tenant(2)", + }, + { + sql: "SHOW DATABASES", + }, + { + sql: "SHOW ENUMS", + setup: "CREATE TYPE e AS ENUM ('a', 'b')", + }, + { + sql: "SHOW TYPES", + setup: "CREATE TYPE p AS (x int, y int)", + }, + { + sql: "SHOW CREATE DATABASE defaultdb", + }, + { + sql: "SHOW CREATE ALL SCHEMAS", + }, + { + sql: "SHOW CREATE ALL TABLES", + }, + { + sql: "SHOW CREATE TABLE show_create_tbl", + setup: "CREATE TABLE show_create_tbl (id int PRIMARY KEY)", + // TODO(sql-foundations): Fix `relation "show_create_tbl" does not exist` error in job. + // See https://github.com/cockroachdb/cockroach/issues/106260. + skip: true, + }, + { + sql: "SHOW CREATE FUNCTION show_create_fn", + setup: "CREATE FUNCTION show_create_fn(i int) RETURNS INT AS 'SELECT i' LANGUAGE SQL", + // TODO(sql-foundations): Fix `unknown function: show_create_fn(): function undefined` error in job. + // See https://github.com/cockroachdb/cockroach/issues/106268. + skip: true, + }, + { + sql: "SHOW CREATE ALL TYPES", + }, + { + sql: "SHOW INDEXES FROM DATABASE defaultdb", + }, + { + sql: "SHOW INDEXES FROM show_indexes_tbl", + setup: "CREATE TABLE show_indexes_tbl (id int PRIMARY KEY)", + // TODO(sql-foundations): Fix `relation "show_indexes_tbl" does not exist` error in job. + // See https://github.com/cockroachdb/cockroach/issues/106260. + skip: true, + }, + { + sql: "SHOW COLUMNS FROM show_columns_tbl", + setup: "CREATE TABLE show_columns_tbl (id int PRIMARY KEY)", + // TODO(sql-foundations): Fix `relation "show_columns_tbl" does not exist` error in job. + // See https://github.com/cockroachdb/cockroach/issues/106260. + skip: true, + }, + { + sql: "SHOW CONSTRAINTS FROM show_constraints_tbl", + setup: "CREATE TABLE show_constraints_tbl (id int PRIMARY KEY)", + // TODO(sql-foundations): Fix `relation "show_constraints_tbl" does not exist` error in job. + // See https://github.com/cockroachdb/cockroach/issues/106260. + skip: true, + }, + { + sql: "SHOW PARTITIONS FROM DATABASE defaultdb", + }, + { + sql: "SHOW PARTITIONS FROM TABLE show_partitions_tbl", + setup: "CREATE TABLE show_partitions_tbl (id int PRIMARY KEY)", + // TODO(sql-foundations): Fix `relation "show_partitions_tbl" does not exist` error in job. + // See https://github.com/cockroachdb/cockroach/issues/106260. + skip: true, + }, + { + sql: "SHOW PARTITIONS FROM INDEX show_partitions_idx_tbl@show_partitions_idx_tbl_pkey", + setup: "CREATE TABLE show_partitions_idx_tbl (id int PRIMARY KEY)", + // TODO(sql-foundations): Fix `relation "show_partitions_idx_tbl" does not exist` error in job. + // See https://github.com/cockroachdb/cockroach/issues/106260. + skip: true, + }, + { + sql: "SHOW GRANTS", + }, + { + sql: "SHOW JOBS", + }, + { + sql: "SHOW CHANGEFEED JOBS", + }, + { + sql: "SHOW ALL CLUSTER STATEMENTS", + }, + { + sql: "SHOW ALL LOCAL STATEMENTS", + }, + { + sql: "SHOW ALL LOCAL STATEMENTS", + }, + { + sql: "SHOW RANGES WITH DETAILS, KEYS, TABLES", + }, + { + sql: "SHOW RANGE FROM TABLE show_ranges_tbl FOR ROW (0)", + setup: "CREATE TABLE show_ranges_tbl (id int PRIMARY KEY)", + // TODO(sql-foundations): Fix `invalid memory address or nil pointer dereference` error in job. + // See https://github.com/cockroachdb/cockroach/issues/106397. + skip: true, + }, + { + sql: "SHOW SURVIVAL GOAL FROM DATABASE", + }, + { + sql: "SHOW REGIONS FROM DATABASE", + }, + { + sql: "SHOW GRANTS ON ROLE", + }, + { + sql: "SHOW ROLES", + }, + { + sql: "SHOW SCHEMAS", + }, + { + sql: "SHOW SEQUENCES", + setup: "CREATE SEQUENCE seq", + }, + { + sql: "SHOW ALL SESSIONS", + }, + { + sql: "SHOW CLUSTER SESSIONS", + }, + { + sql: "SHOW SYNTAX 'SELECT 1'", + }, + { + sql: "SHOW FUNCTIONS", + setup: "CREATE FUNCTION show_functions_fn(i int) RETURNS INT AS 'SELECT i' LANGUAGE SQL", + }, + { + sql: "SHOW TABLES", + }, + { + sql: "SHOW ALL TRANSACTIONS", + }, + { + sql: "SHOW CLUSTER TRANSACTIONS", + }, + { + sql: "SHOW USERS", + }, + { + sql: "SHOW ALL", + }, + { + sql: "SHOW ZONE CONFIGURATIONS", + }, + { + sql: "SHOW SCHEDULES", + }, + { + sql: "SHOW JOBS FOR SCHEDULES SELECT id FROM [SHOW SCHEDULES]", + }, + { + sql: "SHOW FULL TABLE SCANS", + }, + { + sql: "SHOW DEFAULT PRIVILEGES", + }, + } + + ctx := context.Background() + testCluster := serverutils.StartNewTestCluster(t, 1, base.TestClusterArgs{}) + defer testCluster.Stopper().Stop(ctx) + sqlRunner := sqlutils.MakeSQLRunner(testCluster.ServerConn(0)) + + for i, testCase := range testCases { + t.Run(testCase.sql, func(t *testing.T) { + if testCase.skip { + return + } + if testCase.setup != "" { + sqlRunner.Exec(t, testCase.setup) + } + createTableStmt := fmt.Sprintf( + "CREATE TABLE test_table_%d AS SELECT * FROM [%s]", + i, testCase.sql, + ) + sqlRunner.Exec(t, createTableStmt) + createViewStmt := fmt.Sprintf( + "CREATE MATERIALIZED VIEW test_view_%d AS SELECT * FROM [%s]", + i, testCase.sql, + ) + sqlRunner.Exec(t, createViewStmt) + i++ + }) + } + + waitForJobsSuccess(t, sqlRunner) +} + func waitForJobsSuccess(t *testing.T, sqlRunner *sqlutils.SQLRunner) { query := `SELECT job_id, status, error, description FROM [SHOW JOBS] diff --git a/pkg/sql/gcjob_test/BUILD.bazel b/pkg/sql/gcjob_test/BUILD.bazel index d9745d33f500..8521a3998504 100644 --- a/pkg/sql/gcjob_test/BUILD.bazel +++ b/pkg/sql/gcjob_test/BUILD.bazel @@ -38,7 +38,6 @@ go_test( "//pkg/testutils", "//pkg/testutils/jobutils", "//pkg/testutils/serverutils", - "//pkg/testutils/skip", "//pkg/testutils/sqlutils", "//pkg/util/hlc", "//pkg/util/leaktest", diff --git a/pkg/sql/gcjob_test/gc_job_test.go b/pkg/sql/gcjob_test/gc_job_test.go index 5721e4633855..14bf9fea9f87 100644 --- a/pkg/sql/gcjob_test/gc_job_test.go +++ b/pkg/sql/gcjob_test/gc_job_test.go @@ -45,7 +45,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/jobutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" - "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/leaktest" @@ -58,7 +57,6 @@ import ( // TODO(pbardea): Add more testing around the timer calculations. func TestSchemaChangeGCJob(t *testing.T) { defer leaktest.AfterTest(t)() - skip.WithIssue(t, 60664, "flaky test") type DropItem int const ( @@ -78,6 +76,7 @@ func TestSchemaChangeGCJob(t *testing.T) { for _, ttlTime := range []TTLTime{PAST, SOON, FUTURE} { blockGC := make(chan struct{}, 1) params := base.TestServerArgs{} + params.ScanMaxIdleTime = time.Millisecond params.Knobs.JobsTestingKnobs = jobs.NewTestingKnobsWithShortIntervals() params.Knobs.GCJob = &sql.GCJobTestingKnobs{ RunBeforePerformGC: func(_ jobspb.JobID) error { @@ -90,6 +89,11 @@ func TestSchemaChangeGCJob(t *testing.T) { defer s.Stopper().Stop(ctx) sqlDB := sqlutils.MakeSQLRunner(db) + sqlDB.Exec(t, `SET CLUSTER SETTING sql.gc_job.wait_for_gc.interval = '1s';`) + // Refresh protected timestamp cache immediately to make MVCC GC queue to + // process GC immediately. + sqlDB.Exec(t, `SET CLUSTER SETTING kv.protectedts.poll_interval = '1s';`) + jobRegistry := s.JobRegistry().(*jobs.Registry) sqlDB.Exec(t, "CREATE DATABASE my_db") @@ -100,9 +104,10 @@ func TestSchemaChangeGCJob(t *testing.T) { sqlDB.Exec(t, "ALTER TABLE my_table CONFIGURE ZONE USING gc.ttlseconds = 1") sqlDB.Exec(t, "ALTER TABLE my_other_table CONFIGURE ZONE USING gc.ttlseconds = 1") } - myDBID := descpb.ID(bootstrap.TestingUserDescID(2)) - myTableID := descpb.ID(bootstrap.TestingUserDescID(3)) - myOtherTableID := descpb.ID(bootstrap.TestingUserDescID(4)) + + myDBID := descpb.ID(bootstrap.TestingUserDescID(4)) + myTableID := descpb.ID(bootstrap.TestingUserDescID(6)) + myOtherTableID := descpb.ID(bootstrap.TestingUserDescID(7)) var myTableDesc *tabledesc.Mutable var myOtherTableDesc *tabledesc.Mutable @@ -141,7 +146,7 @@ func TestSchemaChangeGCJob(t *testing.T) { ParentID: myTableID, } myTableDesc.SetPublicNonPrimaryIndexes([]descpb.IndexDescriptor{}) - expectedRunningStatus = "performing garbage collection on index 2" + expectedRunningStatus = "deleting data" case TABLE: details = jobspb.SchemaChangeGCDetails{ Tables: []jobspb.SchemaChangeGCDetails_DroppedID{ @@ -153,7 +158,7 @@ func TestSchemaChangeGCJob(t *testing.T) { } myTableDesc.State = descpb.DescriptorState_DROP myTableDesc.DropTime = dropTime - expectedRunningStatus = fmt.Sprintf("performing garbage collection on table %d", myTableID) + expectedRunningStatus = "deleting data" case DATABASE: details = jobspb.SchemaChangeGCDetails{ Tables: []jobspb.SchemaChangeGCDetails_DroppedID{ @@ -172,7 +177,7 @@ func TestSchemaChangeGCJob(t *testing.T) { myTableDesc.DropTime = dropTime myOtherTableDesc.State = descpb.DescriptorState_DROP myOtherTableDesc.DropTime = dropTime - expectedRunningStatus = fmt.Sprintf("performing garbage collection on tables %d, %d", myTableID, myOtherTableID) + expectedRunningStatus = "deleting data" } if err := kvDB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { @@ -238,24 +243,21 @@ func TestSchemaChangeGCJob(t *testing.T) { if err := sql.TestingDescsTxn(ctx, s, func(ctx context.Context, txn isql.Txn, col *descs.Collection) error { myImm, err := col.ByID(txn.KV()).Get().Table(ctx, myTableID) if err != nil { + if ttlTime != FUTURE && (dropItem == TABLE || dropItem == DATABASE) { + // We dropped the table, so expect it to not be found. + require.EqualError(t, err, fmt.Sprintf(`relation "[%d]" does not exist`, myTableID)) + return nil + } return err } - if ttlTime != FUTURE && (dropItem == TABLE || dropItem == DATABASE) { - // We dropped the table, so expect it to not be found. - require.EqualError(t, err, "descriptor not found") - return nil - } myTableDesc = tabledesc.NewBuilder(myImm.TableDesc()).BuildExistingMutableTable() myOtherImm, err := col.ByID(txn.KV()).Get().Table(ctx, myOtherTableID) if err != nil { - return err - } - if ttlTime != FUTURE && dropItem == DATABASE { - // We dropped the entire database, so expect none of the tables to be found. - require.EqualError(t, err, "descriptor not found") - return nil - } - if err != nil { + if ttlTime != FUTURE && dropItem == DATABASE { + // We dropped the entire database, so expect none of the tables to be found. + require.EqualError(t, err, fmt.Sprintf(`relation "[%d]" does not exist`, myOtherTableID)) + return nil + } return err } myOtherTableDesc = tabledesc.NewBuilder(myOtherImm.TableDesc()).BuildExistingMutableTable() diff --git a/pkg/sql/pgwire/pre_serve.go b/pkg/sql/pgwire/pre_serve.go index 967bec8ebe89..37b9ae56281a 100644 --- a/pkg/sql/pgwire/pre_serve.go +++ b/pkg/sql/pgwire/pre_serve.go @@ -217,7 +217,6 @@ func (s *PreServeConnHandler) SendRoutingError( `Double check your "-ccluster=" connection option or your "cluster:" database name prefix.`) _ = s.sendErr(ctx, s.st, conn, err) - _ = conn.Close() } // sendErr sends errors to the client during the connection startup @@ -235,7 +234,6 @@ func (s *PreServeConnHandler) sendErr( // receive error payload are highly correlated with clients // disconnecting abruptly. _ /* err */ = w.writeErr(ctx, err, conn) - _ = conn.Close() return err } @@ -324,7 +322,7 @@ func (s *PreServeConnHandler) PreServe( case versionCancel: // The cancel message is rather peculiar: it is sent without // authentication, always over an unencrypted channel. - if ok, key := readCancelKeyAndCloseConn(ctx, conn, &buf); ok { + if ok, key := readCancelKey(ctx, &buf); ok { return conn, PreServeStatus{ State: PreServeCancel, CancelKey: key, @@ -374,7 +372,7 @@ func (s *PreServeConnHandler) PreServe( // Yet, we've found clients in the wild that send the cancel // after the TLS handshake, for example at // https://github.com/cockroachlabs/support/issues/600. - if ok, key := readCancelKeyAndCloseConn(ctx, conn, &buf); ok { + if ok, key := readCancelKey(ctx, &buf); ok { return conn, PreServeStatus{ State: PreServeCancel, CancelKey: key, diff --git a/pkg/sql/pgwire/server.go b/pkg/sql/pgwire/server.go index 4a9c5dc2dfd0..a7b9942ac017 100644 --- a/pkg/sql/pgwire/server.go +++ b/pkg/sql/pgwire/server.go @@ -928,8 +928,6 @@ func (s *Server) serveImpl( authOpt authOptions, sessionID clusterunique.ID, ) { - defer func() { _ = c.conn.Close() }() - if c.sessionArgs.User.IsRootUser() || c.sessionArgs.User.IsNodeUser() { ctx = logtags.AddTag(ctx, "user", redact.Safe(c.sessionArgs.User)) } else { @@ -1253,18 +1251,13 @@ func (s *Server) serveImpl( } } -// readCancelKeyAndCloseConn retrieves the "backend data" key that identifies +// readCancelKey retrieves the "backend data" key that identifies // a cancellable query, then closes the connection. -func readCancelKeyAndCloseConn( - ctx context.Context, conn net.Conn, buf *pgwirebase.ReadBuffer, +func readCancelKey( + ctx context.Context, buf *pgwirebase.ReadBuffer, ) (ok bool, cancelKey pgwirecancel.BackendKeyData) { telemetry.Inc(sqltelemetry.CancelRequestCounter) backendKeyDataBits, err := buf.GetUint64() - // The connection that issued the cancel is not a SQL session -- it's an - // entirely new connection that's created just to send the cancel. We close - // the connection as soon as possible after reading the data, since there - // is nothing to send back to the client. - _ = conn.Close() // The client is also unwilling to read an error payload, so we just log it locally. if err != nil { log.Sessions.Warningf(ctx, "%v", errors.Wrap(err, "reading cancel key from client")) @@ -1378,7 +1371,6 @@ func (s *Server) sendErr( // receive error payload are highly correlated with clients // disconnecting abruptly. _ /* err */ = w.writeErr(ctx, err, conn) - _ = conn.Close() return err } diff --git a/pkg/sql/sem/tree/show.go b/pkg/sql/sem/tree/show.go index fc929a641600..22c4f2df44e7 100644 --- a/pkg/sql/sem/tree/show.go +++ b/pkg/sql/sem/tree/show.go @@ -560,7 +560,7 @@ func (node *ShowChangefeedJobs) Format(ctx *FmtCtx) { } } -// ShowSurvivalGoal represents a SHOW REGIONS statement +// ShowSurvivalGoal represents a SHOW SURVIVAL GOAL statement type ShowSurvivalGoal struct { DatabaseName Name } diff --git a/pkg/util/netutil/net.go b/pkg/util/netutil/net.go index 76cad5b855c3..6c11c7089254 100644 --- a/pkg/util/netutil/net.go +++ b/pkg/util/netutil/net.go @@ -183,6 +183,9 @@ func (s *TCPServer) ServeWith( } tempDelay = 0 err := s.stopper.RunAsyncTask(ctx, "tcp-serve", func(ctx context.Context) { + defer func() { + _ = rw.Close() + }() s.addConn(rw) defer s.rmConn(rw) serveConn(ctx, rw) diff --git a/pkg/util/tracing/collector/BUILD.bazel b/pkg/util/tracing/collector/BUILD.bazel index 396109011d41..7c0903a53316 100644 --- a/pkg/util/tracing/collector/BUILD.bazel +++ b/pkg/util/tracing/collector/BUILD.bazel @@ -34,11 +34,11 @@ go_test( "//pkg/security/securitytest", "//pkg/security/username", "//pkg/server", - "//pkg/testutils", "//pkg/testutils/serverutils", "//pkg/testutils/sqlutils", "//pkg/testutils/testcluster", "//pkg/util/leaktest", + "//pkg/util/log", "//pkg/util/randutil", "//pkg/util/tracing", "//pkg/util/tracing/tracingpb", diff --git a/pkg/util/tracing/collector/collector_test.go b/pkg/util/tracing/collector/collector_test.go index 9e60522c887a..8fbf28f08a09 100644 --- a/pkg/util/tracing/collector/collector_test.go +++ b/pkg/util/tracing/collector/collector_test.go @@ -23,11 +23,11 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/rpc/nodedialer" "github.com/cockroachdb/cockroach/pkg/security/username" - "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/tracing" "github.com/cockroachdb/cockroach/pkg/util/tracing/collector" "github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb" @@ -210,6 +210,7 @@ func TestTracingCollectorGetSpanRecordings(t *testing.T) { // mixed nodes. func TestClusterInflightTraces(t *testing.T) { defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) ccl.TestingEnableEnterprise() // We'll create tenants. defer ccl.TestingDisableEnterprise() @@ -217,94 +218,146 @@ func TestClusterInflightTraces(t *testing.T) { defer cancel() args := base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - // We'll create our own tenants, to ensure they exist as opposed to them - // being created randomly. + // The test itself creates tenants however necessary. DefaultTestTenant: base.TestTenantDisabled, }, } - testutils.RunTrueAndFalse(t, "tenant", func(t *testing.T, tenant bool) { - tc := testcluster.StartTestCluster(t, 2 /* nodes */, args) - defer tc.Stopper().Stop(ctx) - - type testCase struct { - servers []serverutils.TestTenantInterface - // otherServers, if set, represents the servers corresponding to other - // tenants (or to the system tenant) than the ones being tested. - otherServers []serverutils.TestTenantInterface + getDB := func(sqlAddr, prefix string) (_ *gosql.DB, cleanup func()) { + pgURL, cleanupPGUrl := sqlutils.PGUrl(t, sqlAddr, prefix, url.User(username.RootUser)) + db, err := gosql.Open("postgres", pgURL.String()) + require.NoError(t, err) + return db, func() { + require.NoError(t, db.Close()) + cleanupPGUrl() } - var testCases []testCase - if tenant { - tenantID := roachpb.MustMakeTenantID(10) - tenants := make([]serverutils.TestTenantInterface, len(tc.Servers)) - for i := range tc.Servers { - tenant, err := tc.Servers[i].StartTenant(ctx, base.TestTenantArgs{TenantID: tenantID}) - require.NoError(t, err) - tenants[i] = tenant + } + + for _, config := range []string{ + "single-tenant", + "shared-process", + "separate-process", + } { + t.Run(config, func(t *testing.T) { + tc := testcluster.StartTestCluster(t, 2 /* nodes */, args) + defer tc.Stopper().Stop(ctx) + + systemServers := []serverutils.TestTenantInterface{tc.Servers[0], tc.Servers[1]} + systemDBs := make([]*gosql.DB, len(tc.Servers)) + for i, s := range tc.Servers { + db, cleanup := getDB(s.SQLAddr(), "System" /* prefix */) + defer cleanup() + systemDBs[i] = db } - testCases = []testCase{ - { - servers: tenants, - otherServers: []serverutils.TestTenantInterface{tc.Servers[0], tc.Servers[1]}, - }, - { - servers: []serverutils.TestTenantInterface{tc.Servers[0], tc.Servers[1]}, - otherServers: tenants, - }, + + type testCase struct { + servers []serverutils.TestTenantInterface + dbs []*gosql.DB + // otherServers, if set, represents the servers corresponding to + // other tenants (or to the system tenant) than the ones being + // tested. + otherServers []serverutils.TestTenantInterface } - } else { - testCases = []testCase{{ - servers: []serverutils.TestTenantInterface{tc.Servers[0], tc.Servers[1]}, - }} - } + var testCases []testCase + switch config { + case "single-tenant": + testCases = []testCase{{ + servers: []serverutils.TestTenantInterface{tc.Servers[0], tc.Servers[1]}, + dbs: systemDBs, + }} + + case "shared-process": + tenants := make([]serverutils.TestTenantInterface, len(tc.Servers)) + dbs := make([]*gosql.DB, len(tc.Servers)) + for i, s := range tc.Servers { + tenant, db, err := s.StartSharedProcessTenant(ctx, base.TestSharedProcessTenantArgs{TenantName: "app"}) + require.NoError(t, err) + defer func() { + require.NoError(t, db.Close()) + }() + tenants[i] = tenant + dbs[i] = db + } + testCases = []testCase{ + { + servers: tenants, + dbs: dbs, + otherServers: systemServers, + }, + { + servers: systemServers, + dbs: systemDBs, + otherServers: tenants, + }, + } - for _, tc := range testCases { - // Setup the traces we're going to look for. - localTraceID, _, cleanup := setupTraces(tc.servers[0].Tracer(), tc.servers[1].Tracer()) - defer cleanup() - - // Create some other spans on tc.otherServers, that we don't expect to - // find. - const otherServerSpanName = "other-server-span" - for _, s := range tc.otherServers { - sp := s.Tracer().StartSpan(otherServerSpanName) - defer sp.Finish() + case "separate-process": + tenantID := roachpb.MustMakeTenantID(10) + tenants := make([]serverutils.TestTenantInterface, len(tc.Servers)) + dbs := make([]*gosql.DB, len(tc.Servers)) + for i := range tc.Servers { + tenant, err := tc.Servers[i].StartTenant(ctx, base.TestTenantArgs{TenantID: tenantID}) + require.NoError(t, err) + tenants[i] = tenant + db, cleanup := getDB(tenant.SQLAddr(), "Tenant" /* prefix */) + defer cleanup() + dbs[i] = db + } + testCases = []testCase{ + { + servers: tenants, + dbs: dbs, + otherServers: systemServers, + }, + { + servers: systemServers, + dbs: systemDBs, + otherServers: tenants, + }, + } } - // We're going to query the cluster_inflight_traces through every node. - for _, s := range tc.servers { - pgURL, cleanupPGUrl := sqlutils.PGUrl(t, s.SQLAddr(), "Tenant", url.User(username.RootUser)) - defer cleanupPGUrl() - db, err := gosql.Open("postgres", pgURL.String()) - defer func() { - require.NoError(t, db.Close()) - }() - require.NoError(t, err) - - rows, err := db.Query( - "SELECT node_id, trace_str FROM crdb_internal.cluster_inflight_traces "+ - "WHERE trace_id=$1 ORDER BY node_id", - localTraceID) - require.NoError(t, err) - - expSpans := map[int][]string{ - 1: {"root", "root.child", "root.child.remotechilddone"}, - 2: {"root.child.remotechild"}, + for _, tc := range testCases { + // Set up the traces we're going to look for. + localTraceID, _, cleanup := setupTraces(tc.servers[0].Tracer(), tc.servers[1].Tracer()) + defer cleanup() + + // Create some other spans on tc.otherServers, that we don't + // expect to find. + const otherServerSpanName = "other-server-span" + for _, s := range tc.otherServers { + sp := s.Tracer().StartSpan(otherServerSpanName) + defer sp.Finish() } - for rows.Next() { - var nodeID int - var trace string - require.NoError(t, rows.Scan(&nodeID, &trace)) - exp, ok := expSpans[nodeID] - require.True(t, ok) - delete(expSpans, nodeID) // Consume this entry; we'll check that they were all consumed. - for _, span := range exp { - require.Contains(t, trace, "=== operation:"+span) + + // We're going to query the cluster_inflight_traces through + // every SQL instance. + for _, db := range tc.dbs { + rows, err := db.Query( + "SELECT node_id, trace_str FROM crdb_internal.cluster_inflight_traces "+ + "WHERE trace_id=$1 ORDER BY node_id", + localTraceID) + require.NoError(t, err) + + expSpans := map[int][]string{ + 1: {"root", "root.child", "root.child.remotechilddone"}, + 2: {"root.child.remotechild"}, } - require.NotContains(t, trace, "=== operation:"+otherServerSpanName) + for rows.Next() { + var nodeID int + var trace string + require.NoError(t, rows.Scan(&nodeID, &trace)) + exp, ok := expSpans[nodeID] + require.True(t, ok) + delete(expSpans, nodeID) // Consume this entry; we'll check that they were all consumed. + for _, span := range exp { + require.Contains(t, trace, "=== operation:"+span) + } + require.NotContains(t, trace, "=== operation:"+otherServerSpanName) + } + require.Len(t, expSpans, 0) } - require.Len(t, expSpans, 0) } - } - }) + }) + } }