Skip to content

Commit 295b6ae

Browse files
craig[bot]jordanlewis
andcommitted
Merge #34163
34163: sql: delete distsql modes 2.0-off and 2.0-auto r=jordanlewis a=jordanlewis This removes user ability to query using the local engine. Closes #34162. Release note (sql change): remove the 2.0-off and 2.0-auto distsql modes. All queries are now run via the newer, distsql engine; queries are still only distributed if appropriate. Co-authored-by: Jordan Lewis <[email protected]>
2 parents a56e1f2 + 5744dc9 commit 295b6ae

File tree

18 files changed

+688
-321
lines changed

18 files changed

+688
-321
lines changed

docs/generated/settings/settings.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
<tr><td><code>server.time_until_store_dead</code></td><td>duration</td><td><code>5m0s</code></td><td>the time after which if there is no new gossiped information about a store, it is considered dead</td></tr>
6363
<tr><td><code>server.web_session_timeout</code></td><td>duration</td><td><code>168h0m0s</code></td><td>the duration that a newly created web session will be valid</td></tr>
6464
<tr><td><code>sql.defaults.default_int_size</code></td><td>integer</td><td><code>8</code></td><td>the size, in bytes, of an INT type</td></tr>
65-
<tr><td><code>sql.defaults.distsql</code></td><td>enumeration</td><td><code>1</code></td><td>default distributed SQL execution mode [off = 0, auto = 1, on = 2, 2.0-off = 3, 2.0-auto = 4]</td></tr>
65+
<tr><td><code>sql.defaults.distsql</code></td><td>enumeration</td><td><code>1</code></td><td>default distributed SQL execution mode [off = 0, auto = 1, on = 2]</td></tr>
6666
<tr><td><code>sql.defaults.experimental_optimizer_mutations</code></td><td>boolean</td><td><code>false</code></td><td>default experimental_optimizer_mutations mode</td></tr>
6767
<tr><td><code>sql.defaults.experimental_vectorize</code></td><td>enumeration</td><td><code>0</code></td><td>default experimental_vectorize mode [off = 0, on = 1, always = 2]</td></tr>
6868
<tr><td><code>sql.defaults.optimizer</code></td><td>enumeration</td><td><code>1</code></td><td>default cost-based optimizer mode [off = 0, on = 1, local = 2]</td></tr>

pkg/sql/conn_executor_exec.go

Lines changed: 16 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -714,23 +714,17 @@ func (ex *connExecutor) execStmtInParallel(
714714

715715
planner.statsCollector.PhaseTimes()[plannerStartExecStmt] = timeutil.Now()
716716

717-
shouldUseDistSQL := shouldUseDistSQL(distributePlan, ex.sessionData.DistSQLMode)
718717
samplePlanDescription := ex.sampleLogicalPlanDescription(
719-
stmt, shouldUseDistSQL, false /* optimizerUsed */, planner)
718+
stmt, false /* optimizerUsed */, planner)
720719

721720
var flags planFlags
722-
if shouldUseDistSQL {
723-
if distributePlan {
724-
flags.Set(planFlagDistributed)
725-
} else {
726-
flags.Set(planFlagDistSQLLocal)
727-
}
728-
ex.sessionTracing.TraceExecStart(ctx, "distributed-parallel")
729-
err = ex.execWithDistSQLEngine(ctx, planner, stmt.AST.StatementType(), res, distributePlan)
721+
if distributePlan {
722+
flags.Set(planFlagDistributed)
730723
} else {
731-
ex.sessionTracing.TraceExecStart(ctx, "local-parallel")
732-
err = ex.execWithLocalEngine(ctx, planner, stmt.AST.StatementType(), res)
724+
flags.Set(planFlagDistSQLLocal)
733725
}
726+
ex.sessionTracing.TraceExecStart(ctx, "parallel")
727+
err = ex.execWithDistSQLEngine(ctx, planner, stmt.AST.StatementType(), res, distributePlan)
734728
ex.sessionTracing.TraceExecEnd(ctx, res.Err(), res.RowsAffected())
735729
planner.statsCollector.PhaseTimes()[plannerEndExecStmt] = timeutil.Now()
736730

@@ -851,21 +845,15 @@ func (ex *connExecutor) dispatchToExecutionEngine(
851845
queryMeta.isDistributed = distributePlan
852846
ex.mu.Unlock()
853847

854-
shouldUseDistSQL := shouldUseDistSQL(distributePlan, ex.sessionData.DistSQLMode)
855-
samplePlanDescription := ex.sampleLogicalPlanDescription(stmt, shouldUseDistSQL, flags.IsSet(planFlagOptUsed), planner)
848+
samplePlanDescription := ex.sampleLogicalPlanDescription(stmt, flags.IsSet(planFlagOptUsed), planner)
856849

857-
if shouldUseDistSQL {
858-
if distributePlan {
859-
flags.Set(planFlagDistributed)
860-
} else {
861-
flags.Set(planFlagDistSQLLocal)
862-
}
863-
ex.sessionTracing.TraceExecStart(ctx, "distributed")
864-
err = ex.execWithDistSQLEngine(ctx, planner, stmt.AST.StatementType(), res, distributePlan)
850+
if distributePlan {
851+
flags.Set(planFlagDistributed)
865852
} else {
866-
ex.sessionTracing.TraceExecStart(ctx, "local")
867-
err = ex.execWithLocalEngine(ctx, planner, stmt.AST.StatementType(), res)
853+
flags.Set(planFlagDistSQLLocal)
868854
}
855+
ex.sessionTracing.TraceExecStart(ctx, "distributed")
856+
err = ex.execWithDistSQLEngine(ctx, planner, stmt.AST.StatementType(), res, distributePlan)
869857
ex.sessionTracing.TraceExecEnd(ctx, res.Err(), res.RowsAffected())
870858
planner.statsCollector.PhaseTimes()[plannerEndExecStmt] = timeutil.Now()
871859
if err != nil {
@@ -920,13 +908,13 @@ func (ex *connExecutor) makeExecPlan(
920908
// sampleLogicalPlanDescription returns a serialized representation of a statement's logical plan.
921909
// The returned ExplainTreePlanNode will be nil if plan should not be sampled.
922910
func (ex *connExecutor) sampleLogicalPlanDescription(
923-
stmt Statement, useDistSQL bool, optimizerUsed bool, planner *planner,
911+
stmt Statement, optimizerUsed bool, planner *planner,
924912
) *roachpb.ExplainTreePlanNode {
925913
if !sampleLogicalPlans.Get(&ex.appStats.st.SV) {
926914
return nil
927915
}
928916

929-
if ex.saveLogicalPlanDescription(stmt, useDistSQL, optimizerUsed) {
917+
if ex.saveLogicalPlanDescription(stmt, optimizerUsed) {
930918
return planToTree(context.Background(), planner.curPlan)
931919
}
932920
return nil
@@ -935,10 +923,8 @@ func (ex *connExecutor) sampleLogicalPlanDescription(
935923
// saveLogicalPlanDescription returns if we should save this as a sample logical plan
936924
// for its corresponding fingerprint. We use `saveFingerprintPlanOnceEvery`
937925
// to assess how frequently to sample logical plans.
938-
func (ex *connExecutor) saveLogicalPlanDescription(
939-
stmt Statement, useDistSQL bool, optimizerUsed bool,
940-
) bool {
941-
stats := ex.appStats.getStatsForStmt(stmt, useDistSQL, optimizerUsed, nil, false /* createIfNonexistent */)
926+
func (ex *connExecutor) saveLogicalPlanDescription(stmt Statement, optimizerUsed bool) bool {
927+
stats := ex.appStats.getStatsForStmt(stmt, true /* distSQLUsed */, optimizerUsed, nil, false /* createIfNonexistent */)
942928
if stats == nil {
943929
// Save logical plan the first time we see new statement fingerprint.
944930
return true
@@ -977,68 +963,6 @@ func canFallbackFromOpt(err error, optMode sessiondata.OptimizerMode, stmt State
977963
return true
978964
}
979965

980-
// execWithLocalEngine runs a plan using the local (non-distributed) SQL
981-
// engine.
982-
// If an error is returned, the connection needs to stop processing queries.
983-
// Such errors are also written to res.
984-
// Query execution errors are written to res; they are not returned.
985-
func (ex *connExecutor) execWithLocalEngine(
986-
ctx context.Context, planner *planner, stmtType tree.StatementType, res RestrictedCommandResult,
987-
) error {
988-
// Create a BoundAccount to track the memory usage of each row.
989-
rowAcc := planner.extendedEvalCtx.Mon.MakeBoundAccount()
990-
planner.extendedEvalCtx.ActiveMemAcc = &rowAcc
991-
defer rowAcc.Close(ctx)
992-
993-
params := runParams{
994-
ctx: ctx,
995-
extendedEvalCtx: &planner.extendedEvalCtx,
996-
p: planner,
997-
}
998-
999-
if err := planner.curPlan.start(params); err != nil {
1000-
res.SetError(err)
1001-
return nil
1002-
}
1003-
1004-
switch stmtType {
1005-
case tree.RowsAffected:
1006-
count, err := countRowsAffected(params, planner.curPlan.plan)
1007-
if err != nil {
1008-
res.SetError(err)
1009-
return nil
1010-
}
1011-
res.IncrementRowsAffected(count)
1012-
return nil
1013-
case tree.Rows:
1014-
consumeCtx, cleanup := ex.sessionTracing.TraceExecConsume(ctx)
1015-
defer cleanup()
1016-
1017-
var commErr error
1018-
queryErr := ex.forEachRow(params, planner.curPlan.plan, func(values tree.Datums) error {
1019-
for _, val := range values {
1020-
if err := checkResultType(val.ResolvedType()); err != nil {
1021-
return err
1022-
}
1023-
}
1024-
ex.sessionTracing.TraceExecRowsResult(consumeCtx, values)
1025-
commErr = res.AddRow(consumeCtx, values)
1026-
return commErr
1027-
})
1028-
if commErr != nil {
1029-
res.SetError(commErr)
1030-
return commErr
1031-
}
1032-
if queryErr != nil {
1033-
res.SetError(queryErr)
1034-
}
1035-
return nil
1036-
default:
1037-
// Calling StartPlan is sufficient for other statement types.
1038-
return nil
1039-
}
1040-
}
1041-
1042966
// execWithDistSQLEngine converts a plan to a distributed SQL physical plan and
1043967
// runs it.
1044968
// If an error is returned, the connection needs to stop processing queries.
@@ -1090,31 +1014,6 @@ func (ex *connExecutor) execWithDistSQLEngine(
10901014
return recv.commErr
10911015
}
10921016

1093-
// forEachRow calls the provided closure for each successful call to
1094-
// planNode.Next with planNode.Values, making sure to properly track memory
1095-
// usage.
1096-
//
1097-
// f is not allowed to hold on to the row slice. It needs to make a copy if it
1098-
// want to use the memory later.
1099-
//
1100-
// Errors returned by this method are to be considered query errors. If the
1101-
// caller wants to handle some errors within the callback differently, it has to
1102-
// capture those itself.
1103-
func (ex *connExecutor) forEachRow(params runParams, p planNode, f func(tree.Datums) error) error {
1104-
next, err := p.Next(params)
1105-
for ; next; next, err = p.Next(params) {
1106-
// If we're tracking memory, clear the previous row's memory account.
1107-
if params.extendedEvalCtx.ActiveMemAcc != nil {
1108-
params.extendedEvalCtx.ActiveMemAcc.Clear(params.ctx)
1109-
}
1110-
1111-
if err := f(p.Values()); err != nil {
1112-
return err
1113-
}
1114-
}
1115-
return err
1116-
}
1117-
11181017
// execStmtInNoTxnState "executes" a statement when no transaction is in scope.
11191018
// For anything but BEGIN, this method doesn't actually execute the statement;
11201019
// it just returns an Event that will generate a transaction. The statement will

pkg/sql/exec_util.go

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -154,11 +154,9 @@ var DistSQLClusterExecMode = settings.RegisterEnumSetting(
154154
"default distributed SQL execution mode",
155155
"auto",
156156
map[int64]string{
157-
int64(sessiondata.DistSQLOff): "off",
158-
int64(sessiondata.DistSQLAuto): "auto",
159-
int64(sessiondata.DistSQLOn): "on",
160-
int64(sessiondata.DistSQL2Dot0Auto): "2.0-auto",
161-
int64(sessiondata.DistSQL2Dot0Off): "2.0-off",
157+
int64(sessiondata.DistSQLOff): "off",
158+
int64(sessiondata.DistSQLAuto): "auto",
159+
int64(sessiondata.DistSQLOn): "on",
162160
},
163161
)
164162

@@ -526,29 +524,13 @@ func countRowsAffected(params runParams, p planNode) (int, error) {
526524
return count, err
527525
}
528526

529-
// shouldUseDistSQL returns true if the combination of mode and distribution
530-
// requirement given by shouldDistributeGivenRecAndMode requires that the
531-
// distSQL engine should be used.
532-
// This is always true unless the mode is set to 2.0-auto or 2.0-off.
533-
// 2.0-auto causes the distribution recommendation to control whether distsql is
534-
// used at all, and 2.0-off causes distsql to never be used regardless of the
535-
// recommendation.
536-
func shouldUseDistSQL(distributePlan bool, mode sessiondata.DistSQLExecMode) bool {
537-
if mode == sessiondata.DistSQL2Dot0Off {
538-
return false
539-
} else if mode == sessiondata.DistSQL2Dot0Auto {
540-
return distributePlan
541-
}
542-
return true
543-
}
544-
545527
func shouldDistributeGivenRecAndMode(
546528
rec distRecommendation, mode sessiondata.DistSQLExecMode,
547529
) bool {
548530
switch mode {
549-
case sessiondata.DistSQLOff, sessiondata.DistSQL2Dot0Off:
531+
case sessiondata.DistSQLOff:
550532
return false
551-
case sessiondata.DistSQLAuto, sessiondata.DistSQL2Dot0Auto:
533+
case sessiondata.DistSQLAuto:
552534
return rec == shouldDistribute
553535
case sessiondata.DistSQLOn, sessiondata.DistSQLAlways:
554536
return rec != cannotDistribute

pkg/sql/explain_distsql.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,7 @@ type explainDistSQLRun struct {
6262
}
6363

6464
func (n *explainDistSQLNode) startExec(params runParams) error {
65-
if n.analyze &&
66-
(params.SessionData().DistSQLMode == sessiondata.DistSQLOff ||
67-
params.SessionData().DistSQLMode == sessiondata.DistSQL2Dot0Off) {
65+
if n.analyze && params.SessionData().DistSQLMode == sessiondata.DistSQLOff {
6866
return pgerror.NewErrorf(
6967
pgerror.CodeObjectNotInPrerequisiteStateError,
7068
"cannot run EXPLAIN ANALYZE while distsql is disabled",

pkg/sql/logictest/logic.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -418,25 +418,25 @@ type testClusterConfig struct {
418418
// If no configs are indicated, the default one is used (unless overridden
419419
// via -config).
420420
var logicTestConfigs = []testClusterConfig{
421-
{name: "local", numNodes: 1, overrideDistSQLMode: "2.0-off", overrideOptimizerMode: "off"},
421+
{name: "local", numNodes: 1, overrideDistSQLMode: "off", overrideOptimizerMode: "off"},
422422
{name: "[email protected]", numNodes: 1,
423-
overrideDistSQLMode: "2.0-off", overrideOptimizerMode: "off",
423+
overrideDistSQLMode: "off", overrideOptimizerMode: "off",
424424
bootstrapVersion: cluster.ClusterVersion{
425425
Version: cluster.VersionByKey(cluster.VersionBase),
426426
},
427427
serverVersion: roachpb.Version{Major: 1, Minor: 1},
428428
disableUpgrade: true,
429429
},
430-
{name: "local-opt", numNodes: 1, overrideDistSQLMode: "2.0-off", overrideOptimizerMode: "on"},
431-
{name: "local-parallel-stmts", numNodes: 1, parallelStmts: true, overrideDistSQLMode: "2.0-off", overrideOptimizerMode: "off"},
430+
{name: "local-opt", numNodes: 1, overrideDistSQLMode: "off", overrideOptimizerMode: "on"},
431+
{name: "local-parallel-stmts", numNodes: 1, parallelStmts: true, overrideDistSQLMode: "off", overrideOptimizerMode: "off"},
432432
{name: "local-vec", numNodes: 1, overrideOptimizerMode: "off", overrideExpVectorize: "on"},
433433
{name: "fakedist", numNodes: 3, useFakeSpanResolver: true, overrideDistSQLMode: "on", overrideOptimizerMode: "off"},
434434
{name: "fakedist-opt", numNodes: 3, useFakeSpanResolver: true, overrideDistSQLMode: "on", overrideOptimizerMode: "on"},
435435
{name: "fakedist-metadata", numNodes: 3, useFakeSpanResolver: true, overrideDistSQLMode: "on", overrideOptimizerMode: "off",
436436
distSQLMetadataTestEnabled: true, skipShort: true},
437437
{name: "fakedist-disk", numNodes: 3, useFakeSpanResolver: true, overrideDistSQLMode: "on", overrideOptimizerMode: "off",
438438
distSQLUseDisk: true, skipShort: true},
439-
{name: "5node-local", numNodes: 5, overrideDistSQLMode: "2.0-off", overrideOptimizerMode: "off"},
439+
{name: "5node-local", numNodes: 5, overrideDistSQLMode: "off", overrideOptimizerMode: "off"},
440440
{name: "5node-dist", numNodes: 5, overrideDistSQLMode: "on", overrideOptimizerMode: "off"},
441441
{name: "5node-dist-opt", numNodes: 5, overrideDistSQLMode: "on", overrideOptimizerMode: "on"},
442442
{name: "5node-dist-metadata", numNodes: 5, overrideDistSQLMode: "on", distSQLMetadataTestEnabled: true,

pkg/sql/logictest/testdata/logic_test/pg_catalog

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,7 +1338,7 @@ datestyle ISO, MDY NULL NULL NULL
13381338
default_int_size 8 NULL NULL NULL string
13391339
default_transaction_isolation serializable NULL NULL NULL string
13401340
default_transaction_read_only off NULL NULL NULL string
1341-
distsql 2.0-off NULL NULL NULL string
1341+
distsql off NULL NULL NULL string
13421342
experimental_enable_zigzag_join off NULL NULL NULL string
13431343
experimental_force_lookup_join off NULL NULL NULL string
13441344
experimental_force_split_at off NULL NULL NULL string
@@ -1383,7 +1383,7 @@ datestyle ISO, MDY NULL user NULL ISO, M
13831383
default_int_size 8 NULL user NULL 8 8
13841384
default_transaction_isolation serializable NULL user NULL default default
13851385
default_transaction_read_only off NULL user NULL off off
1386-
distsql 2.0-off NULL user NULL 2.0-off 2.0-off
1386+
distsql off NULL user NULL off off
13871387
experimental_enable_zigzag_join off NULL user NULL off off
13881388
experimental_force_lookup_join off NULL user NULL off off
13891389
experimental_force_split_at off NULL user NULL off off

pkg/sql/logictest/testdata/logic_test/prepare

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ DEALLOCATE ALL
181181
statement
182182
PREPARE x AS SELECT avg(column1) OVER (PARTITION BY column2) FROM (VALUES (1, 2), (3, 4))
183183

184-
query R
184+
query R rowsort
185185
EXECUTE x
186186
----
187187
1
@@ -190,7 +190,7 @@ EXECUTE x
190190
statement
191191
PREPARE y AS SELECT avg(a.column1) OVER (PARTITION BY a.column2) FROM (VALUES (1, 2), (3, 4)) a
192192

193-
query R
193+
query R rowsort
194194
EXECUTE y
195195
----
196196
1

pkg/sql/logictest/testdata/logic_test/set

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ query T colnames
101101
SHOW distsql
102102
----
103103
distsql
104-
2.0-off
104+
off
105105

106106
## Test that our no-op compatibility vars work
107107

@@ -165,23 +165,17 @@ SET distsql = on
165165
statement ok
166166
SET distsql = off
167167

168-
statement ok
169-
SET distsql = '2.0-off'
170-
171-
statement ok
172-
SET distsql = '2.0-auto'
173-
174168
statement error invalid value for parameter "distsql": "bogus"
175169
SET distsql = bogus
176170

177171
statement ok
178172
SET experimental_vectorize = on
179173

180174
statement ok
181-
SET experimental_vectorize = off
175+
SET experimental_vectorize = always
182176

183177
statement ok
184-
SET experimental_vectorize = always
178+
SET experimental_vectorize = off
185179

186180
statement error invalid value for parameter "experimental_vectorize": "bogus"
187181
SET experimental_vectorize = bogus

pkg/sql/logictest/testdata/logic_test/show_source

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ datestyle ISO, MDY
3535
default_int_size 8
3636
default_transaction_isolation serializable
3737
default_transaction_read_only off
38-
distsql 2.0-off
38+
distsql off
3939
experimental_enable_zigzag_join off
4040
experimental_force_lookup_join off
4141
experimental_force_split_at off
@@ -66,7 +66,7 @@ query I colnames
6666
SELECT * FROM [SHOW CLUSTER SETTING sql.defaults.distsql]
6767
----
6868
sql.defaults.distsql
69-
3
69+
0
7070

7171
query TTTT colnames
7272
SELECT * FROM [SHOW ALL CLUSTER SETTINGS] WHERE variable LIKE '%organization'

0 commit comments

Comments
 (0)