Skip to content

Commit 53a7791

Browse files
tanjinxClaude
tanjinx
and
Claude
committed
remove scatter-update-limit-passthru flag, enable by default
The behavior is now always-on: non-scatter multi-shard DML with LIMIT pushes the LIMIT to each shard directly. Scatter DML with LIMIT still uses DMLWithInput as before. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Tanjin Xu <tanjin.xu@slack-corp.com>
1 parent 843abb7 commit 53a7791

10 files changed

Lines changed: 71 additions & 218 deletions

File tree

‎go/test/vschemawrapper/vschema_wrapper.go‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,18 +46,17 @@ var _ plancontext.VSchema = (*VSchemaWrapper)(nil)
4646
// VSchemaWrapper is a wrapper around VSchema that implements the ContextVSchema interface.
4747
// It is used in tests to provide a VSchema implementation.
4848
type VSchemaWrapper struct {
49-
Vcursor *econtext.VCursorImpl
50-
V *vindexes.VSchema
51-
Keyspace *vindexes.Keyspace
52-
TabletType_ topodatapb.TabletType
53-
Dest key.ShardDestination
54-
SysVarEnabled bool
55-
ForeignKeyChecksState *bool
56-
Version plancontext.PlannerVersion
57-
EnableViews bool
58-
EnableScatterUpdateLimitPassthru bool
59-
TestBuilder func(query string, vschema plancontext.VSchema, keyspace string) (*engine.Plan, error)
60-
Env *vtenv.Environment
49+
Vcursor *econtext.VCursorImpl
50+
V *vindexes.VSchema
51+
Keyspace *vindexes.Keyspace
52+
TabletType_ topodatapb.TabletType
53+
Dest key.ShardDestination
54+
SysVarEnabled bool
55+
ForeignKeyChecksState *bool
56+
Version plancontext.PlannerVersion
57+
EnableViews bool
58+
TestBuilder func(query string, vschema plancontext.VSchema, keyspace string) (*engine.Plan, error)
59+
Env *vtenv.Environment
6160
}
6261

6362
func NewVschemaWrapper(
@@ -133,10 +132,6 @@ func (vw *VSchemaWrapper) IsShardRoutingEnabled() bool {
133132
return false
134133
}
135134

136-
func (vw *VSchemaWrapper) IsScatterUpdateLimitPassthruEnabled() bool {
137-
return vw.EnableScatterUpdateLimitPassthru
138-
}
139-
140135
func (vw *VSchemaWrapper) GetVSchema() *vindexes.VSchema {
141136
return vw.V
142137
}

‎go/vt/vtgate/executor.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1541,12 +1541,11 @@ func (e *Executor) initVConfig(warnOnShardedOnly bool, pv plancontext.PlannerVer
15411541
QueryTimeout: queryTimeout,
15421542
MaxMemoryRows: maxMemoryRows,
15431543

1544-
SetVarEnabled: sysVarSetEnabled,
1545-
EnableViews: enableViews,
1546-
ForeignKeyMode: fkMode(foreignKeyMode),
1547-
EnableShardRouting: enableShardRouting,
1548-
ScatterUpdateLimitPassthru: scatterUpdateLimitPassthru,
1549-
WarnShardedOnly: warnOnShardedOnly,
1544+
SetVarEnabled: sysVarSetEnabled,
1545+
EnableViews: enableViews,
1546+
ForeignKeyMode: fkMode(foreignKeyMode),
1547+
EnableShardRouting: enableShardRouting,
1548+
WarnShardedOnly: warnOnShardedOnly,
15501549

15511550
DBDDLPlugin: dbDDLPlugin,
15521551

‎go/vt/vtgate/executorcontext/vcursor_impl.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,6 @@ type (
8989
WarnShardedOnly bool
9090
PlannerVersion plancontext.PlannerVersion
9191

92-
ScatterUpdateLimitPassthru bool
93-
9492
WarmingReadsPercent int
9593
WarmingReadsTimeout time.Duration
9694
WarmingReadsChannel chan bool
@@ -401,10 +399,6 @@ func (vc *VCursorImpl) IsShardRoutingEnabled() bool {
401399
return vc.config.EnableShardRouting
402400
}
403401

404-
func (vc *VCursorImpl) IsScatterUpdateLimitPassthruEnabled() bool {
405-
return vc.config.ScatterUpdateLimitPassthru
406-
}
407-
408402
func (vc *VCursorImpl) ReadTransaction(ctx context.Context, transactionID string) (*querypb.TransactionMetadata, error) {
409403
return vc.executor.ReadTransaction(ctx, transactionID)
410404
}

‎go/vt/vtgate/planbuilder/operators/query_planning.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -525,11 +525,7 @@ func tryPushingDownLimitInRoute(ctx *plancontext.PlanningContext, in *Limit, src
525525
}
526526

527527
if sqlparser.IsDMLStatement(ctx.Statement) {
528-
// When scatter-update-limit-passthru is enabled, for non-scatter multi-shard DML
529-
// routes (e.g., IN clause on vindex), push the LIMIT under the route so each shard
530-
// gets the LIMIT directly. This restores v19 behavior where LIMIT was pushed to
531-
// individual shards rather than requiring DMLWithInput and schema tracking.
532-
if ctx.VSchema.IsScatterUpdateLimitPassthruEnabled() && src.Routing.OpCode() != engine.Scatter {
528+
if src.Routing.OpCode() != engine.Scatter {
533529
return Swap(in, src, "push limit under route")
534530
}
535531
return setUpperLimit(in)

‎go/vt/vtgate/planbuilder/plan_test.go‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -122,20 +122,6 @@ func (s *planTestSuite) TestPlan() {
122122
s.testFile("cte_cases.json", vw, false)
123123
}
124124

125-
// TestScatterUpdateLimitPassthru tests planning when scatter-update-limit-passthru is enabled.
126-
func (s *planTestSuite) TestScatterUpdateLimitPassthru() {
127-
env := vtenv.NewTestEnv()
128-
vschema := loadSchema(s.T(), "vschemas/schema.json", true)
129-
vw, err := vschemawrapper.NewVschemaWrapper(env, vschema, TestBuilder)
130-
require.NoError(s.T(), err)
131-
132-
vw.EnableScatterUpdateLimitPassthru = true
133-
134-
s.addPKs(vschema, "user", []string{"user", "music"})
135-
s.addPKsProvided(vschema, "user", []string{"user_extra"}, []string{"id", "user_id"})
136-
s.testFile("scatter_update_limit_passthru_cases.json", vw, false)
137-
}
138-
139125
// TestForeignKeyPlanning tests the planning of foreign keys in a managed mode by Vitess.
140126
func (s *planTestSuite) TestForeignKeyPlanning() {
141127
env := vtenv.NewTestEnv()

‎go/vt/vtgate/planbuilder/plancontext/planning_context_test.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -336,10 +336,6 @@ func (v *vschema) IsShardRoutingEnabled() bool {
336336
panic("implement me")
337337
}
338338

339-
func (v *vschema) IsScatterUpdateLimitPassthruEnabled() bool {
340-
return false
341-
}
342-
343339
func (v *vschema) IsViewsEnabled() bool {
344340
// TODO implement me
345341
panic("implement me")

‎go/vt/vtgate/planbuilder/plancontext/vschema.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,6 @@ type VSchema interface {
8080
// IsShardRoutingEnabled returns true if partial shard routing is enabled
8181
IsShardRoutingEnabled() bool
8282

83-
// IsScatterUpdateLimitPassthruEnabled returns true if non-scatter multi-shard DML
84-
// with LIMIT should push the LIMIT to each shard instead of using DMLWithInput.
85-
IsScatterUpdateLimitPassthruEnabled() bool
86-
8783
// IsViewsEnabled returns true if Vitess manages the views.
8884
IsViewsEnabled() bool
8985

‎go/vt/vtgate/planbuilder/testdata/dml_cases.json‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7430,5 +7430,57 @@
74307430
]
74317431
},
74327432
"skip_e2e": true
7433+
},
7434+
{
7435+
"comment": "update with IN clause on vindex and limit pushes limit to each shard",
7436+
"query": "update user_extra set val = 1 where user_id in (1, 2) limit 5",
7437+
"plan": {
7438+
"Type": "MultiShard",
7439+
"QueryType": "UPDATE",
7440+
"Original": "update user_extra set val = 1 where user_id in (1, 2) limit 5",
7441+
"Instructions": {
7442+
"OperatorType": "Update",
7443+
"Variant": "IN",
7444+
"Keyspace": {
7445+
"Name": "user",
7446+
"Sharded": true
7447+
},
7448+
"Query": "update user_extra set val = 1 where user_id in ::__vals limit 5",
7449+
"Table": "user_extra",
7450+
"Values": [
7451+
"(1, 2)"
7452+
],
7453+
"Vindex": "user_index"
7454+
},
7455+
"TablesUsed": [
7456+
"user.user_extra"
7457+
]
7458+
}
7459+
},
7460+
{
7461+
"comment": "delete with IN clause on vindex and limit pushes limit to each shard",
7462+
"query": "delete from user_extra where user_id in (1, 2) limit 5",
7463+
"plan": {
7464+
"Type": "MultiShard",
7465+
"QueryType": "DELETE",
7466+
"Original": "delete from user_extra where user_id in (1, 2) limit 5",
7467+
"Instructions": {
7468+
"OperatorType": "Delete",
7469+
"Variant": "IN",
7470+
"Keyspace": {
7471+
"Name": "user",
7472+
"Sharded": true
7473+
},
7474+
"Query": "delete from user_extra where user_id in ::__vals limit 5",
7475+
"Table": "user_extra",
7476+
"Values": [
7477+
"(1, 2)"
7478+
],
7479+
"Vindex": "user_index"
7480+
},
7481+
"TablesUsed": [
7482+
"user.user_extra"
7483+
]
7484+
}
74337485
}
74347486
]

‎go/vt/vtgate/planbuilder/testdata/scatter_update_limit_passthru_cases.json‎

Lines changed: 0 additions & 159 deletions
This file was deleted.

‎go/vt/vtgate/vtgate.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,8 @@ var (
8080
maxPayloadSize int
8181
warnPayloadSize int
8282

83-
noScatter bool
84-
enableShardRouting bool
85-
scatterUpdateLimitPassthru bool
83+
noScatter bool
84+
enableShardRouting bool
8685

8786
// healthCheckRetryDelay is the time to wait before retrying healthcheck
8887
healthCheckRetryDelay = 2 * time.Millisecond
@@ -183,7 +182,6 @@ func registerFlags(fs *pflag.FlagSet) {
183182
fs.StringVar(&dbDDLPlugin, "dbddl_plugin", dbDDLPlugin, "controls how to handle CREATE/DROP DATABASE. use it if you are using your own database provisioning service")
184183
fs.BoolVar(&noScatter, "no_scatter", noScatter, "when set to true, the planner will fail instead of producing a plan that includes scatter queries")
185184
fs.BoolVar(&enableShardRouting, "enable-partial-keyspace-migration", enableShardRouting, "(Experimental) Follow shard routing rules: enable only while migrating a keyspace shard by shard. See documentation on Partial MoveTables for more. (default false)")
186-
fs.BoolVar(&scatterUpdateLimitPassthru, "scatter-update-limit-passthru", scatterUpdateLimitPassthru, "When enabled, multi-shard UPDATE/DELETE with LIMIT on non-scatter routes (e.g. IN clause on vindex) will push LIMIT to each shard instead of requiring schema tracking for DMLWithInput.")
187185
fs.DurationVar(&healthCheckRetryDelay, "healthcheck_retry_delay", healthCheckRetryDelay, "health check retry delay")
188186
fs.DurationVar(&healthCheckTimeout, "healthcheck_timeout", healthCheckTimeout, "the health check timeout period")
189187
fs.IntVar(&maxPayloadSize, "max_payload_size", maxPayloadSize, "The threshold for query payloads in bytes. A payload greater than this threshold will result in a failure to handle the query.")

0 commit comments

Comments
 (0)