From be112f25595cde3842eb13ed047532421b26c0da Mon Sep 17 00:00:00 2001 From: Steven Danna Date: Fri, 15 Dec 2023 17:06:35 +0000 Subject: [PATCH] sql,*: remove DistributionTypeSystemTenantOnly Uses of DistributionTypeSystemTenantOnly represent places where UA/MT performance may not match single-tenant performance. This replaces all uses of DistributionTypeSystemTenantOnly with DistributionTypeAlways. Epic: none Fixes #116534 --- pkg/ccl/streamingccl/streamproducer/stream_lifetime.go | 2 +- pkg/sql/backfill.go | 4 ++-- pkg/sql/create_stats.go | 2 +- pkg/sql/distsql_physical_planner.go | 5 +---- pkg/sql/distsql_physical_planner_test.go | 4 ++-- pkg/sql/distsql_plan_ctas.go | 2 +- pkg/sql/distsql_running_test.go | 2 +- pkg/sql/distsql_spec_exec_factory.go | 2 +- pkg/sql/index_backfiller.go | 2 +- pkg/sql/mvcc_backfiller.go | 2 +- pkg/sql/testutils.go | 2 +- 11 files changed, 13 insertions(+), 16 deletions(-) diff --git a/pkg/ccl/streamingccl/streamproducer/stream_lifetime.go b/pkg/ccl/streamingccl/streamproducer/stream_lifetime.go index fbc661ba0c5d..85b57c7d672c 100644 --- a/pkg/ccl/streamingccl/streamproducer/stream_lifetime.go +++ b/pkg/ccl/streamingccl/streamproducer/stream_lifetime.go @@ -295,7 +295,7 @@ func buildReplicationStreamSpec( // Partition the spans with SQLPlanner dsp := jobExecCtx.DistSQLPlanner() planCtx := dsp.NewPlanningCtx(ctx, jobExecCtx.ExtendedEvalContext(), - nil /* planner */, nil /* txn */, sql.DistributionTypeSystemTenantOnly) + nil /* planner */, nil /* txn */, sql.DistributionTypeAlways) spanPartitions, err := dsp.PartitionSpans(ctx, planCtx, targetSpans) if err != nil { diff --git a/pkg/sql/backfill.go b/pkg/sql/backfill.go index 027a10375e52..efd265dd70c2 100644 --- a/pkg/sql/backfill.go +++ b/pkg/sql/backfill.go @@ -1045,7 +1045,7 @@ func (sc *SchemaChanger) distIndexBackfill( sd := NewInternalSessionData(ctx, sc.execCfg.Settings, "dist-index-backfill") evalCtx = createSchemaChangeEvalCtx(ctx, sc.execCfg, sd, txn.KV().ReadTimestamp(), txn.Descriptors()) planCtx = sc.distSQLPlanner.NewPlanningCtx(ctx, &evalCtx, nil, /* planner */ - txn.KV(), DistributionTypeSystemTenantOnly) + txn.KV(), DistributionTypeAlways) indexBatchSize := indexBackfillBatchSize.Get(&sc.execCfg.Settings.SV) chunkSize := sc.getChunkSize(indexBatchSize) spec, err := initIndexBackfillerSpec(*tableDesc.TableDesc(), writeAsOf, readAsOf, writeAtRequestTimestamp, chunkSize, addedIndexes) @@ -1352,7 +1352,7 @@ func (sc *SchemaChanger) distColumnBackfill( defer recv.Release() planCtx := sc.distSQLPlanner.NewPlanningCtx(ctx, &evalCtx, nil /* planner */, txn.KV(), - DistributionTypeSystemTenantOnly) + DistributionTypeAlways) spec, err := initColumnBackfillerSpec(tableDesc, duration, chunkSize, backfillUpdateChunkSizeThresholdBytes, readAsOf) if err != nil { return err diff --git a/pkg/sql/create_stats.go b/pkg/sql/create_stats.go index 13897b598110..ba626c6642be 100644 --- a/pkg/sql/create_stats.go +++ b/pkg/sql/create_stats.go @@ -665,7 +665,7 @@ func (r *createStatsResumer) Resume(ctx context.Context, execCtx interface{}) er } planCtx := dsp.NewPlanningCtx(ctx, evalCtx, nil /* planner */, txn.KV(), - DistributionTypeSystemTenantOnly) + DistributionTypeAlways) // CREATE STATS flow doesn't produce any rows and only emits the // metadata, so we can use a nil rowContainerHelper. resultWriter := NewRowResultWriter(nil /* rowContainer */) diff --git a/pkg/sql/distsql_physical_planner.go b/pkg/sql/distsql_physical_planner.go index a023ddb5999d..d16cb79aaff5 100644 --- a/pkg/sql/distsql_physical_planner.go +++ b/pkg/sql/distsql_physical_planner.go @@ -149,9 +149,6 @@ const ( // DistributionTypeAlways distributes a plan across multiple instances whether // it is a system tenant or non-system tenant. DistributionTypeAlways - // DistributionTypeSystemTenantOnly only distributes a plan if it is for a - // system tenant. Plans on non-system tenants are not distributed. - DistributionTypeSystemTenantOnly ) // ReplicaOraclePolicy controls which policy the physical planner uses to choose @@ -4840,7 +4837,7 @@ func (dsp *DistSQLPlanner) NewPlanningCtxWithOracle( oracle replicaoracle.Oracle, localityFiler roachpb.Locality, ) *PlanningCtx { - distribute := distributionType == DistributionTypeAlways || (distributionType == DistributionTypeSystemTenantOnly && evalCtx.Codec.ForSystemTenant()) + distribute := distributionType == DistributionTypeAlways infra := physicalplan.NewPhysicalInfrastructure(uuid.FastMakeV4(), dsp.gatewaySQLInstanceID) planCtx := &PlanningCtx{ ExtendedEvalCtx: evalCtx, diff --git a/pkg/sql/distsql_physical_planner_test.go b/pkg/sql/distsql_physical_planner_test.go index ea5b601120d5..5cd072e7523d 100644 --- a/pkg/sql/distsql_physical_planner_test.go +++ b/pkg/sql/distsql_physical_planner_test.go @@ -1198,7 +1198,7 @@ func TestPartitionSpans(t *testing.T) { } planCtx := dsp.NewPlanningCtxWithOracle(ctx, &extendedEvalContext{ Context: *evalCtx, - }, nil, nil, DistributionTypeSystemTenantOnly, physicalplan.DefaultReplicaChooser, locFilter) + }, nil, nil, DistributionTypeAlways, physicalplan.DefaultReplicaChooser, locFilter) planCtx.spanPartitionState.testingOverrideRandomSelection = tc.partitionState.testingOverrideRandomSelection var spans []roachpb.Span for _, s := range tc.spans { @@ -1515,7 +1515,7 @@ func TestPartitionSpansSkipsNodesNotInGossip(t *testing.T) { ctx := context.Background() planCtx := dsp.NewPlanningCtx(ctx, &extendedEvalContext{ Context: eval.Context{Codec: keys.SystemSQLCodec}, - }, nil, nil, DistributionTypeSystemTenantOnly) + }, nil, nil, DistributionTypeAlways) partitions, err := dsp.PartitionSpans(ctx, planCtx, roachpb.Spans{span}) if err != nil { t.Fatal(err) diff --git a/pkg/sql/distsql_plan_ctas.go b/pkg/sql/distsql_plan_ctas.go index 4fdbfbe83ced..e361418fe082 100644 --- a/pkg/sql/distsql_plan_ctas.go +++ b/pkg/sql/distsql_plan_ctas.go @@ -33,7 +33,7 @@ func PlanAndRunCTAS( ) { distribute := DistributionType(DistributionTypeNone) if !isLocal { - distribute = DistributionTypeSystemTenantOnly + distribute = DistributionTypeAlways } planCtx := dsp.NewPlanningCtx(ctx, planner.ExtendedEvalContext(), planner, txn, distribute) diff --git a/pkg/sql/distsql_running_test.go b/pkg/sql/distsql_running_test.go index 742b92c31674..52b55c13ae3d 100644 --- a/pkg/sql/distsql_running_test.go +++ b/pkg/sql/distsql_running_test.go @@ -183,7 +183,7 @@ func TestDistSQLRunningInAbortedTxn(t *testing.T) { // the root txn meta to leaf txns. Local flows can start in aborted txns // because they just use the root txn. planCtx := execCfg.DistSQLPlanner.NewPlanningCtx(ctx, evalCtx, p, nil, - DistributionTypeSystemTenantOnly) + DistributionTypeAlways) planCtx.stmtType = recv.stmtType execCfg.DistSQLPlanner.PlanAndRun( diff --git a/pkg/sql/distsql_spec_exec_factory.go b/pkg/sql/distsql_spec_exec_factory.go index 544c36afdfde..73159a4e110d 100644 --- a/pkg/sql/distsql_spec_exec_factory.go +++ b/pkg/sql/distsql_spec_exec_factory.go @@ -75,7 +75,7 @@ func newDistSQLSpecExecFactory( } distribute := DistributionType(DistributionTypeNone) if e.planningMode != distSQLLocalOnlyPlanning { - distribute = DistributionTypeSystemTenantOnly + distribute = DistributionTypeAlways } evalCtx := p.ExtendedEvalContext() e.planCtx = e.dsp.NewPlanningCtx(ctx, evalCtx, e.planner, e.planner.txn, distribute) diff --git a/pkg/sql/index_backfiller.go b/pkg/sql/index_backfiller.go index df704dc7530d..1e2a752f4b01 100644 --- a/pkg/sql/index_backfiller.go +++ b/pkg/sql/index_backfiller.go @@ -193,7 +193,7 @@ func (ib *IndexBackfillPlanner) plan( sd := NewInternalSessionData(ctx, ib.execCfg.Settings, "plan-index-backfill") evalCtx = createSchemaChangeEvalCtx(ctx, ib.execCfg, sd, nowTimestamp, descriptors) planCtx = ib.execCfg.DistSQLPlanner.NewPlanningCtx(ctx, &evalCtx, - nil /* planner */, txn.KV(), DistributionTypeSystemTenantOnly) + nil /* planner */, txn.KV(), DistributionTypeAlways) // TODO(ajwerner): Adopt util.ConstantWithMetamorphicTestRange for the // batch size. Also plumb in a testing knob. chunkSize := indexBackfillBatchSize.Get(&ib.execCfg.Settings.SV) diff --git a/pkg/sql/mvcc_backfiller.go b/pkg/sql/mvcc_backfiller.go index 378a49dabca9..30d18edb46d3 100644 --- a/pkg/sql/mvcc_backfiller.go +++ b/pkg/sql/mvcc_backfiller.go @@ -139,7 +139,7 @@ func (im *IndexBackfillerMergePlanner) plan( sd := NewInternalSessionData(ctx, im.execCfg.Settings, "plan-index-backfill-merge") evalCtx = createSchemaChangeEvalCtx(ctx, im.execCfg, sd, txn.KV().ReadTimestamp(), descriptors) planCtx = im.execCfg.DistSQLPlanner.NewPlanningCtx(ctx, &evalCtx, nil /* planner */, txn.KV(), - DistributionTypeSystemTenantOnly) + DistributionTypeAlways) spec, err := initIndexBackfillMergerSpec(*tableDesc.TableDesc(), addedIndexes, temporaryIndexes, mergeTimestamp) if err != nil { diff --git a/pkg/sql/testutils.go b/pkg/sql/testutils.go index 1cf955428bc0..7cca07c55b72 100644 --- a/pkg/sql/testutils.go +++ b/pkg/sql/testutils.go @@ -153,7 +153,7 @@ func (dsp *DistSQLPlanner) Exec( distributionType := DistributionType(DistributionTypeNone) if distribute { - distributionType = DistributionTypeSystemTenantOnly + distributionType = DistributionTypeAlways } evalCtx := p.ExtendedEvalContext() planCtx := execCfg.DistSQLPlanner.NewPlanningCtx(ctx, evalCtx, p, p.txn,