Skip to content

Commit cb94136

Browse files
craig[bot]shubhamdhama
craig[bot]
and
shubhamdhama
committed
Merge #142391
142391: sql: increase tenant testing coverage r=rafiss,msbutler a=shubhamdhama `` sql: increase tenant testing coverage for `jobs_profiler_execution_details_test` `` `` sql: increase tenant testing coverage for `mvcc_backfiller_test` `` `` sql: increase tenant testing coverage for `txn_restart_test` `` `` sql: enable shared-process mode tenant testing for `TestUnsplitRanges` `` I opened issue #142388 to track the work for enabling external-process mode testing, as it turned out to be a non-trivial effort. Informs: #140446 Epic: CRDB-48564 Release note: None Co-authored-by: Shubham Dhama <[email protected]>
2 parents c78ecb9 + 7c40dd2 commit cb94136

File tree

5 files changed

+108
-49
lines changed

5 files changed

+108
-49
lines changed

‎pkg/base/test_server_args.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,19 @@ func TestIsForStuffThatShouldWorkWithSharedProcessModeButDoesntYet(
526526
//
527527
// It should link to a github issue with label C-investigation.
528528
func TestSkippedForExternalModeDueToPerformance(issueNumber int) DefaultTestTenantOptions {
529+
return testSkippedForExternalProcessMode(issueNumber)
530+
}
531+
532+
// TestDoesNotWorkWithExternalProcessMode disables selecting the external
533+
// process virtual cluster for tests that are not functional in that mode and
534+
// require further investigation. Any test using this function should reference
535+
// a GitHub issue tagged with "C-investigation" describing the underlying
536+
// problem.
537+
func TestDoesNotWorkWithExternalProcessMode(issueNumber int) DefaultTestTenantOptions {
538+
return testSkippedForExternalProcessMode(issueNumber)
539+
}
540+
541+
func testSkippedForExternalProcessMode(issueNumber int) DefaultTestTenantOptions {
529542
return DefaultTestTenantOptions{
530543
testBehavior: ttSharedProcess,
531544
allowAdditionalTenants: true,

‎pkg/sql/jobs_profiler_execution_details_test.go‎

Lines changed: 51 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ func TestShowJobsWithExecutionDetails(t *testing.T) {
115115
ctx, cancel := context.WithTimeout(ctx, time.Minute*2)
116116
defer cancel()
117117

118-
params, _ := createTestServerParams()
118+
params, _ := createTestServerParamsAllowTenants()
119119
params.Knobs.JobsTestingKnobs = jobs.NewTestingKnobsWithShortIntervals()
120120
defer jobs.ResetConstructors()()
121121
s, sqlDB, _ := serverutils.StartServer(t, params)
@@ -159,7 +159,7 @@ func TestReadWriteProfilerExecutionDetails(t *testing.T) {
159159
ctx, cancel := context.WithTimeout(ctx, time.Minute*2)
160160
defer cancel()
161161

162-
params, _ := createTestServerParams()
162+
params, _ := createTestServerParamsAllowTenants()
163163
params.Knobs.JobsTestingKnobs = jobs.NewTestingKnobsWithShortIntervals()
164164
defer jobs.ResetConstructors()()
165165
s, sqlDB, _ := serverutils.StartServer(t, params)
@@ -224,8 +224,18 @@ func TestReadWriteProfilerExecutionDetails(t *testing.T) {
224224
require.NoError(t, err)
225225
continueCh <- struct{}{}
226226
jobutils.WaitForJobToSucceed(t, runner, jobspb.JobID(importJobID))
227-
require.True(t, strings.Contains(string(goroutines), fmt.Sprintf("labels: {\"foo\":\"bar\", \"job\":\"IMPORT id=%d\", \"n\":\"1\"}", importJobID)))
228-
require.True(t, strings.Contains(string(goroutines), "github.com/cockroachdb/cockroach/pkg/sql_test.fakeExecResumer.Resume"))
227+
expectedSubstrings := []string{
228+
fmt.Sprintf("labels: {\"foo\":\"bar\", \"job\":\"IMPORT id=%d\", \"n\":\"1\"}", importJobID),
229+
"github.com/cockroachdb/cockroach/pkg/sql_test.fakeExecResumer.Resume",
230+
}
231+
goroutinesStr := string(goroutines)
232+
for _, substr := range expectedSubstrings {
233+
if s.DeploymentMode().IsExternal() {
234+
require.NotContains(t, goroutinesStr, substr)
235+
} else {
236+
require.Contains(t, goroutinesStr, substr)
237+
}
238+
}
229239
})
230240

231241
t.Run("execution details for invalid job ID", func(t *testing.T) {
@@ -323,7 +333,7 @@ func TestListProfilerExecutionDetails(t *testing.T) {
323333
ctx, cancel := context.WithTimeout(ctx, time.Minute*2)
324334
defer cancel()
325335

326-
params, _ := createTestServerParams()
336+
params, _ := createTestServerParamsAllowTenants()
327337
params.Knobs.JobsTestingKnobs = jobs.NewTestingKnobsWithShortIntervals()
328338
defer jobs.ResetConstructors()()
329339
s, sqlDB, _ := serverutils.StartServer(t, params)
@@ -366,19 +376,30 @@ func TestListProfilerExecutionDetails(t *testing.T) {
366376

367377
runner.Exec(t, `SELECT crdb_internal.request_job_execution_details($1)`, importJobID)
368378
files := listExecutionDetails(t, s, jobspb.JobID(importJobID))
369-
require.Len(t, files, 3)
370-
require.Regexp(t, "distsql\\..*\\.html", files[0])
371-
require.Regexp(t, "goroutines\\..*\\.txt", files[1])
372-
require.Regexp(t, "trace\\..*\\.zip", files[2])
379+
380+
patterns := []string{
381+
"distsql\\..*\\.html",
382+
}
383+
if !s.DeploymentMode().IsExternal() {
384+
patterns = append(patterns, "goroutines\\..*\\.txt")
385+
}
386+
patterns = append(patterns, "trace\\..*\\.zip")
387+
388+
require.Len(t, files, len(patterns))
389+
for i, pattern := range patterns {
390+
require.Regexp(t, pattern, files[i])
391+
}
373392

374393
continueCh <- struct{}{}
375394
jobutils.WaitForJobToPause(t, runner, jobspb.JobID(importJobID))
376395

377396
testutils.SucceedsSoon(t, func() error {
378397
files = listExecutionDetails(t, s, jobspb.JobID(importJobID))
379-
if len(files) != 5 {
380-
return errors.Newf("expected 5 files, got %d: %v", len(files), files)
398+
expectedCount := 5
399+
if s.DeploymentMode().IsExternal() {
400+
expectedCount--
381401
}
402+
require.Len(t, files, expectedCount)
382403
return nil
383404
})
384405

@@ -393,21 +414,28 @@ func TestListProfilerExecutionDetails(t *testing.T) {
393414
jobutils.WaitForJobToSucceed(t, runner, jobspb.JobID(importJobID))
394415
testutils.SucceedsSoon(t, func() error {
395416
files = listExecutionDetails(t, s, jobspb.JobID(importJobID))
396-
if len(files) != 10 {
397-
return errors.Newf("expected 10 files, got %d: %v", len(files), files)
417+
expectedCount := 10
418+
if s.DeploymentMode().IsExternal() {
419+
expectedCount = 8
398420
}
421+
require.Len(t, files, expectedCount)
399422
return nil
400423
})
401-
require.Regexp(t, "[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb", files[0])
402-
require.Regexp(t, "[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb.txt", files[1])
403-
require.Regexp(t, "[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb", files[2])
404-
require.Regexp(t, "[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb.txt", files[3])
405-
require.Regexp(t, "distsql\\..*\\.html", files[4])
406-
require.Regexp(t, "distsql\\..*\\.html", files[5])
407-
require.Regexp(t, "goroutines\\..*\\.txt", files[6])
408-
require.Regexp(t, "goroutines\\..*\\.txt", files[7])
409-
require.Regexp(t, "trace\\..*\\.zip", files[8])
410-
require.Regexp(t, "trace\\..*\\.zip", files[9])
424+
patterns = []string{
425+
"[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb",
426+
"[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb.txt",
427+
"[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb",
428+
"[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb.txt",
429+
"distsql\\..*\\.html",
430+
"distsql\\..*\\.html",
431+
}
432+
if !s.DeploymentMode().IsExternal() {
433+
patterns = append(patterns, "goroutines\\..*\\.txt", "goroutines\\..*\\.txt")
434+
}
435+
patterns = append(patterns, "trace\\..*\\.zip", "trace\\..*\\.zip")
436+
for i, pattern := range patterns {
437+
require.Regexp(t, pattern, files[i])
438+
}
411439
})
412440
}
413441

‎pkg/sql/mvcc_backfiller_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ func TestIndexBackfillMergeTxnRetry(t *testing.T) {
556556
additionalRowsForMerge = 10
557557
)
558558

559-
params, _ := createTestServerParams()
559+
params, _ := createTestServerParamsAllowTenants()
560560
params.Knobs = base.TestingKnobs{
561561
SQLSchemaChanger: &sql.SchemaChangerTestingKnobs{
562562
// Ensure that the temp index has work to do.
@@ -598,6 +598,7 @@ func TestIndexBackfillMergeTxnRetry(t *testing.T) {
598598
var err error
599599
scratch, err = s.ScratchRange()
600600
require.NoError(t, err)
601+
scratch = append(s.Codec().TenantPrefix(), scratch...)
601602

602603
if _, err := sqlDB.Exec(`
603604
SET use_declarative_schema_changer='off';

‎pkg/sql/txn_restart_test.go‎

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,13 @@ import (
2525
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
2626
"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
2727
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
28+
"github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilitiespb"
2829
"github.com/cockroachdb/cockroach/pkg/roachpb"
2930
"github.com/cockroachdb/cockroach/pkg/security/username"
3031
"github.com/cockroachdb/cockroach/pkg/sql"
3132
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
3233
"github.com/cockroachdb/cockroach/pkg/sql/tests"
3334
"github.com/cockroachdb/cockroach/pkg/testutils"
34-
"github.com/cockroachdb/cockroach/pkg/testutils/pgurlutils"
3535
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
3636
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
3737
"github.com/cockroachdb/cockroach/pkg/util/caller"
@@ -213,7 +213,8 @@ func checkRestarts(t *testing.T, magicVals *filterVals) {
213213
// s, sqlDB, _ := serverutils.StartServer(t, params)
214214
// defer s.Stopper().Stop(context.Background())
215215
// {
216-
// pgURL, cleanup := sqlutils.PGUrl(t, s.AdvSQLAddr(), "TestTxnAutoRetry", url.User(security.RootUser)
216+
// pgURL, cleanup := s.ApplicationLayer().PGUrl(t,
217+
// serverutils.CertsDirPrefix("TestTxnAutoRetry"), serverutils.User(username.RootUser))
217218
// defer cleanup()
218219
// if err := aborter.Init(pgURL); err != nil {
219220
// t.Fatal(err)
@@ -449,12 +450,13 @@ func TestTxnAutoRetry(t *testing.T) {
449450

450451
aborter := NewTxnAborter()
451452
defer aborter.Close(t)
452-
params, cmdFilters := createTestServerParams()
453+
params, cmdFilters := createTestServerParamsAllowTenants()
453454
params.Knobs.SQLExecutor = aborter.executorKnobs()
454455
s, sqlDB, _ := serverutils.StartServer(t, params)
455456
defer s.Stopper().Stop(context.Background())
456457
{
457-
pgURL, cleanup := pgurlutils.PGUrl(t, s.AdvSQLAddr(), "TestTxnAutoRetry", url.User(username.RootUser))
458+
pgURL, cleanup := s.ApplicationLayer().PGUrl(t,
459+
serverutils.CertsDirPrefix("TestTxnAutoRetry"), serverutils.User(username.RootUser))
458460
defer cleanup()
459461
if err := aborter.Init(pgURL); err != nil {
460462
t.Fatal(err)
@@ -628,12 +630,13 @@ func TestAbortedTxnOnlyRetriedOnce(t *testing.T) {
628630

629631
aborter := NewTxnAborter()
630632
defer aborter.Close(t)
631-
params, _ := createTestServerParams()
633+
params, _ := createTestServerParamsAllowTenants()
632634
params.Knobs.SQLExecutor = aborter.executorKnobs()
633635
s, sqlDB, _ := serverutils.StartServer(t, params)
634636
defer s.Stopper().Stop(context.Background())
635637
{
636-
pgURL, cleanup := pgurlutils.PGUrl(t, s.AdvSQLAddr(), "TestAbortedTxnOnlyRetriedOnce", url.User(username.RootUser))
638+
pgURL, cleanup := s.ApplicationLayer().PGUrl(t,
639+
serverutils.CertsDirPrefix("TestAbortedTxnOnlyRetriedOnce"), serverutils.User(username.RootUser))
637640
defer cleanup()
638641
if err := aborter.Init(pgURL); err != nil {
639642
t.Fatal(err)
@@ -784,12 +787,14 @@ func TestTxnUserRestart(t *testing.T) {
784787
t.Run(fmt.Sprintf("err=%s,stgy=%d", tc.expectedErr, rs), func(t *testing.T) {
785788
aborter := NewTxnAborter()
786789
defer aborter.Close(t)
787-
params, cmdFilters := createTestServerParams()
790+
params, cmdFilters := createTestServerParamsAllowTenants()
788791
params.Knobs.SQLExecutor = aborter.executorKnobs()
789792
s, sqlDB, _ := serverutils.StartServer(t, params)
790793
defer s.Stopper().Stop(context.Background())
791794
{
792-
pgURL, cleanup := pgurlutils.PGUrl(t, s.AdvSQLAddr(), "TestTxnUserRestart", url.User(username.RootUser))
795+
796+
pgURL, cleanup := s.ApplicationLayer().PGUrl(t,
797+
serverutils.CertsDirPrefix("TestTxnUserRestart"), serverutils.User(username.RootUser))
793798
defer cleanup()
794799
if err := aborter.Init(pgURL); err != nil {
795800
t.Fatal(err)
@@ -864,7 +869,7 @@ func TestCommitWaitState(t *testing.T) {
864869
defer leaktest.AfterTest(t)()
865870
defer log.Scope(t).Close(t)
866871

867-
params, _ := createTestServerParams()
872+
params, _ := createTestServerParamsAllowTenants()
868873
s, sqlDB, _ := serverutils.StartServer(t, params)
869874
defer s.Stopper().Stop(context.Background())
870875
if _, err := sqlDB.Exec(`
@@ -900,12 +905,14 @@ func TestErrorOnCommitFinalizesTxn(t *testing.T) {
900905

901906
aborter := NewTxnAborter()
902907
defer aborter.Close(t)
903-
params, _ := createTestServerParams()
908+
params, _ := createTestServerParamsAllowTenants()
904909
params.Knobs.SQLExecutor = aborter.executorKnobs()
905910
s, sqlDB, _ := serverutils.StartServer(t, params)
906911
defer s.Stopper().Stop(context.Background())
907912
{
908-
pgURL, cleanup := pgurlutils.PGUrl(t, s.AdvSQLAddr(), "TestErrorOnCommitFinalizesTxn", url.User(username.RootUser))
913+
914+
pgURL, cleanup := s.ApplicationLayer().PGUrl(t,
915+
serverutils.CertsDirPrefix("TestErrorOnCommitFinalizesTxn"), serverutils.User(username.RootUser))
909916
defer cleanup()
910917
if err := aborter.Init(pgURL); err != nil {
911918
t.Fatal(err)
@@ -987,12 +994,13 @@ func TestRollbackInRestartWait(t *testing.T) {
987994

988995
aborter := NewTxnAborter()
989996
defer aborter.Close(t)
990-
params, _ := createTestServerParams()
997+
params, _ := createTestServerParamsAllowTenants()
991998
params.Knobs.SQLExecutor = aborter.executorKnobs()
992999
s, sqlDB, _ := serverutils.StartServer(t, params)
9931000
defer s.Stopper().Stop(context.Background())
9941001
{
995-
pgURL, cleanup := pgurlutils.PGUrl(t, s.AdvSQLAddr(), "TestRollbackInRestartWait", url.User(username.RootUser))
1002+
pgURL, cleanup := s.ApplicationLayer().PGUrl(t,
1003+
serverutils.CertsDirPrefix("TestRollbackInRestartWait"), serverutils.User(username.RootUser))
9961004
defer cleanup()
9971005
if err := aborter.Init(pgURL); err != nil {
9981006
t.Fatal(err)
@@ -1048,7 +1056,7 @@ func TestUnexpectedStatementInRestartWait(t *testing.T) {
10481056
defer leaktest.AfterTest(t)()
10491057
defer log.Scope(t).Close(t)
10501058

1051-
params, _ := createTestServerParams()
1059+
params, _ := createTestServerParamsAllowTenants()
10521060
s, sqlDB, _ := serverutils.StartServer(t, params)
10531061
defer s.Stopper().Stop(context.Background())
10541062

@@ -1099,7 +1107,7 @@ func TestNonRetryableError(t *testing.T) {
10991107
defer leaktest.AfterTest(t)()
11001108
defer log.Scope(t).Close(t)
11011109

1102-
params, cmdFilters := createTestServerParams()
1110+
params, cmdFilters := createTestServerParamsAllowTenants()
11031111
s, sqlDB, _ := serverutils.StartServer(t, params)
11041112
defer s.Stopper().Stop(context.Background())
11051113

@@ -1191,7 +1199,7 @@ func TestReacquireLeaseOnRestart(t *testing.T) {
11911199
DisableMaxOffsetCheck: true,
11921200
}
11931201

1194-
params, _ := createTestServerParams()
1202+
params, _ := createTestServerParamsAllowTenants()
11951203
params.Knobs.Store = storeTestingKnobs
11961204
params.Knobs.KVClient = clientTestingKnobs
11971205
var sqlDB *gosql.DB
@@ -1241,7 +1249,7 @@ func TestFlushUncommitedDescriptorCacheOnRestart(t *testing.T) {
12411249
},
12421250
}
12431251

1244-
params, _ := createTestServerParams()
1252+
params, _ := createTestServerParamsAllowTenants()
12451253
params.Knobs.Store = testingKnobs
12461254
s, sqlDB, _ := serverutils.StartServer(t, params)
12471255
defer s.Stopper().Stop(context.Background())
@@ -1311,7 +1319,9 @@ func TestDistSQLRetryableError(t *testing.T) {
13111319
// targetKey is represents one of the rows in the table.
13121320
// +2 since the first two available ids are allocated to the database and
13131321
// public schema.
1314-
firstTableID := func() (id uint32) {
1322+
var firstTableID uint32
1323+
var codec keys.SQLCodec
1324+
func() {
13151325
tc := serverutils.StartCluster(t, 3, /* numNodes */
13161326
base.TestClusterArgs{
13171327
ReplicationMode: base.ReplicationManual,
@@ -1322,12 +1332,12 @@ func TestDistSQLRetryableError(t *testing.T) {
13221332
createTable(db)
13231333
row := db.QueryRow("SELECT 't'::REGCLASS::OID")
13241334
require.NotNil(t, row)
1325-
require.NoError(t, row.Scan(&id))
1326-
return id
1335+
require.NoError(t, row.Scan(&firstTableID))
1336+
codec = tc.Server(0).Codec()
13271337
}()
13281338
indexID := uint32(1)
13291339
valInTable := uint64(2)
1330-
indexKey := keys.SystemSQLCodec.IndexPrefix(firstTableID, indexID)
1340+
indexKey := codec.IndexPrefix(firstTableID, indexID)
13311341
targetKey := encoding.EncodeUvarintAscending(indexKey, valInTable)
13321342

13331343
restarted := true
@@ -1366,6 +1376,11 @@ func TestDistSQLRetryableError(t *testing.T) {
13661376
})
13671377
defer tc.Stopper().Stop(context.Background())
13681378

1379+
if tc.DefaultTenantDeploymentMode().IsExternal() {
1380+
tc.GrantTenantCapabilities(ctx, t, serverutils.TestTenantID(),
1381+
map[tenantcapabilitiespb.ID]string{tenantcapabilitiespb.CanAdminRelocateRange: "true"})
1382+
}
1383+
13691384
db := tc.ServerConn(0)
13701385
createTable(db)
13711386

@@ -1460,7 +1475,7 @@ func TestRollbackToSavepointFromUnusualStates(t *testing.T) {
14601475
defer leaktest.AfterTest(t)()
14611476
defer log.Scope(t).Close(t)
14621477

1463-
params, _ := createTestServerParams()
1478+
params, _ := createTestServerParamsAllowTenants()
14641479
s, sqlDB, _ := serverutils.StartServer(t, params)
14651480
defer s.Stopper().Stop(context.Background())
14661481

@@ -1523,7 +1538,7 @@ func TestTxnAutoRetriesDisabledAfterResultsHaveBeenSentToClient(t *testing.T) {
15231538
defer leaktest.AfterTest(t)()
15241539
defer log.Scope(t).Close(t)
15251540

1526-
params, _ := createTestServerParams()
1541+
params, _ := createTestServerParamsAllowTenants()
15271542
s, sqlDB, _ := serverutils.StartServer(t, params)
15281543
defer s.Stopper().Stop(context.Background())
15291544
defer sqlDB.Close()
@@ -1612,7 +1627,7 @@ func TestTxnAutoRetryReasonAvailable(t *testing.T) {
16121627
const numRetries = 3
16131628
retryCount := 0
16141629

1615-
params, cmdFilters := createTestServerParams()
1630+
params, cmdFilters := createTestServerParamsAllowTenants()
16161631
params.Knobs.SQLExecutor = &sql.ExecutorTestingKnobs{
16171632
BeforeRestart: func(ctx context.Context, reason error) {
16181633
retryCount++

‎pkg/sql/unsplit_range_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"errors"
1313
"testing"
1414

15+
"github.com/cockroachdb/cockroach/pkg/base"
1516
"github.com/cockroachdb/cockroach/pkg/jobs"
1617
"github.com/cockroachdb/cockroach/pkg/keys"
1718
"github.com/cockroachdb/cockroach/pkg/kv"
@@ -269,7 +270,8 @@ func TestUnsplitRanges(t *testing.T) {
269270

270271
ctx := context.Background()
271272
run := func(t *testing.T, tc testCase) {
272-
params, _ := createTestServerParams()
273+
params, _ := createTestServerParamsAllowTenants()
274+
params.DefaultTestTenant = base.TestDoesNotWorkWithExternalProcessMode(142388)
273275
params.Knobs.JobsTestingKnobs = jobs.NewTestingKnobsWithShortIntervals()
274276
params.Knobs.GCJob = &sql.GCJobTestingKnobs{
275277
SkipWaitingForMVCCGC: true,

0 commit comments

Comments
 (0)