From fab56098f8a1a11d05ab49e50768202732f07387 Mon Sep 17 00:00:00 2001 From: Rohit Nayak Date: Thu, 31 Aug 2023 11:26:29 +0200 Subject: [PATCH] Revert d4e5cc777c172d4b204d65d711c8e35243de5207 Signed-off-by: Rohit Nayak --- go/vt/vtctl/vtctl.go | 4 +- go/vt/wrangler/materializer.go | 57 +++++++++++++---------------- go/vt/wrangler/materializer_test.go | 44 ++++------------------ go/vt/wrangler/workflow.go | 5 +-- 4 files changed, 36 insertions(+), 74 deletions(-) diff --git a/go/vt/vtctl/vtctl.go b/go/vt/vtctl/vtctl.go index 86a002bde9a..93628b4d0f5 100644 --- a/go/vt/vtctl/vtctl.go +++ b/go/vt/vtctl/vtctl.go @@ -467,7 +467,7 @@ var commands = []commandGroup{ { name: "MoveTables", method: commandMoveTables, - params: "[--source=] [--tables=] [--cells=] [--tablet_types=] [--all] [--exclude=] [--auto_start] [--stop_after_copy] [--defer-secondary-keys] [--on-ddl=] [--source_shards=] [--no-routing-rules] 'action must be one of the following: Create, Complete, Cancel, SwitchTraffic, ReverseTrafffic, Show, or Progress' ", + params: "[--source=] [--tables=] [--cells=] [--tablet_types=] [--all] [--exclude=] [--auto_start] [--stop_after_copy] [--defer-secondary-keys] [--on-ddl=] [--source_shards=] 'action must be one of the following: Create, Complete, Cancel, SwitchTraffic, ReverseTrafffic, Show, or Progress' ", 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":{}}'.`, }, { @@ -2125,7 +2125,6 @@ func commandVRWorkflow(ctx context.Context, wr *wrangler.Wrangler, subFlags *pfl // MoveTables-only params renameTables := subFlags.Bool("rename_tables", false, "MoveTables only. Rename tables instead of dropping them. --rename_tables is only supported for Complete.") - 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.") // MoveTables and Reshard params sourceShards := subFlags.String("source_shards", "", "Source shards") @@ -2263,7 +2262,6 @@ func commandVRWorkflow(ctx context.Context, wr *wrangler.Wrangler, subFlags *pfl vrwp.ExternalCluster = externalClusterName vrwp.SourceTimeZone = *sourceTimeZone vrwp.DropForeignKeys = *dropForeignKeys - vrwp.NoRoutingRules = *noRoutingRules if *sourceShards != "" { vrwp.SourceShards = strings.Split(*sourceShards, ",") } diff --git a/go/vt/wrangler/materializer.go b/go/vt/wrangler/materializer.go index 98bb3f42bc7..a44d15b36e2 100644 --- a/go/vt/wrangler/materializer.go +++ b/go/vt/wrangler/materializer.go @@ -122,8 +122,7 @@ func shouldInclude(table string, excludes []string) bool { // MoveTables initiates moving table(s) over to another keyspace func (wr *Wrangler) MoveTables(ctx context.Context, workflow, sourceKeyspace, targetKeyspace, tableSpecs, cell, tabletTypes string, allTables bool, excludeTables string, autoStart, stopAfterCopy bool, - externalCluster string, dropForeignKeys, deferSecondaryKeys bool, sourceTimeZone, onDDL string, - sourceShards []string, noRoutingRules bool) error { + externalCluster string, dropForeignKeys, deferSecondaryKeys bool, sourceTimeZone, onDDL string, sourceShards []string) error { //FIXME validate tableSpecs, allTables, excludeTables var tables []string var externalTopo *topo.Server @@ -207,36 +206,32 @@ func (wr *Wrangler) MoveTables(ctx context.Context, workflow, sourceKeyspace, ta } } if externalTopo == nil { - if noRoutingRules { - log.Warningf("Found --no-routing-rules flag, not creating routing rules for workflow %s.%s", targetKeyspace, workflow) - } else { - // Save routing rules before vschema. If we save vschema first, and routing rules - // fails to save, we may generate duplicate table errors. - rules, err := topotools.GetRoutingRules(ctx, wr.ts) - if err != nil { - return err - } - for _, table := range tables { - toSource := []string{sourceKeyspace + "." + table} - rules[table] = toSource - rules[table+"@replica"] = toSource - rules[table+"@rdonly"] = toSource - rules[targetKeyspace+"."+table] = toSource - rules[targetKeyspace+"."+table+"@replica"] = toSource - rules[targetKeyspace+"."+table+"@rdonly"] = toSource - rules[targetKeyspace+"."+table] = toSource - rules[sourceKeyspace+"."+table+"@replica"] = toSource - rules[sourceKeyspace+"."+table+"@rdonly"] = toSource - } - if err := topotools.SaveRoutingRules(ctx, wr.ts, rules); err != nil { - return err - } + // Save routing rules before vschema. If we save vschema first, and routing rules + // fails to save, we may generate duplicate table errors. + rules, err := topotools.GetRoutingRules(ctx, wr.ts) + if err != nil { + return err + } + for _, table := range tables { + toSource := []string{sourceKeyspace + "." + table} + rules[table] = toSource + rules[table+"@replica"] = toSource + rules[table+"@rdonly"] = toSource + rules[targetKeyspace+"."+table] = toSource + rules[targetKeyspace+"."+table+"@replica"] = toSource + rules[targetKeyspace+"."+table+"@rdonly"] = toSource + rules[targetKeyspace+"."+table] = toSource + rules[sourceKeyspace+"."+table+"@replica"] = toSource + rules[sourceKeyspace+"."+table+"@rdonly"] = toSource + } + if err := topotools.SaveRoutingRules(ctx, wr.ts, rules); err != nil { + return err + } - if vschema != nil { - // We added to the vschema. - if err := wr.ts.SaveVSchema(ctx, targetKeyspace, vschema); err != nil { - return err - } + if vschema != nil { + // We added to the vschema. + if err := wr.ts.SaveVSchema(ctx, targetKeyspace, vschema); err != nil { + return err } } } diff --git a/go/vt/wrangler/materializer_test.go b/go/vt/wrangler/materializer_test.go index e297f534e33..8a7e68f9d82 100644 --- a/go/vt/wrangler/materializer_test.go +++ b/go/vt/wrangler/materializer_test.go @@ -46,34 +46,6 @@ const mzCheckJournal = "/select val from _vt.resharding_journal where id=" var defaultOnDDL = binlogdatapb.OnDDLAction_name[int32(binlogdatapb.OnDDLAction_IGNORE)] -// TestMoveTablesNoRoutingRules confirms that MoveTables does not create routing rules if --no-routing-rules is specified. -func TestMoveTablesNoRoutingRules(t *testing.T) { - ms := &vtctldatapb.MaterializeSettings{ - Workflow: "workflow", - SourceKeyspace: "sourceks", - TargetKeyspace: "targetks", - TableSettings: []*vtctldatapb.TableMaterializeSettings{{ - TargetTable: "t1", - SourceExpression: "select * from t1", - }}, - } - env := newTestMaterializerEnv(t, ms, []string{"0"}, []string{"0"}) - defer env.close() - - env.tmc.expectVRQuery(100, mzCheckJournal, &sqltypes.Result{}) - env.tmc.expectVRQuery(200, mzSelectFrozenQuery, &sqltypes.Result{}) - env.tmc.expectVRQuery(200, insertPrefix, &sqltypes.Result{}) - env.tmc.expectVRQuery(200, mzSelectIDQuery, &sqltypes.Result{}) - env.tmc.expectVRQuery(200, mzUpdateQuery, &sqltypes.Result{}) - - ctx := context.Background() - err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, true) - require.NoError(t, err) - rr, err := env.wr.ts.GetRoutingRules(ctx) - require.NoError(t, err) - require.Equal(t, 0, len(rr.Rules)) -} - func TestMigrateTables(t *testing.T) { ms := &vtctldatapb.MaterializeSettings{ Workflow: "workflow", @@ -94,7 +66,7 @@ func TestMigrateTables(t *testing.T) { env.tmc.expectVRQuery(200, mzUpdateQuery, &sqltypes.Result{}) ctx := context.Background() - err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false) + err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil) require.NoError(t, err) vschema, err := env.wr.ts.GetSrvVSchema(ctx, env.cell) require.NoError(t, err) @@ -135,11 +107,11 @@ func TestMissingTables(t *testing.T) { env.tmc.expectVRQuery(200, mzUpdateQuery, &sqltypes.Result{}) ctx := context.Background() - err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1,tyt", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false) + err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1,tyt", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil) require.EqualError(t, err, "table(s) not found in source keyspace sourceks: tyt") - err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1,tyt,t2,txt", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false) + err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1,tyt,t2,txt", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil) require.EqualError(t, err, "table(s) not found in source keyspace sourceks: tyt,txt") - err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false) + err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil) require.NoError(t, err) } @@ -195,7 +167,7 @@ func TestMoveTablesAllAndExclude(t *testing.T) { env.tmc.expectVRQuery(200, insertPrefix, &sqltypes.Result{}) env.tmc.expectVRQuery(200, mzSelectIDQuery, &sqltypes.Result{}) env.tmc.expectVRQuery(200, mzUpdateQuery, &sqltypes.Result{}) - err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "", "", "", tcase.allTables, tcase.excludeTables, true, false, "", false, false, "", defaultOnDDL, nil, false) + err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "", "", "", tcase.allTables, tcase.excludeTables, true, false, "", false, false, "", defaultOnDDL, nil) require.NoError(t, err) require.EqualValues(t, tcase.want, targetTables(env)) }) @@ -229,7 +201,7 @@ func TestMoveTablesStopFlags(t *testing.T) { env.tmc.expectVRQuery(200, mzSelectIDQuery, &sqltypes.Result{}) // -auto_start=false is tested by NOT expecting the update query which sets state to RUNNING err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", - "", false, "", false, true, "", false, false, "", defaultOnDDL, nil, false) + "", false, "", false, true, "", false, false, "", defaultOnDDL, nil) require.NoError(t, err) env.tmc.verifyQueries(t) }) @@ -255,7 +227,7 @@ func TestMigrateVSchema(t *testing.T) { env.tmc.expectVRQuery(200, mzUpdateQuery, &sqltypes.Result{}) ctx := context.Background() - err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", `{"t1":{}}`, "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false) + err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", `{"t1":{}}`, "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil) require.NoError(t, err) vschema, err := env.wr.ts.GetSrvVSchema(ctx, env.cell) require.NoError(t, err) @@ -2856,7 +2828,7 @@ func TestMoveTablesDDLFlag(t *testing.T) { env.tmc.expectVRQuery(200, mzUpdateQuery, &sqltypes.Result{}) err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", - "", false, "", false, true, "", false, false, "", onDDLAction, nil, false) + "", false, "", false, true, "", false, false, "", onDDLAction, nil) require.NoError(t, err) }) } diff --git a/go/vt/wrangler/workflow.go b/go/vt/wrangler/workflow.go index 630330058c9..fbdeb58dd17 100644 --- a/go/vt/wrangler/workflow.go +++ b/go/vt/wrangler/workflow.go @@ -71,9 +71,6 @@ type VReplicationWorkflowParams struct { // Migrate specific ExternalCluster string - - // MoveTables only - NoRoutingRules bool } // VReplicationWorkflow stores various internal objects for a workflow @@ -436,7 +433,7 @@ func (vrw *VReplicationWorkflow) initMoveTables() error { return vrw.wr.MoveTables(vrw.ctx, vrw.params.Workflow, vrw.params.SourceKeyspace, vrw.params.TargetKeyspace, vrw.params.Tables, vrw.params.Cells, vrw.params.TabletTypes, vrw.params.AllTables, vrw.params.ExcludeTables, vrw.params.AutoStart, vrw.params.StopAfterCopy, vrw.params.ExternalCluster, vrw.params.DropForeignKeys, - vrw.params.DeferSecondaryKeys, vrw.params.SourceTimeZone, vrw.params.OnDDL, vrw.params.SourceShards, vrw.params.NoRoutingRules) + vrw.params.DeferSecondaryKeys, vrw.params.SourceTimeZone, vrw.params.OnDDL, vrw.params.SourceShards) } func (vrw *VReplicationWorkflow) initReshard() error {