From 5ca2b6add4d672b9366543a5932b044a34945d3d Mon Sep 17 00:00:00 2001 From: Steven Danna Date: Wed, 22 Feb 2023 15:18:09 +0000 Subject: [PATCH 1/2] server: initialize DBContext with correct instance ID Before this change the DBContext in a tenant would always have a zero'd SQLInstanceID. Note that for SQL pods not co-located on an KV-node, OptionalNodeID() should still return 0 and anything that requires a NodeID rather than an InstanceID _should_ be using that method. Informs: #96353 Epic: none Release note: None --- pkg/server/tenant.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/server/tenant.go b/pkg/server/tenant.go index aa9171dc88fa..ae1fd14cb4dc 100644 --- a/pkg/server/tenant.go +++ b/pkg/server/tenant.go @@ -1017,7 +1017,11 @@ func makeTenantSQLServerArgs( }, ds, ) - db := kv.NewDB(baseCfg.AmbientCtx, tcsFactory, clock, stopper) + + dbCtx := kv.DefaultDBContext(stopper) + dbCtx.NodeID = instanceIDContainer + db := kv.NewDBWithContext(baseCfg.AmbientCtx, tcsFactory, clock, dbCtx) + rangeFeedKnobs, _ := baseCfg.TestingKnobs.RangeFeed.(*rangefeed.TestingKnobs) rangeFeedFactory, err := rangefeed.NewFactory(stopper, db, st, rangeFeedKnobs) if err != nil { From e49d44ff6e375e53b4a6453d585315d4d62fc206 Mon Sep 17 00:00:00 2001 From: j82w Date: Wed, 22 Feb 2023 08:16:31 -0500 Subject: [PATCH 2/2] sql: add multi-tenant specific test for transaction_contention_events part of #96353 Epic: None Release note: None --- pkg/sql/crdb_internal_test.go | 49 ++++++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/pkg/sql/crdb_internal_test.go b/pkg/sql/crdb_internal_test.go index bb69181517b7..ce0fd2b4c888 100644 --- a/pkg/sql/crdb_internal_test.go +++ b/pkg/sql/crdb_internal_test.go @@ -912,7 +912,38 @@ func TestTxnContentionEventsTable(t *testing.T) { defer tc.Stopper().Stop(ctx) conn := tc.ServerConn(0) sqlDB := sqlutils.MakeSQLRunner(tc.ServerConn(0)) + testTxnContentionEventsTableHelper(t, ctx, conn, sqlDB) +} + +func TestTxnContentionEventsTableMultiTenant(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + ctx := context.Background() + tc := testcluster.StartTestCluster(t, 1, + base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + // Test is designed to run with explicit tenants. No need to + // implicitly create a tenant. + DisableDefaultTestTenant: true, + }, + }) + defer tc.Stopper().Stop(ctx) + _, tSQL := serverutils.StartTenant(t, tc.Server(0), base.TestTenantArgs{ + TenantID: roachpb.MustMakeTenantID(10), + }) + + conn, err := tSQL.Conn(ctx) + require.NoError(t, err) + sqlDB := sqlutils.MakeSQLRunner(conn) + defer tSQL.Close() + testTxnContentionEventsTableHelper(t, ctx, tSQL, sqlDB) +} + +func testTxnContentionEventsTableHelper( + t *testing.T, ctx context.Context, conn *gosql.DB, sqlDB *sqlutils.SQLRunner, +) { sqlDB.Exec( t, `SET CLUSTER SETTING sql.metrics.statement_details.plan_collection.enabled = false;`) @@ -977,8 +1008,8 @@ func TestTxnContentionEventsTable(t *testing.T) { // This ensures the event is the one caused in the test and not by some other // internal workflow. testutils.SucceedsWithin(t, func() error { - rows, errVerify := conn.QueryContext(ctx, `SELECT - blocking_txn_id, + rows, errVerify := conn.QueryContext(ctx, `SELECT + blocking_txn_id, waiting_txn_id, waiting_stmt_id, encode( @@ -989,14 +1020,14 @@ func TestTxnContentionEventsTable(t *testing.T) { schema_name, table_name, index_name - FROM crdb_internal.transaction_contention_events tce - inner join ( + FROM crdb_internal.transaction_contention_events tce + inner join ( select fingerprint_id, - transaction_fingerprint_id, - metadata->'query' as query - from crdb_internal.statement_statistics t - where metadata->>'query' like 'UPDATE t SET %') stats + transaction_fingerprint_id, + metadata->'query' as query + from crdb_internal.statement_statistics t + where metadata->>'query' like 'UPDATE t SET %') stats on stats.transaction_fingerprint_id = tce.waiting_txn_fingerprint_id and stats.fingerprint_id = tce.waiting_stmt_fingerprint_id`) if errVerify != nil { @@ -1188,7 +1219,7 @@ func TestInternalSystemJobsTableMirrorsSystemJobsTable(t *testing.T) { res := tdb.QueryStr(t, "SELECT * FROM system.jobs ORDER BY id") tdb.CheckQueryResults(t, `SELECT * FROM crdb_internal.system_jobs ORDER BY id`, res) tdb.CheckQueryResults(t, ` - SELECT id, status, created, payload, progress, created_by_type, created_by_id, claim_session_id, + SELECT id, status, created, payload, progress, created_by_type, created_by_id, claim_session_id, claim_instance_id, num_runs, last_run, job_type FROM crdb_internal.system_jobs ORDER BY id`, res,