From da2c94dbdcdc9371fdd013aec59b1c16b93f6e67 Mon Sep 17 00:00:00 2001 From: Rohit Nayak Date: Fri, 25 Aug 2023 20:41:50 +0200 Subject: [PATCH 1/3] Add new flag to MoveTables --no-routing-rules, so that routing rules are not created when a MoveTables workflow is created Signed-off-by: Rohit Nayak --- go/vt/vtctl/vtctl.go | 4 +- go/vt/wrangler/materializer.go | 57 ++++++++++++++++------------- go/vt/wrangler/materializer_test.go | 45 +++++++++++++++++++---- go/vt/wrangler/workflow.go | 5 ++- 4 files changed, 75 insertions(+), 36 deletions(-) diff --git a/go/vt/vtctl/vtctl.go b/go/vt/vtctl/vtctl.go index 93628b4d0f5..da3b111213f 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=] '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=] [--no-routing-rules] '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,6 +2125,7 @@ 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, "MoveTables only. Do not create routing rules for the target keyspace. --no_routing_rules is only supported for Create. You cannot use SwitchTraffic and can only Cancel the workflow after manually pointing the application to the target keyspace.") // MoveTables and Reshard params sourceShards := subFlags.String("source_shards", "", "Source shards") @@ -2262,6 +2263,7 @@ 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 a44d15b36e2..b440ad939db 100644 --- a/go/vt/wrangler/materializer.go +++ b/go/vt/wrangler/materializer.go @@ -122,7 +122,8 @@ 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) error { + externalCluster string, dropForeignKeys, deferSecondaryKeys bool, sourceTimeZone, onDDL string, + sourceShards []string, noRoutingRules bool) error { //FIXME validate tableSpecs, allTables, excludeTables var tables []string var externalTopo *topo.Server @@ -206,33 +207,37 @@ func (wr *Wrangler) MoveTables(ctx context.Context, workflow, sourceKeyspace, ta } } if externalTopo == nil { - // 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 { + 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 } + + if vschema != nil { + // We added to the vschema. + if err := wr.ts.SaveVSchema(ctx, targetKeyspace, vschema); err != nil { + return err + } + } } } if err := wr.ts.RebuildSrvVSchema(ctx, nil); err != nil { diff --git a/go/vt/wrangler/materializer_test.go b/go/vt/wrangler/materializer_test.go index 8a7e68f9d82..5248bc8a9c4 100644 --- a/go/vt/wrangler/materializer_test.go +++ b/go/vt/wrangler/materializer_test.go @@ -46,6 +46,35 @@ 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) + vschema, err := env.wr.ts.GetSrvVSchema(ctx, env.cell) + require.NoError(t, err) + got := fmt.Sprintf("%v", vschema) + require.Contains(t, got, `keyspaces:{key:"sourceks" value:{}} keyspaces:{key:"targetks" value:{}} routing_rules:{} shard_routing_rules:{}`) +} + func TestMigrateTables(t *testing.T) { ms := &vtctldatapb.MaterializeSettings{ Workflow: "workflow", @@ -66,7 +95,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) + err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false) require.NoError(t, err) vschema, err := env.wr.ts.GetSrvVSchema(ctx, env.cell) require.NoError(t, err) @@ -107,11 +136,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) + err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1,tyt", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false) 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) + err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1,tyt,t2,txt", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false) 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) + err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false) require.NoError(t, err) } @@ -167,7 +196,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) + err = env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "", "", "", tcase.allTables, tcase.excludeTables, true, false, "", false, false, "", defaultOnDDL, nil, false) require.NoError(t, err) require.EqualValues(t, tcase.want, targetTables(env)) }) @@ -201,7 +230,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, true, "", false, false, "", defaultOnDDL, nil, false) require.NoError(t, err) env.tmc.verifyQueries(t) }) @@ -227,7 +256,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) + err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", `{"t1":{}}`, "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, false) require.NoError(t, err) vschema, err := env.wr.ts.GetSrvVSchema(ctx, env.cell) require.NoError(t, err) @@ -2828,7 +2857,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, true, "", false, false, "", onDDLAction, nil, false) require.NoError(t, err) }) } diff --git a/go/vt/wrangler/workflow.go b/go/vt/wrangler/workflow.go index fbdeb58dd17..630330058c9 100644 --- a/go/vt/wrangler/workflow.go +++ b/go/vt/wrangler/workflow.go @@ -71,6 +71,9 @@ type VReplicationWorkflowParams struct { // Migrate specific ExternalCluster string + + // MoveTables only + NoRoutingRules bool } // VReplicationWorkflow stores various internal objects for a workflow @@ -433,7 +436,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.DeferSecondaryKeys, vrw.params.SourceTimeZone, vrw.params.OnDDL, vrw.params.SourceShards, vrw.params.NoRoutingRules) } func (vrw *VReplicationWorkflow) initReshard() error { From 7d6dd0fe7120e0622592f0ff275d74a393368329 Mon Sep 17 00:00:00 2001 From: Rohit Nayak Date: Sat, 26 Aug 2023 21:25:36 +0200 Subject: [PATCH 2/3] Address review comments Signed-off-by: Rohit Nayak --- go/vt/vtctl/vtctl.go | 2 +- go/vt/wrangler/materializer.go | 2 +- go/vt/wrangler/materializer_test.go | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/go/vt/vtctl/vtctl.go b/go/vt/vtctl/vtctl.go index da3b111213f..9fc08bc03c1 100644 --- a/go/vt/vtctl/vtctl.go +++ b/go/vt/vtctl/vtctl.go @@ -2125,7 +2125,7 @@ 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, "MoveTables only. Do not create routing rules for the target keyspace. --no_routing_rules is only supported for Create. You cannot use SwitchTraffic and can only Cancel the workflow after manually pointing the application to the target keyspace.") + noRoutingRules := subFlags.Bool("no-routing-rules", false, "MoveTables Create only. Do not create routing rules in the target keyspace. When this is used you cannot use SwitchTraffic for the workflow and instead can only Cancel the workflow after manually pointing the application to the target keyspace.") // MoveTables and Reshard params sourceShards := subFlags.String("source_shards", "", "Source shards") diff --git a/go/vt/wrangler/materializer.go b/go/vt/wrangler/materializer.go index b440ad939db..98bb3f42bc7 100644 --- a/go/vt/wrangler/materializer.go +++ b/go/vt/wrangler/materializer.go @@ -208,7 +208,7 @@ 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) + 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. diff --git a/go/vt/wrangler/materializer_test.go b/go/vt/wrangler/materializer_test.go index 5248bc8a9c4..e297f534e33 100644 --- a/go/vt/wrangler/materializer_test.go +++ b/go/vt/wrangler/materializer_test.go @@ -69,10 +69,9 @@ func TestMoveTablesNoRoutingRules(t *testing.T) { ctx := context.Background() err := env.wr.MoveTables(ctx, "workflow", "sourceks", "targetks", "t1", "", "", false, "", true, false, "", false, false, "", defaultOnDDL, nil, true) require.NoError(t, err) - vschema, err := env.wr.ts.GetSrvVSchema(ctx, env.cell) + rr, err := env.wr.ts.GetRoutingRules(ctx) require.NoError(t, err) - got := fmt.Sprintf("%v", vschema) - require.Contains(t, got, `keyspaces:{key:"sourceks" value:{}} keyspaces:{key:"targetks" value:{}} routing_rules:{} shard_routing_rules:{}`) + require.Equal(t, 0, len(rr.Rules)) } func TestMigrateTables(t *testing.T) { From 8f9995de7cc8bad7c900e0d1135e1e67e29047dc Mon Sep 17 00:00:00 2001 From: Rohit Nayak Date: Sun, 27 Aug 2023 20:02:50 +0200 Subject: [PATCH 3/3] Update vtctl flag docs since we _can_ use SwitchTraffic with this flag Signed-off-by: Rohit Nayak --- go/vt/vtctl/vtctl.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/vtctl/vtctl.go b/go/vt/vtctl/vtctl.go index 9fc08bc03c1..86a002bde9a 100644 --- a/go/vt/vtctl/vtctl.go +++ b/go/vt/vtctl/vtctl.go @@ -2125,7 +2125,7 @@ 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, "MoveTables Create only. Do not create routing rules in the target keyspace. When this is used you cannot use SwitchTraffic for the workflow and instead can only Cancel the workflow after manually pointing the application to the target keyspace.") + 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")