Skip to content

Commit a682c1e

Browse files
rohit-nayak-psrohit-nayak-ps
authored
Revert "[release-16.0] MoveTables: add flag to specify that routing rules should not be created when a movetables workflow is created" (vitessio#13899)
Signed-off-by: Rohit Nayak <[email protected]>
1 parent d4e5cc7 commit a682c1e

File tree

4 files changed

+36
-74
lines changed

4 files changed

+36
-74
lines changed

‎go/vt/vtctl/vtctl.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ var commands = []commandGroup{
467467
{
468468
name: "MoveTables",
469469
method: commandMoveTables,
470-
params: "[--source=<sourceKs>] [--tables=<tableSpecs>] [--cells=<cells>] [--tablet_types=<source_tablet_types>] [--all] [--exclude=<tables>] [--auto_start] [--stop_after_copy] [--defer-secondary-keys] [--on-ddl=<ddl-action>] [--source_shards=<source_shards>] [--no-routing-rules] <action> 'action must be one of the following: Create, Complete, Cancel, SwitchTraffic, ReverseTrafffic, Show, or Progress' <targetKs.workflow>",
470+
params: "[--source=<sourceKs>] [--tables=<tableSpecs>] [--cells=<cells>] [--tablet_types=<source_tablet_types>] [--all] [--exclude=<tables>] [--auto_start] [--stop_after_copy] [--defer-secondary-keys] [--on-ddl=<ddl-action>] [--source_shards=<source_shards>] <action> 'action must be one of the following: Create, Complete, Cancel, SwitchTraffic, ReverseTrafffic, Show, or Progress' <targetKs.workflow>",
471471
help: `Move table(s) to another keyspace, table_specs is a list of tables or the tables section of the vschema for the target keyspace. Example: '{"t1":{"column_vindexes": [{"column": "id1", "name": "hash"}]}, "t2":{"column_vindexes": [{"column": "id2", "name": "hash"}]}}'. In the case of an unsharded target keyspace the vschema for each table may be empty. Example: '{"t1":{}, "t2":{}}'.`,
472472
},
473473
{
@@ -2125,7 +2125,6 @@ func commandVRWorkflow(ctx context.Context, wr *wrangler.Wrangler, subFlags *pfl
21252125

21262126
// MoveTables-only params
21272127
renameTables := subFlags.Bool("rename_tables", false, "MoveTables only. Rename tables instead of dropping them. --rename_tables is only supported for Complete.")
2128-
noRoutingRules := subFlags.Bool("no-routing-rules", false, "(Advanced) MoveTables Create only. Do not create routing rules while creating the workflow. See the reference documentation for limitations if you use this flag.")
21292128

21302129
// MoveTables and Reshard params
21312130
sourceShards := subFlags.String("source_shards", "", "Source shards")
@@ -2263,7 +2262,6 @@ func commandVRWorkflow(ctx context.Context, wr *wrangler.Wrangler, subFlags *pfl
22632262
vrwp.ExternalCluster = externalClusterName
22642263
vrwp.SourceTimeZone = *sourceTimeZone
22652264
vrwp.DropForeignKeys = *dropForeignKeys
2266-
vrwp.NoRoutingRules = *noRoutingRules
22672265
if *sourceShards != "" {
22682266
vrwp.SourceShards = strings.Split(*sourceShards, ",")
22692267
}

‎go/vt/wrangler/materializer.go

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,7 @@ func shouldInclude(table string, excludes []string) bool {
122122
// MoveTables initiates moving table(s) over to another keyspace
123123
func (wr *Wrangler) MoveTables(ctx context.Context, workflow, sourceKeyspace, targetKeyspace, tableSpecs,
124124
cell, tabletTypes string, allTables bool, excludeTables string, autoStart, stopAfterCopy bool,
125-
externalCluster string, dropForeignKeys, deferSecondaryKeys bool, sourceTimeZone, onDDL string,
126-
sourceShards []string, noRoutingRules bool) error {
125+
externalCluster string, dropForeignKeys, deferSecondaryKeys bool, sourceTimeZone, onDDL string, sourceShards []string) error {
127126
//FIXME validate tableSpecs, allTables, excludeTables
128127
var tables []string
129128
var externalTopo *topo.Server
@@ -207,36 +206,32 @@ func (wr *Wrangler) MoveTables(ctx context.Context, workflow, sourceKeyspace, ta
207206
}
208207
}
209208
if externalTopo == nil {
210-
if noRoutingRules {
211-
log.Warningf("Found --no-routing-rules flag, not creating routing rules for workflow %s.%s", targetKeyspace, workflow)
212-
} else {
213-
// Save routing rules before vschema. If we save vschema first, and routing rules
214-
// fails to save, we may generate duplicate table errors.
215-
rules, err := topotools.GetRoutingRules(ctx, wr.ts)
216-
if err != nil {
217-
return err
218-
}
219-
for _, table := range tables {
220-
toSource := []string{sourceKeyspace + "." + table}
221-
rules[table] = toSource
222-
rules[table+"@replica"] = toSource
223-
rules[table+"@rdonly"] = toSource
224-
rules[targetKeyspace+"."+table] = toSource
225-
rules[targetKeyspace+"."+table+"@replica"] = toSource
226-
rules[targetKeyspace+"."+table+"@rdonly"] = toSource
227-
rules[targetKeyspace+"."+table] = toSource
228-
rules[sourceKeyspace+"."+table+"@replica"] = toSource
229-
rules[sourceKeyspace+"."+table+"@rdonly"] = toSource
230-
}
231-
if err := topotools.SaveRoutingRules(ctx, wr.ts, rules); err != nil {
232-
return err
233-
}
209+
// Save routing rules before vschema. If we save vschema first, and routing rules
210+
// fails to save, we may generate duplicate table errors.
211+
rules, err := topotools.GetRoutingRules(ctx, wr.ts)
212+
if err != nil {
213+
return err
214+
}
215+
for _, table := range tables {
216+
toSource := []string{sourceKeyspace + "." + table}
217+
rules[table] = toSource
218+
rules[table+"@replica"] = toSource
219+
rules[table+"@rdonly"] = toSource
220+
rules[targetKeyspace+"."+table] = toSource
221+
rules[targetKeyspace+"."+table+"@replica"] = toSource
222+
rules[targetKeyspace+"."+table+"@rdonly"] = toSource
223+
rules[targetKeyspace+"."+table] = toSource
224+
rules[sourceKeyspace+"."+table+"@replica"] = toSource
225+
rules[sourceKeyspace+"."+table+"@rdonly"] = toSource
226+
}
227+
if err := topotools.SaveRoutingRules(ctx, wr.ts, rules); err != nil {
228+
return err
229+
}
234230

235-
if vschema != nil {
236-
// We added to the vschema.
237-
if err := wr.ts.SaveVSchema(ctx, targetKeyspace, vschema); err != nil {
238-
return err
239-
}
231+
if vschema != nil {
232+
// We added to the vschema.
233+
if err := wr.ts.SaveVSchema(ctx, targetKeyspace, vschema); err != nil {
234+
return err
240235
}
241236
}
242237
}

‎go/vt/wrangler/materializer_test.go

Lines changed: 8 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -46,34 +46,6 @@ const mzCheckJournal = "/select val from _vt.resharding_journal where id="
4646

4747
var defaultOnDDL = binlogdatapb.OnDDLAction_name[int32(binlogdatapb.OnDDLAction_IGNORE)]
4848

49-
// TestMoveTablesNoRoutingRules confirms that MoveTables does not create routing rules if --no-routing-rules is specified.
50-
func TestMoveTablesNoRoutingRules(t *testing.T) {
51-
ms := &vtctldatapb.MaterializeSettings{
52-
Workflow: "workflow",
53-
SourceKeyspace: "sourceks",
54-
TargetKeyspace: "targetks",
55-
TableSettings: []*vtctldatapb.TableMaterializeSettings{{
56-
TargetTable: "t1",
57-
SourceExpression: "select * from t1",
58-
}},
59-
}
60-
env := newTestMaterializerEnv(t, ms, []string{"0"}, []string{"0"})
61-
defer env.close()
62-
63-
env.tmc.expectVRQuery(100, mzCheckJournal, &sqltypes.Result{})
64-
env.tmc.expectVRQuery(200, mzSelectFrozenQuery, &sqltypes.Result{})
65-
env.tmc.expectVRQuery(200, insertPrefix, &sqltypes.Result{})
66-
env.tmc.expectVRQuery(200, mzSelectIDQuery, &sqltypes.Result{})
67-
env.tmc.expectVRQuery(200, mzUpdateQuery, &sqltypes.Result{})
68-
69-
ctx := context.Background()
70-
err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, true)
71-
require.NoError(t, err)
72-
rr, err := env.wr.ts.GetRoutingRules(ctx)
73-
require.NoError(t, err)
74-
require.Equal(t, 0, len(rr.Rules))
75-
}
76-
7749
func TestMigrateTables(t *testing.T) {
7850
ms := &vtctldatapb.MaterializeSettings{
7951
Workflow: "workflow",
@@ -94,7 +66,7 @@ func TestMigrateTables(t *testing.T) {
9466
env.tmc.expectVRQuery(200, mzUpdateQuery, &sqltypes.Result{})
9567

9668
ctx := context.Background()
97-
err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false)
69+
err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil)
9870
require.NoError(t, err)
9971
vschema, err := env.wr.ts.GetSrvVSchema(ctx, env.cell)
10072
require.NoError(t, err)
@@ -135,11 +107,11 @@ func TestMissingTables(t *testing.T) {
135107
env.tmc.expectVRQuery(200, mzUpdateQuery, &sqltypes.Result{})
136108

137109
ctx := context.Background()
138-
err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1,tyt", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false)
110+
err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1,tyt", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil)
139111
require.EqualError(t, err, "table(s) not found in source keyspace sourceks: tyt")
140-
err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1,tyt,t2,txt", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false)
112+
err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1,tyt,t2,txt", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil)
141113
require.EqualError(t, err, "table(s) not found in source keyspace sourceks: tyt,txt")
142-
err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false)
114+
err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil)
143115
require.NoError(t, err)
144116
}
145117

@@ -195,7 +167,7 @@ func TestMoveTablesAllAndExclude(t *testing.T) {
195167
env.tmc.expectVRQuery(200, insertPrefix, &sqltypes.Result{})
196168
env.tmc.expectVRQuery(200, mzSelectIDQuery, &sqltypes.Result{})
197169
env.tmc.expectVRQuery(200, mzUpdateQuery, &sqltypes.Result{})
198-
err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "", "", "", tcase.allTables, tcase.excludeTables, true, false, "", false, false, "", defaultOnDDL, nil, false)
170+
err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "", "", "", tcase.allTables, tcase.excludeTables, true, false, "", false, false, "", defaultOnDDL, nil)
199171
require.NoError(t, err)
200172
require.EqualValues(t, tcase.want, targetTables(env))
201173
})
@@ -229,7 +201,7 @@ func TestMoveTablesStopFlags(t *testing.T) {
229201
env.tmc.expectVRQuery(200, mzSelectIDQuery, &sqltypes.Result{})
230202
// -auto_start=false is tested by NOT expecting the update query which sets state to RUNNING
231203
err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "",
232-
"", false, "", false, true, "", false, false, "", defaultOnDDL, nil, false)
204+
"", false, "", false, true, "", false, false, "", defaultOnDDL, nil)
233205
require.NoError(t, err)
234206
env.tmc.verifyQueries(t)
235207
})
@@ -255,7 +227,7 @@ func TestMigrateVSchema(t *testing.T) {
255227
env.tmc.expectVRQuery(200, mzUpdateQuery, &sqltypes.Result{})
256228

257229
ctx := context.Background()
258-
err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", `{"t1":{}}`, "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false)
230+
err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", `{"t1":{}}`, "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil)
259231
require.NoError(t, err)
260232
vschema, err := env.wr.ts.GetSrvVSchema(ctx, env.cell)
261233
require.NoError(t, err)
@@ -2856,7 +2828,7 @@ func TestMoveTablesDDLFlag(t *testing.T) {
28562828
env.tmc.expectVRQuery(200, mzUpdateQuery, &sqltypes.Result{})
28572829

28582830
err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "",
2859-
"", false, "", false, true, "", false, false, "", onDDLAction, nil, false)
2831+
"", false, "", false, true, "", false, false, "", onDDLAction, nil)
28602832
require.NoError(t, err)
28612833
})
28622834
}

‎go/vt/wrangler/workflow.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,6 @@ type VReplicationWorkflowParams struct {
7171

7272
// Migrate specific
7373
ExternalCluster string
74-
75-
// MoveTables only
76-
NoRoutingRules bool
7774
}
7875

7976
// VReplicationWorkflow stores various internal objects for a workflow
@@ -436,7 +433,7 @@ func (vrw *VReplicationWorkflow) initMoveTables() error {
436433
return vrw.wr.MoveTables(vrw.ctx, vrw.params.Workflow, vrw.params.SourceKeyspace, vrw.params.TargetKeyspace,
437434
vrw.params.Tables, vrw.params.Cells, vrw.params.TabletTypes, vrw.params.AllTables, vrw.params.ExcludeTables,
438435
vrw.params.AutoStart, vrw.params.StopAfterCopy, vrw.params.ExternalCluster, vrw.params.DropForeignKeys,
439-
vrw.params.DeferSecondaryKeys, vrw.params.SourceTimeZone, vrw.params.OnDDL, vrw.params.SourceShards, vrw.params.NoRoutingRules)
436+
vrw.params.DeferSecondaryKeys, vrw.params.SourceTimeZone, vrw.params.OnDDL, vrw.params.SourceShards)
440437
}
441438

442439
func (vrw *VReplicationWorkflow) initReshard() error {

0 commit comments

Comments
 (0)