From df0c4ffbf1d04ab690abe30a847fd254c0418cac Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Tue, 15 Aug 2023 13:20:47 +0200 Subject: [PATCH 1/3] testutils: add testserver APIs useful for DistSQL Release note: None --- .../multiregion_system_table_test.go | 2 +- pkg/ccl/testccl/sqlstatsccl/sql_stats_test.go | 2 +- pkg/kv/kvserver/client_lease_test.go | 6 ++-- pkg/server/testserver.go | 32 +++++++++++++++++-- pkg/sql/distsql_physical_planner.go | 4 +++ pkg/sql/physicalplan/span_resolver_test.go | 6 ++-- pkg/testutils/serverutils/api.go | 18 ++++++----- 7 files changed, 52 insertions(+), 18 deletions(-) diff --git a/pkg/ccl/multiregionccl/multiregion_system_table_test.go b/pkg/ccl/multiregionccl/multiregion_system_table_test.go index 943260386a7f..fca216b037c0 100644 --- a/pkg/ccl/multiregionccl/multiregion_system_table_test.go +++ b/pkg/ccl/multiregionccl/multiregion_system_table_test.go @@ -55,7 +55,7 @@ func TestMrSystemDatabase(t *testing.T) { tenantArgs := base.TestTenantArgs{ Settings: cs, TenantID: id, - Locality: *cluster.Servers[0].Locality(), + Locality: cluster.Servers[0].Locality(), } _, tenantSQL := serverutils.StartTenant(t, cluster.Servers[0], tenantArgs) diff --git a/pkg/ccl/testccl/sqlstatsccl/sql_stats_test.go b/pkg/ccl/testccl/sqlstatsccl/sql_stats_test.go index 0b1e7343605e..4f33f82b018e 100644 --- a/pkg/ccl/testccl/sqlstatsccl/sql_stats_test.go +++ b/pkg/ccl/testccl/sqlstatsccl/sql_stats_test.go @@ -120,7 +120,7 @@ func TestSQLStatsRegions(t *testing.T) { _, tenantDb := serverutils.StartTenant(t, server, base.TestTenantArgs{ Settings: st, TenantID: roachpb.MustMakeTenantID(11), - Locality: *server.Locality(), + Locality: server.Locality(), }) tenantDbs = append(tenantDbs, tenantDb) } diff --git a/pkg/kv/kvserver/client_lease_test.go b/pkg/kv/kvserver/client_lease_test.go index 67bb11bc050b..dfdcde67809d 100644 --- a/pkg/kv/kvserver/client_lease_test.go +++ b/pkg/kv/kvserver/client_lease_test.go @@ -1054,7 +1054,8 @@ func TestLeasePreferencesDuringOutage(t *testing.T) { srv, err := tc.FindMemberServer(newLeaseHolder.StoreID) require.NoError(t, err) - region, ok := srv.Locality().Find("region") + loc := srv.Locality() + region, ok := loc.Find("region") require.True(t, ok) require.Equal(t, "us", region) require.Equal(t, 3, len(repl.Desc().Replicas().Voters().VoterDescriptors())) @@ -1062,7 +1063,8 @@ func TestLeasePreferencesDuringOutage(t *testing.T) { for _, replDesc := range repl.Desc().Replicas().Voters().VoterDescriptors() { serv, err := tc.FindMemberServer(replDesc.StoreID) require.NoError(t, err) - dc, ok := serv.Locality().Find("dc") + memberLoc := serv.Locality() + dc, ok := memberLoc.Find("dc") require.True(t, ok) require.NotEqual(t, "sf", dc) } diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go index 144e592c07b0..3ab342eb7838 100644 --- a/pkg/server/testserver.go +++ b/pkg/server/testserver.go @@ -901,11 +901,27 @@ func (t *testTenant) DistSenderI() interface{} { return t.sql.execCfg.DistSender } +// NodeDescStoreI is part of the serverutils.ApplicationLayerInterface. +func (t *testTenant) NodeDescStoreI() interface{} { + return t.sql.execCfg.DistSQLPlanner.NodeDescStore() +} + // InternalDB is part of the serverutils.ApplicationLayerInterface. func (t *testTenant) InternalDB() interface{} { return t.sql.internalDB } +// Locality is part of the serverutils.ApplicationLayerInterface. +func (t *testTenant) Locality() roachpb.Locality { + return t.Cfg.Locality +} + +// DistSQLPlanningNodeID is part of the serverutils.ApplicationLayerInterface. +func (t *testTenant) DistSQLPlanningNodeID() roachpb.NodeID { + // See comments on replicaoracle.Config. + return 0 +} + // LeaseManager is part of the serverutils.ApplicationLayerInterface. func (t *testTenant) LeaseManager() interface{} { return t.sql.leaseMgr @@ -1704,9 +1720,14 @@ func (ts *testServer) MustGetSQLNetworkCounter(name string) int64 { return mustGetSQLCounterForRegistry(reg, name) } -// Locality is part of the serverutils.StorageLayerInterface. -func (ts *testServer) Locality() *roachpb.Locality { - return &ts.cfg.Locality +// Locality is part of the serverutils.ApplicationLayerInterface. +func (ts *testServer) Locality() roachpb.Locality { + return ts.cfg.Locality +} + +// DistSQLPlanningNodeID is part of the serverutils.ApplicationLayerInterface. +func (ts *testServer) DistSQLPlanningNodeID() roachpb.NodeID { + return ts.NodeID() } // LeaseManager is part of the serverutils.ApplicationLayerInterface. @@ -1734,6 +1755,11 @@ func (ts *testServer) DistSenderI() interface{} { return ts.distSender } +// NodeDescStoreI is part of the serverutils.ApplicationLayerInterface. +func (ts *testServer) NodeDescStoreI() interface{} { + return ts.sqlServer.execCfg.DistSQLPlanner.NodeDescStore() +} + // MigrationServer is part of the serverutils.ApplicationLayerInterface. func (ts *testServer) MigrationServer() interface{} { return ts.topLevelServer.migrationServer diff --git a/pkg/sql/distsql_physical_planner.go b/pkg/sql/distsql_physical_planner.go index 09be0629bb44..ef8a1613dba5 100644 --- a/pkg/sql/distsql_physical_planner.go +++ b/pkg/sql/distsql_physical_planner.go @@ -4768,3 +4768,7 @@ func (dsp *DistSQLPlanner) createPlanForInsert( execinfrapb.Ordering{}) return plan, nil } + +func (dsp *DistSQLPlanner) NodeDescStore() kvcoord.NodeDescStore { + return dsp.nodeDescs +} diff --git a/pkg/sql/physicalplan/span_resolver_test.go b/pkg/sql/physicalplan/span_resolver_test.go index cb55e4aba1ed..1109bc6b5246 100644 --- a/pkg/sql/physicalplan/span_resolver_test.go +++ b/pkg/sql/physicalplan/span_resolver_test.go @@ -90,7 +90,7 @@ func TestSpanResolverUsesCaches(t *testing.T) { s3.DistSenderI().(*kvcoord.DistSender), s3.GossipI().(*gossip.Gossip), s3.NodeID(), - *s3.Locality(), + s3.Locality(), s3.Clock(), nil, // rpcCtx replicaoracle.BinPackingChoice) @@ -202,7 +202,7 @@ func TestSpanResolver(t *testing.T) { s.DistSenderI().(*kvcoord.DistSender), s.GossipI().(*gossip.Gossip), s.NodeID(), - *s.Locality(), + s.Locality(), s.Clock(), nil, // rpcCtx replicaoracle.BinPackingChoice) @@ -301,7 +301,7 @@ func TestMixedDirections(t *testing.T) { s.DistSenderI().(*kvcoord.DistSender), s.GossipI().(*gossip.Gossip), s.NodeID(), - *s.Locality(), + s.Locality(), s.Clock(), nil, // rpcCtx replicaoracle.BinPackingChoice) diff --git a/pkg/testutils/serverutils/api.go b/pkg/testutils/serverutils/api.go index 92ef526770c8..847614dda47e 100644 --- a/pkg/testutils/serverutils/api.go +++ b/pkg/testutils/serverutils/api.go @@ -404,6 +404,16 @@ type ApplicationLayerInterface interface { // server. The concrete return value is of type // privchecker.SQLPrivilegeChecker (interface). PrivilegeChecker() interface{} + + // NodeDescStoreI returns the node descriptor lookup interface. + // The concrete return type is compatible with interface kvcoord.NodeDescStore. + NodeDescStoreI() interface{} + + // Locality returns the locality used by the server. + Locality() roachpb.Locality + + // DistSQLPlanningNodeID returns the NodeID to use by the DistSQL span resolver. + DistSQLPlanningNodeID() roachpb.NodeID } // TenantControlInterface defines the API of a test server that can @@ -607,14 +617,6 @@ type StorageLayerInterface interface { // TsDB returns the ts.DB instance used by the TestServer. TsDB() interface{} - // Locality returns a pointer to the locality used by the server. - // - // TODO(test-eng): investigate if this should really be a pointer. - // - // TODO(test-eng): Investigate if this method should be on - // ApplicationLayerInterface instead. - Locality() *roachpb.Locality - // DefaultSystemZoneConfig returns the internal system zone config // for the server. // Note: most tests should instead use the .DefaultZoneConfig() method From 9c30b093f71a60971f2d100824dcd423fb93147c Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Tue, 15 Aug 2023 13:21:07 +0200 Subject: [PATCH 2/3] sql/physicalplan: make more tests compatible with secondary tenants Release note: None --- pkg/sql/physicalplan/BUILD.bazel | 1 - pkg/sql/physicalplan/aggregator_funcs_test.go | 35 ++++---- .../physicalplan/fake_span_resolver_test.go | 6 +- pkg/sql/physicalplan/span_resolver_test.go | 89 +++++++++++-------- 4 files changed, 74 insertions(+), 57 deletions(-) diff --git a/pkg/sql/physicalplan/BUILD.bazel b/pkg/sql/physicalplan/BUILD.bazel index aa27d71c363a..6fb380b1d48d 100644 --- a/pkg/sql/physicalplan/BUILD.bazel +++ b/pkg/sql/physicalplan/BUILD.bazel @@ -54,7 +54,6 @@ go_test( shard_count = 16, deps = [ "//pkg/base", - "//pkg/gossip", "//pkg/keys", "//pkg/kv", "//pkg/kv/kvclient/kvcoord", diff --git a/pkg/sql/physicalplan/aggregator_funcs_test.go b/pkg/sql/physicalplan/aggregator_funcs_test.go index 05f2345b9eec..e8db1e11c658 100644 --- a/pkg/sql/physicalplan/aggregator_funcs_test.go +++ b/pkg/sql/physicalplan/aggregator_funcs_test.go @@ -18,7 +18,6 @@ import ( "github.com/cockroachdb/apd/v3" "github.com/cockroachdb/cockroach/pkg/base" - "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog" @@ -147,17 +146,17 @@ func checkDistAggregationInfo( Spans: make([]roachpb.Span, 1), } if err := rowenc.InitIndexFetchSpec( - &tr.FetchSpec, keys.SystemSQLCodec, tableDesc, tableDesc.GetPrimaryIndex(), columnIDs, + &tr.FetchSpec, srv.Codec(), tableDesc, tableDesc.GetPrimaryIndex(), columnIDs, ); err != nil { t.Fatal(err) } var err error - tr.Spans[0].Key, err = randgen.TestingMakePrimaryIndexKey(tableDesc, startPK) + tr.Spans[0].Key, err = randgen.TestingMakePrimaryIndexKeyForTenant(tableDesc, srv.Codec(), startPK) if err != nil { t.Fatal(err) } - tr.Spans[0].EndKey, err = randgen.TestingMakePrimaryIndexKey(tableDesc, endPK) + tr.Spans[0].EndKey, err = randgen.TestingMakePrimaryIndexKeyForTenant(tableDesc, srv.Codec(), endPK) if err != nil { t.Fatal(err) } @@ -449,8 +448,9 @@ func TestSingleArgumentDistAggregateFunctions(t *testing.T) { defer log.Scope(t).Close(t) const numRows = 100 - tc := serverutils.StartCluster(t, 1, base.TestClusterArgs{}) - defer tc.Stopper().Stop(context.Background()) + srv, db, kvDB := serverutils.StartServer(t, base.TestServerArgs{}) + defer srv.Stopper().Stop(context.Background()) + ts := srv.ApplicationLayer() // Create a table with a few columns: // - k - primary key with values from 0 to number of rows @@ -466,7 +466,7 @@ func TestSingleArgumentDistAggregateFunctions(t *testing.T) { // - random ten bytes rng, _ := randutil.NewTestRand() sqlutils.CreateTable( - t, tc.ServerConn(0), "t", + t, db, "t", "k INT PRIMARY KEY, int1 INT, int2 INT, int3 INT, bool1 BOOL, bool2 BOOL, dec1 DECIMAL, dec2 DECIMAL, float1 FLOAT, float2 FLOAT, b BYTES", numRows, func(row int) []tree.Datum { @@ -489,8 +489,7 @@ func TestSingleArgumentDistAggregateFunctions(t *testing.T) { }, ) - kvDB := tc.Server(0).DB() - desc := desctestutils.TestingGetPublicTableDescriptor(kvDB, keys.SystemSQLCodec, "test", "t") + desc := desctestutils.TestingGetPublicTableDescriptor(kvDB, ts.Codec(), "test", "t") for fn, info := range physicalplan.DistAggregationTable { if fn == execinfrapb.AnyNotNull { @@ -528,7 +527,8 @@ func TestSingleArgumentDistAggregateFunctions(t *testing.T) { name := fmt.Sprintf("%s/%s/%d", fn, col.GetName(), numRows) t.Run(name, func(t *testing.T) { checkDistAggregationInfo( - context.Background(), t, tc.Server(0), desc, []int{col.Ordinal()}, + // TODO(#76378): pass ts, not srv, here. + context.Background(), t, srv, desc, []int{col.Ordinal()}, numRows, fn, info, ) }) @@ -550,8 +550,11 @@ func TestTwoArgumentRegressionAggregateFunctions(t *testing.T) { defer log.Scope(t).Close(t) const numRows = 100 - tc := serverutils.StartCluster(t, 1, base.TestClusterArgs{}) - defer tc.Stopper().Stop(context.Background()) + srv, db, kvDB := serverutils.StartServer(t, base.TestServerArgs{ + DefaultTestTenant: base.TestIsForStuffThatShouldWorkWithSecondaryTenantsButDoesntYet(108763), + }) + defer srv.Stopper().Stop(context.Background()) + ts := srv.ApplicationLayer() // Create a table with a few columns: // - k - primary key with values from 0 to number of rows @@ -563,7 +566,7 @@ func TestTwoArgumentRegressionAggregateFunctions(t *testing.T) { // - random decimals (with some NULLs) rng, _ := randutil.NewTestRand() sqlutils.CreateTable( - t, tc.ServerConn(0), "t", + t, db, "t", "k INT PRIMARY KEY, int1 INT, dec1 DECIMAL, float1 FLOAT, int2 INT, dec2 DECIMAL, float2 FLOAT", numRows, func(row int) []tree.Datum { @@ -579,8 +582,7 @@ func TestTwoArgumentRegressionAggregateFunctions(t *testing.T) { }, ) - kvDB := tc.Server(0).DB() - desc := desctestutils.TestingGetTableDescriptor(kvDB, keys.SystemSQLCodec, "test", "public", "t") + desc := desctestutils.TestingGetTableDescriptor(kvDB, ts.Codec(), "test", "public", "t") for fn, info := range physicalplan.DistAggregationTable { if !isTwoArgumentFunction(fn) { @@ -598,7 +600,8 @@ func TestTwoArgumentRegressionAggregateFunctions(t *testing.T) { name := fmt.Sprintf("%s/%s-%s/%d", fn, cols[i].GetName(), cols[j].GetName(), numRows) t.Run(name, func(t *testing.T) { checkDistAggregationInfo( - context.Background(), t, tc.Server(0), desc, []int{i, j}, numRows, + // TODO(#76378): pass ts, not srv, here. + context.Background(), t, srv, desc, []int{i, j}, numRows, fn, info, ) }) diff --git a/pkg/sql/physicalplan/fake_span_resolver_test.go b/pkg/sql/physicalplan/fake_span_resolver_test.go index 72370c91005f..66542c18a3f3 100644 --- a/pkg/sql/physicalplan/fake_span_resolver_test.go +++ b/pkg/sql/physicalplan/fake_span_resolver_test.go @@ -34,7 +34,11 @@ func TestFakeSpanResolver(t *testing.T) { defer log.Scope(t).Close(t) ctx := context.Background() - tc := serverutils.StartCluster(t, 3, base.TestClusterArgs{}) + tc := serverutils.StartCluster(t, 3, base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + DefaultTestTenant: base.TestIsForStuffThatShouldWorkWithSecondaryTenantsButDoesntYet(108763), + }, + }) defer tc.Stopper().Stop(ctx) sqlutils.CreateTable( diff --git a/pkg/sql/physicalplan/span_resolver_test.go b/pkg/sql/physicalplan/span_resolver_test.go index 1109bc6b5246..cc1987af54d1 100644 --- a/pkg/sql/physicalplan/span_resolver_test.go +++ b/pkg/sql/physicalplan/span_resolver_test.go @@ -17,9 +17,7 @@ import ( "testing" "github.com/cockroachdb/cockroach/pkg/base" - "github.com/cockroachdb/cockroach/pkg/gossip" "github.com/cockroachdb/cockroach/pkg/keys" - "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog" @@ -45,13 +43,17 @@ func TestSpanResolverUsesCaches(t *testing.T) { base.TestClusterArgs{ ReplicationMode: base.ReplicationManual, ServerArgs: base.TestServerArgs{ - UseDatabase: "t", + DefaultTestTenant: base.TestIsForStuffThatShouldWorkWithSecondaryTenantsButDoesntYet(108763), + UseDatabase: "t", }, }) defer tc.Stopper().Stop(context.Background()) rowRanges, _ := setupRanges( - tc.Conns[0], tc.Servers[0], tc.Servers[0].DB(), t) + tc.ServerConn(0), + tc.Server(0).ApplicationLayer(), + tc.Server(0).StorageLayer(), + t) // Replicate the row ranges on all of the first 3 nodes. Save the 4th node in // a pristine state, with empty caches. @@ -83,13 +85,13 @@ func TestSpanResolverUsesCaches(t *testing.T) { } // Create a SpanResolver using the 4th node, with empty caches. - s3 := tc.Servers[3] + s3 := tc.Server(3).ApplicationLayer() lr := physicalplan.NewSpanResolver( s3.ClusterSettings(), s3.DistSenderI().(*kvcoord.DistSender), - s3.GossipI().(*gossip.Gossip), - s3.NodeID(), + s3.NodeDescStoreI().(kvcoord.NodeDescStore), + s3.DistSQLPlanningNodeID(), s3.Locality(), s3.Clock(), nil, // rpcCtx @@ -169,18 +171,21 @@ func populateCache(db *gosql.DB, expectedNumRows int) error { // `CREATE TABLE test (k INT PRIMARY KEY)` at row with value pk (the row will be // the first on the right of the split). func splitRangeAtVal( - ts serverutils.TestServerInterface, tableDesc catalog.TableDescriptor, pk int, + s serverutils.ApplicationLayerInterface, + stg serverutils.StorageLayerInterface, + tableDesc catalog.TableDescriptor, + pk int, ) (roachpb.RangeDescriptor, roachpb.RangeDescriptor, error) { if len(tableDesc.PublicNonPrimaryIndexes()) != 0 { return roachpb.RangeDescriptor{}, roachpb.RangeDescriptor{}, errors.AssertionFailedf("expected table with just a PK, got: %+v", tableDesc) } - pik, err := randgen.TestingMakePrimaryIndexKey(tableDesc, pk) + pik, err := randgen.TestingMakePrimaryIndexKeyForTenant(tableDesc, s.Codec(), pk) if err != nil { return roachpb.RangeDescriptor{}, roachpb.RangeDescriptor{}, err } - leftRange, rightRange, err := ts.SplitRange(pik) + leftRange, rightRange, err := stg.SplitRange(pik) if err != nil { return roachpb.RangeDescriptor{}, roachpb.RangeDescriptor{}, errors.Wrapf(err, "failed to split at row: %d", pk) @@ -191,17 +196,18 @@ func splitRangeAtVal( func TestSpanResolver(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - s, db, cdb := serverutils.StartServer(t, base.TestServerArgs{ + ts, db, _ := serverutils.StartServer(t, base.TestServerArgs{ UseDatabase: "t", }) - defer s.Stopper().Stop(context.Background()) + defer ts.Stopper().Stop(context.Background()) + s := ts.ApplicationLayer() - rowRanges, tableDesc := setupRanges(db, s, cdb, t) + rowRanges, tableDesc := setupRanges(db, s, ts.StorageLayer(), t) lr := physicalplan.NewSpanResolver( s.ClusterSettings(), s.DistSenderI().(*kvcoord.DistSender), - s.GossipI().(*gossip.Gossip), - s.NodeID(), + s.NodeDescStoreI().(kvcoord.NodeDescStore), + s.DistSQLPlanningNodeID(), s.Locality(), s.Clock(), nil, // rpcCtx @@ -215,7 +221,7 @@ func TestSpanResolver(t *testing.T) { expected [][]rngInfo }{ { - []roachpb.Span{makeSpan(tableDesc, 0, 10000)}, + []roachpb.Span{makeSpan(tableDesc, s.Codec(), 0, 10000)}, [][]rngInfo{{ onlyReplica(rowRanges[0]), onlyReplica(rowRanges[1]), @@ -223,9 +229,9 @@ func TestSpanResolver(t *testing.T) { }, { []roachpb.Span{ - makeSpan(tableDesc, 0, 9), - makeSpan(tableDesc, 11, 19), - makeSpan(tableDesc, 21, 29), + makeSpan(tableDesc, s.Codec(), 0, 9), + makeSpan(tableDesc, s.Codec(), 11, 19), + makeSpan(tableDesc, s.Codec(), 21, 29), }, [][]rngInfo{ {onlyReplica(rowRanges[0])}, @@ -235,8 +241,8 @@ func TestSpanResolver(t *testing.T) { }, { []roachpb.Span{ - makeSpan(tableDesc, 0, 20), - makeSpan(tableDesc, 20, 29), + makeSpan(tableDesc, s.Codec(), 0, 20), + makeSpan(tableDesc, s.Codec(), 20, 29), }, [][]rngInfo{ {onlyReplica(rowRanges[0]), onlyReplica(rowRanges[1])}, @@ -245,12 +251,12 @@ func TestSpanResolver(t *testing.T) { }, { []roachpb.Span{ - makeSpan(tableDesc, 0, 1), - makeSpan(tableDesc, 1, 2), - makeSpan(tableDesc, 2, 3), - makeSpan(tableDesc, 3, 4), - makeSpan(tableDesc, 5, 11), - makeSpan(tableDesc, 20, 29), + makeSpan(tableDesc, s.Codec(), 0, 1), + makeSpan(tableDesc, s.Codec(), 1, 2), + makeSpan(tableDesc, s.Codec(), 2, 3), + makeSpan(tableDesc, s.Codec(), 3, 4), + makeSpan(tableDesc, s.Codec(), 5, 11), + makeSpan(tableDesc, s.Codec(), 20, 29), }, [][]rngInfo{ {onlyReplica(rowRanges[0])}, @@ -290,17 +296,19 @@ func TestSpanResolver(t *testing.T) { func TestMixedDirections(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - s, db, cdb := serverutils.StartServer(t, base.TestServerArgs{ + ts, db, _ := serverutils.StartServer(t, base.TestServerArgs{ UseDatabase: "t", }) - defer s.Stopper().Stop(context.Background()) + defer ts.Stopper().Stop(context.Background()) - rowRanges, tableDesc := setupRanges(db, s, cdb, t) + s := ts.ApplicationLayer() + + rowRanges, tableDesc := setupRanges(db, s, ts.StorageLayer(), t) lr := physicalplan.NewSpanResolver( s.ClusterSettings(), s.DistSenderI().(*kvcoord.DistSender), - s.GossipI().(*gossip.Gossip), - s.NodeID(), + s.NodeDescStoreI().(kvcoord.NodeDescStore), + s.DistSQLPlanningNodeID(), s.Locality(), s.Clock(), nil, // rpcCtx @@ -310,8 +318,8 @@ func TestMixedDirections(t *testing.T) { it := lr.NewSpanResolverIterator(nil, nil) spans := []spanWithDir{ - orient(kvcoord.Ascending, makeSpan(tableDesc, 11, 15))[0], - orient(kvcoord.Descending, makeSpan(tableDesc, 1, 14))[0], + orient(kvcoord.Ascending, makeSpan(tableDesc, s.Codec(), 11, 15))[0], + orient(kvcoord.Descending, makeSpan(tableDesc, s.Codec(), 1, 14))[0], } replicas, err := resolveSpans(ctx, it, spans...) if err != nil { @@ -327,7 +335,10 @@ func TestMixedDirections(t *testing.T) { } func setupRanges( - db *gosql.DB, s serverutils.TestServerInterface, cdb *kv.DB, t *testing.T, + db *gosql.DB, + s serverutils.ApplicationLayerInterface, + stg serverutils.StorageLayerInterface, + t *testing.T, ) ([]roachpb.RangeDescriptor, catalog.TableDescriptor) { if _, err := db.Exec(`CREATE DATABASE t`); err != nil { t.Fatal(err) @@ -344,13 +355,13 @@ func setupRanges( } } - tableDesc := desctestutils.TestingGetPublicTableDescriptor(cdb, keys.SystemSQLCodec, "t", "test") + tableDesc := desctestutils.TestingGetPublicTableDescriptor(s.DB(), s.Codec(), "t", "test") // Split every SQL row to its own range. rowRanges := make([]roachpb.RangeDescriptor, len(values)) for i, val := range values { var err error var l roachpb.RangeDescriptor - l, rowRanges[i], err = splitRangeAtVal(s, tableDesc, val) + l, rowRanges[i], err = splitRangeAtVal(s, stg, tableDesc, val) if err != nil { t.Fatal(err) } @@ -458,9 +469,9 @@ func expectResolved(actual [][]rngInfo, expected ...[]rngInfo) error { return nil } -func makeSpan(tableDesc catalog.TableDescriptor, i, j int) roachpb.Span { +func makeSpan(tableDesc catalog.TableDescriptor, codec keys.SQLCodec, i, j int) roachpb.Span { makeKey := func(val int) roachpb.Key { - key, err := randgen.TestingMakePrimaryIndexKey(tableDesc, val) + key, err := randgen.TestingMakePrimaryIndexKeyForTenant(tableDesc, codec, val) if err != nil { panic(err) } From cebb8bae765a9ae8afdb99fc5847a1efe2003cfb Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Tue, 15 Aug 2023 13:17:52 +0200 Subject: [PATCH 3/3] sql: make TestScatterResponse compatible with secondary tenants Release note: None --- pkg/sql/scatter_test.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/pkg/sql/scatter_test.go b/pkg/sql/scatter_test.go index e1a7d00381b7..94edac3cab63 100644 --- a/pkg/sql/scatter_test.go +++ b/pkg/sql/scatter_test.go @@ -17,8 +17,8 @@ import ( "testing" "github.com/cockroachdb/cockroach/pkg/base" - "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/catalog/desctestutils" "github.com/cockroachdb/cockroach/pkg/sql/randgen" "github.com/cockroachdb/cockroach/pkg/testutils" @@ -115,8 +115,13 @@ func TestScatterResponse(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{}) - defer s.Stopper().Stop(context.Background()) + ts, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{}) + defer ts.Stopper().Stop(context.Background()) + + s := ts.ApplicationLayer() + + sql.SecondaryTenantSplitAtEnabled.Override(ctx, &s.ClusterSettings().SV, true) + sql.SecondaryTenantScatterEnabled.Override(ctx, &s.ClusterSettings().SV, true) sqlutils.CreateTable( t, sqlDB, "t", @@ -124,7 +129,7 @@ func TestScatterResponse(t *testing.T) { 1000, sqlutils.ToRowFn(sqlutils.RowIdxFn, sqlutils.RowModuloFn(10)), ) - tableDesc := desctestutils.TestingGetPublicTableDescriptor(kvDB, keys.SystemSQLCodec, "test", "t") + tableDesc := desctestutils.TestingGetPublicTableDescriptor(kvDB, s.Codec(), "test", "t") r := sqlutils.MakeSQLRunner(sqlDB) @@ -153,10 +158,10 @@ func TestScatterResponse(t *testing.T) { } var expectedKey roachpb.Key if i == 0 { - expectedKey = keys.SystemSQLCodec.TablePrefix(uint32(tableDesc.GetID())) + expectedKey = s.Codec().TablePrefix(uint32(tableDesc.GetID())) } else { var err error - expectedKey, err = randgen.TestingMakePrimaryIndexKey(tableDesc, i*10) + expectedKey, err = randgen.TestingMakePrimaryIndexKeyForTenant(tableDesc, s.Codec(), i*10) if err != nil { t.Fatal(err) }