Skip to content

Commit a70b974

Browse files
committed
sql: enable test tenants
Some tests have been adjusted to work with test tenants, others have specific issues to investigate further. Release note: None
1 parent 6406b7d commit a70b974

35 files changed

+327
-258
lines changed

pkg/base/test_server_args.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,8 @@ type DefaultTestTenantOptions struct {
326326
// warn".
327327
noWarnImplicitInterfaces bool
328328

329-
// If test tenant is disabled, issue and label to link in log message.
329+
// If test tenant is disabled, issue and label to link in log message. These
330+
// can be left unset if one of the tenant modes is explicitly skipped.
330331
issueNum int
331332
label string
332333
}
@@ -575,6 +576,12 @@ func TestDoesNotWorkWithExternalProcessMode(issueNumber int) DefaultTestTenantOp
575576
return testSkippedForExternalProcessMode(issueNumber)
576577
}
577578

579+
// TestSkipForExternalProcessMode disables selecting the external process
580+
// virtual cluster for tests that are not applicable to that mode.
581+
func TestSkipForExternalProcessMode() DefaultTestTenantOptions {
582+
return testSkippedForExternalProcessMode(0 /* issueNumber */)
583+
}
584+
578585
func testSkippedForExternalProcessMode(issueNumber int) DefaultTestTenantOptions {
579586
return DefaultTestTenantOptions{
580587
testBehavior: ttSharedProcess,

pkg/ccl/changefeedccl/schemafeed/schematestutils/schema_test_utils.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,7 @@ func FetchDescVersionModificationTime(
145145
tableName string,
146146
version int,
147147
) hlc.Timestamp {
148-
db := serverutils.OpenDBConn(
149-
t, s.SQLAddr(), dbName, false, s.AppStopper())
148+
db := s.SQLConn(t, serverutils.DBName(dbName))
150149

151150
tblKey := s.Codec().IndexPrefix(keys.DescriptorTableID, keys.DescriptorTablePrimaryKeyIndexID)
152151
header := kvpb.RequestHeader{

pkg/ccl/testccl/sqlccl/gc_job_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func TestGCJobGetsMarkedIdle(t *testing.T) {
3232
s, mainDB, _ := serverutils.StartServer(t, base.TestServerArgs{
3333
DefaultTestTenant: base.TestControlsTenantsExplicitly,
3434
})
35-
sqltestutils.SetShortRangeFeedIntervals(t, mainDB)
35+
sqltestutils.SetShortRangeFeedIntervals(t, s)
3636
defer s.Stopper().Stop(ctx)
3737
tenant, tenantDB := serverutils.StartTenant(t, s, base.TestTenantArgs{
3838
TenantID: serverutils.TestTenantID(),

pkg/sql/authorization_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,16 +98,16 @@ func TestConcurrentGrants(t *testing.T) {
9898

9999
runConcurrentGrantsWithPriority := func(t *testing.T, priority string) {
100100
ctx := context.Background()
101-
s, db, _ := serverutils.StartServer(t, base.TestServerArgs{})
102-
defer s.Stopper().Stop(ctx)
101+
srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{})
102+
defer srv.Stopper().Stop(ctx)
103103
tdb := sqlutils.MakeSQLRunner(db)
104104

105105
// Shortening this cluster setting is essential for this test. A small value
106106
// will force `txn1` to bump up its write timestamp by the time it gets to
107107
// commit. This is needed to force `txn2` to encounter a WriteTooOldError
108108
// after being unblocked, and hence enter a retry, as retry in `txn2` is
109109
// prerequisite for the potential deadlock described in #117144.
110-
tdb.Exec(t, "SET CLUSTER SETTING kv.closed_timestamp.target_duration = '1ms'")
110+
sqlutils.MakeSQLRunner(srv.SystemLayer().SQLConn(t)).Exec(t, "SET CLUSTER SETTING kv.closed_timestamp.target_duration = '1ms'")
111111

112112
tdb.Exec(t, "CREATE ROLE developer;")
113113
tdb.Exec(t, "CREATE USER user1")

pkg/sql/backfill_protected_timestamp_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ func TestBackfillQueryWithProtectedTS(t *testing.T) {
241241
var db *gosql.DB
242242
var tableID uint32
243243
s, db, _ = serverutils.StartServer(t, base.TestServerArgs{
244+
DefaultTestTenant: base.TestIsForStuffThatShouldWorkWithSecondaryTenantsButDoesntYet(156127),
244245
Knobs: base.TestingKnobs{
245246
SQLEvalContext: &eval.TestingKnobs{
246247
ForceProductionValues: true,

pkg/sql/conn_executor_test.go

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,9 @@ func TestPrepareInExplicitTransactionDoesNotDeadlock(t *testing.T) {
643643
defer leaktest.AfterTest(t)()
644644
defer log.Scope(t).Close(t)
645645

646-
s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{})
646+
s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{
647+
DefaultTestTenant: base.TestDoesNotWorkWithExternalProcessMode(156146),
648+
})
647649
defer s.Stopper().Stop(context.Background())
648650

649651
testDB := sqlutils.MakeSQLRunner(sqlDB)
@@ -1737,8 +1739,11 @@ func TestAbortedTxnLocks(t *testing.T) {
17371739
defer log.Scope(t).Close(t)
17381740
ctx := context.Background()
17391741

1740-
s := serverutils.StartServerOnly(t, base.TestServerArgs{})
1741-
defer s.Stopper().Stop(ctx)
1742+
srv := serverutils.StartServerOnly(t, base.TestServerArgs{
1743+
DefaultTestTenant: base.TestDoesNotWorkWithSecondaryTenantsButWeDontKnowWhyYet(156127),
1744+
})
1745+
defer srv.Stopper().Stop(ctx)
1746+
s := srv.ApplicationLayer()
17421747

17431748
var TransactionStatus string
17441749

@@ -2425,22 +2430,19 @@ func TestInternalAppNamePrefix(t *testing.T) {
24252430
ctx := context.Background()
24262431
params := base.TestServerArgs{}
24272432
params.Insecure = true
2428-
s, sqlDB, _ := serverutils.StartServer(t, params)
2429-
defer s.Stopper().Stop(ctx)
2433+
srv, sqlDB, _ := serverutils.StartServer(t, params)
2434+
defer srv.Stopper().Stop(ctx)
2435+
s := srv.ApplicationLayer()
24302436

24312437
// Create a test table.
24322438
_, err := sqlDB.Exec("CREATE TABLE test (k INT PRIMARY KEY, v INT)")
24332439
require.NoError(t, err)
24342440

24352441
t.Run("app name set at conn init", func(t *testing.T) {
24362442
// Create a connection.
2437-
connURL := url.URL{
2438-
Scheme: "postgres",
2439-
User: url.User(username.RootUser),
2440-
Host: s.AdvSQLAddr(),
2441-
}
2443+
connURL, cleanup := s.PGUrl(t, serverutils.User(username.RootUser))
2444+
defer cleanup()
24422445
q := connURL.Query()
2443-
q.Add("sslmode", "disable")
24442446
q.Add("application_name", catconstants.InternalAppNamePrefix+"mytest")
24452447
connURL.RawQuery = q.Encode()
24462448
db, err := gosql.Open("postgres", connURL.String())
@@ -2462,15 +2464,7 @@ func TestInternalAppNamePrefix(t *testing.T) {
24622464

24632465
t.Run("app name set in session", func(t *testing.T) {
24642466
// Create a connection.
2465-
connURL := url.URL{
2466-
Scheme: "postgres",
2467-
User: url.User(username.RootUser),
2468-
Host: s.AdvSQLAddr(),
2469-
RawQuery: "sslmode=disable",
2470-
}
2471-
db, err := gosql.Open("postgres", connURL.String())
2472-
require.NoError(t, err)
2473-
defer db.Close()
2467+
db := s.SQLConn(t, serverutils.User(username.RootUser))
24742468
runner := sqlutils.MakeSQLRunner(db)
24752469

24762470
// Get initial metric values

pkg/sql/crdb_internal_test.go

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"context"
1010
gosql "database/sql"
1111
"fmt"
12-
"net/url"
1312
"strings"
1413
"sync"
1514
"sync/atomic"
@@ -32,6 +31,7 @@ import (
3231
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/protectedts"
3332
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/protectedts/ptpb"
3433
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/protectedts/ptstorage"
34+
"github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilitiespb"
3535
"github.com/cockroachdb/cockroach/pkg/roachpb"
3636
"github.com/cockroachdb/cockroach/pkg/scheduledjobs"
3737
"github.com/cockroachdb/cockroach/pkg/security/username"
@@ -122,7 +122,8 @@ func TestRangeLocalityBasedOnNodeIDs(t *testing.T) {
122122
tc := testcluster.StartTestCluster(t, 1,
123123
base.TestClusterArgs{
124124
ServerArgs: base.TestServerArgs{
125-
Locality: roachpb.Locality{Tiers: []roachpb.Tier{{Key: "node", Value: "1"}}},
125+
Locality: roachpb.Locality{Tiers: []roachpb.Tier{{Key: "node", Value: "1"}}},
126+
DefaultTestTenant: base.TestIsSpecificToStorageLayerAndNeedsASystemTenant,
126127
},
127128
ReplicationMode: base.ReplicationAuto,
128129
},
@@ -581,6 +582,11 @@ func TestDistSQLFlowsVirtualTables(t *testing.T) {
581582
})
582583
defer tc.Stopper().Stop(context.Background())
583584

585+
if tc.DefaultTenantDeploymentMode().IsExternal() {
586+
tc.GrantTenantCapabilities(context.Background(), t, serverutils.TestTenantID(),
587+
map[tenantcapabilitiespb.ID]string{tenantcapabilitiespb.CanAdminRelocateRange: "true"})
588+
}
589+
584590
// Create a table with 3 rows, split them into 3 ranges with each node
585591
// having one.
586592
db := tc.ServerConn(gatewayNodeID)
@@ -855,13 +861,17 @@ func TestTxnContentionEventsTableWithRangeDescriptor(t *testing.T) {
855861
defer log.Scope(t).Close(t)
856862

857863
ctx := context.Background()
858-
s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{})
859-
defer s.Stopper().Stop(ctx)
864+
srv, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{
865+
DefaultTestTenant: base.TestDoesNotWorkWithSecondaryTenantsButWeDontKnowWhyYet(156145),
866+
})
867+
defer srv.Stopper().Stop(ctx)
868+
s := srv.ApplicationLayer()
869+
860870
_, err := sqlDB.Exec("SET CLUSTER SETTING sql.contention.event_store.resolution_interval = '10ms'")
861871
require.NoError(t, err)
862872
rangeKey := "/Local/Range/Table/106/1/-1704619207610523008/RangeDescriptor"
863873
rangeKeyEscaped := fmt.Sprintf("\"%s\"", rangeKey)
864-
s.ApplicationLayer().ExecutorConfig().(sql.ExecutorConfig).ContentionRegistry.AddContentionEvent(contentionpb.ExtendedContentionEvent{
874+
s.ExecutorConfig().(sql.ExecutorConfig).ContentionRegistry.AddContentionEvent(contentionpb.ExtendedContentionEvent{
865875
BlockingEvent: kvpb.ContentionEvent{
866876
Key: roachpb.Key(rangeKey),
867877
TxnMeta: enginepb.TxnMeta{
@@ -1367,33 +1377,27 @@ func TestInternalSystemJobsAccess(t *testing.T) {
13671377
defer leaktest.AfterTest(t)()
13681378
defer log.Scope(t).Close(t)
13691379

1370-
s, db, _ := serverutils.StartServer(t, base.TestServerArgs{
1380+
srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{
13711381
Knobs: base.TestingKnobs{
13721382
KeyVisualizer: &keyvisualizer.TestingKnobs{SkipJobBootstrap: true},
13731383
},
13741384
})
13751385
ctx := context.Background()
1376-
defer s.Stopper().Stop(ctx)
1386+
defer srv.Stopper().Stop(ctx)
1387+
s := srv.ApplicationLayer()
13771388
rootDB := sqlutils.MakeSQLRunner(db)
13781389

13791390
// Even though this test modifies the system.jobs table and asserts its contents, we
13801391
// do not disable background job creation nor job adoption. This is because creating
13811392
// users requires jobs to be created and run. Thus, this test only creates jobs of type
13821393
// jobspb.TypeImport and overrides the import resumer.
1383-
registry := s.ApplicationLayer().JobRegistry().(*jobs.Registry)
1394+
registry := s.JobRegistry().(*jobs.Registry)
13841395
registry.TestingWrapResumerConstructor(jobspb.TypeImport, func(r jobs.Resumer) jobs.Resumer {
13851396
return &fakeResumer{}
13861397
})
13871398

13881399
asUser := func(user string, f func(userDB *sqlutils.SQLRunner)) {
1389-
pgURL := url.URL{
1390-
Scheme: "postgres",
1391-
User: url.UserPassword(user, "test"),
1392-
Host: s.AdvSQLAddr(),
1393-
}
1394-
db2, err := gosql.Open("postgres", pgURL.String())
1395-
assert.NoError(t, err)
1396-
defer db2.Close()
1400+
db2 := s.SQLConn(t, serverutils.UserPassword(user, "test"), serverutils.ClientCerts(false))
13971401
userDB := sqlutils.MakeSQLRunner(db2)
13981402

13991403
f(userDB)

pkg/sql/create_as_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/cockroachdb/cockroach/pkg/sql/types"
1919
"github.com/cockroachdb/cockroach/pkg/testutils/kvclientutils"
2020
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
21+
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
2122
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
2223
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
2324
"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -30,6 +31,8 @@ func TestCreateAsVTable(t *testing.T) {
3031
defer leaktest.AfterTest(t)()
3132
defer log.Scope(t).Close(t)
3233

34+
skip.UnderRace(t, "too slow")
35+
3336
ctx := context.Background()
3437
s, db, _ := serverutils.StartServer(t, base.TestServerArgs{})
3538
defer s.Stopper().Stop(ctx)
@@ -83,12 +86,17 @@ func TestCreateAsVTable(t *testing.T) {
8386
`"".crdb_internal.gossip_liveness`: {},
8487
`"".crdb_internal.gossip_nodes`: {},
8588
`"".crdb_internal.kv_flow_controller`: {},
89+
`"".crdb_internal.kv_flow_controller_v2`: {},
8690
`"".crdb_internal.kv_flow_control_handles`: {},
91+
`"".crdb_internal.kv_flow_control_handles_v2`: {},
8792
`"".crdb_internal.kv_flow_token_deductions`: {},
93+
`"".crdb_internal.kv_flow_token_deductions_v2`: {},
8894
`"".crdb_internal.kv_node_status`: {},
8995
`"".crdb_internal.kv_node_liveness`: {},
9096
`"".crdb_internal.kv_store_status`: {},
9197
`"".crdb_internal.node_tenant_capabilities_cache`: {},
98+
`"".crdb_internal.store_liveness_support_for`: {},
99+
`"".crdb_internal.store_liveness_support_from`: {},
92100
`"".crdb_internal.tenant_usage_details`: {},
93101
}
94102
if _, ok := onlySystemTenant[fqName]; ok {

pkg/sql/create_stats_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ func TestStatsWithLowTTL(t *testing.T) {
5959
},
6060
},
6161
},
62+
// In external-process mode the tenant doesn't have kvpb.GCRequest
63+
// capability (and this capability can't be granted at the time of
64+
// writing either), so we skip the external mode only.
65+
DefaultTestTenant: base.TestSkipForExternalProcessMode(),
6266
})
6367
defer s.Stopper().Stop(context.Background())
6468

pkg/sql/distsql_physical_planner_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,10 @@ func TestDistSQLRangeCachesIntegrationTest(t *testing.T) {
278278
ReplicationMode: base.ReplicationManual,
279279
ServerArgs: base.TestServerArgs{
280280
UseDatabase: "test",
281+
// Probably this test could work in shared-process mode, but
282+
// it's occasionally flaking there and doesn't seem worth
283+
// investigating since we're touching the ranges directly.
284+
DefaultTestTenant: base.TestIsSpecificToStorageLayerAndNeedsASystemTenant,
281285
},
282286
})
283287
defer tc.Stopper().Stop(context.Background())
@@ -298,7 +302,7 @@ func TestDistSQLRangeCachesIntegrationTest(t *testing.T) {
298302
//
299303
// TODO(andrei): This is super hacky. What this test really wants to do is to
300304
// precisely control the contents of the range cache on node 4.
301-
tc.Server(3).DistSenderI().(*kvcoord.DistSender).DisableFirstRangeUpdates()
305+
tc.ApplicationLayer(3).DistSenderI().(*kvcoord.DistSender).DisableFirstRangeUpdates()
302306
db3 := tc.ServerConn(3)
303307
// Force the DistSQL on this connection.
304308
_, err := db3.Exec(`SET CLUSTER SETTING sql.defaults.distsql = always;`)

0 commit comments

Comments
 (0)