diff --git a/go/vt/vtctl/vtctl.go b/go/vt/vtctl/vtctl.go index 70ae6c90832..1e475dd9ae4 100644 --- a/go/vt/vtctl/vtctl.go +++ b/go/vt/vtctl/vtctl.go @@ -608,8 +608,19 @@ var commands = []commandGroup{ " \nvtctl OnlineDDL test_keyspace show running" + " \nvtctl OnlineDDL test_keyspace show complete" + " \nvtctl OnlineDDL test_keyspace show failed" + + " \nvtctl OnlineDDL test_keyspace cleanup 82fa54ac_e83e_11ea_96b7_f875a4d24e90" + " \nvtctl OnlineDDL test_keyspace retry 82fa54ac_e83e_11ea_96b7_f875a4d24e90" + - " \nvtctl OnlineDDL test_keyspace cancel 82fa54ac_e83e_11ea_96b7_f875a4d24e90", + " \nvtctl OnlineDDL test_keyspace cancel 82fa54ac_e83e_11ea_96b7_f875a4d24e90" + + " \nvtctl OnlineDDL test_keyspace cancel-all" + + " \nvtctl OnlineDDL test_keyspace launch 82fa54ac_e83e_11ea_96b7_f875a4d24e90" + + " \nvtctl OnlineDDL test_keyspace launch-all" + + " \nvtctl OnlineDDL test_keyspace complete 82fa54ac_e83e_11ea_96b7_f875a4d24e90" + + " \nvtctl OnlineDDL test_keyspace complete-all" + + " \nvtctl OnlineDDL test_keyspace throttle 82fa54ac_e83e_11ea_96b7_f875a4d24e90" + + " \nvtctl OnlineDDL test_keyspace throttle-all" + + " \nvtctl OnlineDDL test_keyspace unthrottle 82fa54ac_e83e_11ea_96b7_f875a4d24e90" + + " \nvtctl OnlineDDL test_keyspace unthrottle-all" + + "", }, { name: "ValidateVersionShard", @@ -2921,6 +2932,34 @@ func commandApplySchema(ctx context.Context, wr *wrangler.Wrangler, subFlags *pf return nil } +func generateOnlineDDLQuery(command string, arg string, allSupported bool) (string, error) { + // Accept inputs like so: + // "launch", "all" + // "launch", + // "launch-all", + if tokens := strings.Split(command, "-"); len(tokens) == 2 && tokens[1] == "all" { + // command is e.g. "launch-all" + if arg != "" { + return "", fmt.Errorf("UUID not allowed in '%s' command", command) + } + // transform "launch-all" into "launch", "all" + command = tokens[0] + arg = "all" + } + switch arg { + case "": + return "", fmt.Errorf("UUID|all required") + case "all": + if !allSupported { + return "", fmt.Errorf("'all' not supported for '%s' command", command) + } + return fmt.Sprintf(`alter vitess_migration %s all`, command), nil + default: + query := `alter vitess_migration %a ` + command + return sqlparser.ParseAndBind(query, sqltypes.StringBindVariable(arg)) + } +} + func commandOnlineDDL(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error { json := subFlags.Bool("json", false, "Output JSON instead of human-readable table") orderBy := subFlags.String("order", "ascending", "Sort the results by `id` property of the Schema migration (default is ascending. Allowed values are `ascending` or `descending`.") @@ -2944,7 +2983,7 @@ func commandOnlineDDL(ctx context.Context, wr *wrangler.Wrangler, subFlags *pfla applySchemaQuery := "" executeFetchQuery := "" - var bindErr error + var err error switch command { case "show": condition := "" @@ -2960,12 +2999,12 @@ func commandOnlineDDL(ctx context.Context, wr *wrangler.Wrangler, subFlags *pfla string(schema.OnlineDDLStatusRunning), string(schema.OnlineDDLStatusComplete), string(schema.OnlineDDLStatusFailed): - condition, bindErr = sqlparser.ParseAndBind("migration_status=%a", sqltypes.StringBindVariable(arg)) + condition, err = sqlparser.ParseAndBind("migration_status=%a", sqltypes.StringBindVariable(arg)) default: if schema.IsOnlineDDLUUID(arg) { - condition, bindErr = sqlparser.ParseAndBind("migration_uuid=%a", sqltypes.StringBindVariable(arg)) + condition, err = sqlparser.ParseAndBind("migration_uuid=%a", sqltypes.StringBindVariable(arg)) } else { - condition, bindErr = sqlparser.ParseAndBind("migration_context=%a", sqltypes.StringBindVariable(arg)) + condition, err = sqlparser.ParseAndBind("migration_context=%a", sqltypes.StringBindVariable(arg)) } } order := " order by `id` " @@ -2984,31 +3023,29 @@ func commandOnlineDDL(ctx context.Context, wr *wrangler.Wrangler, subFlags *pfla executeFetchQuery = fmt.Sprintf(`select * from _vt.schema_migrations where %s %s %s`, condition, order, skipLimit) - case "retry": - if arg == "" { - return fmt.Errorf("UUID required") - } - applySchemaQuery, bindErr = sqlparser.ParseAndBind(`alter vitess_migration %a retry`, sqltypes.StringBindVariable(arg)) - case "complete": - if arg == "" { - return fmt.Errorf("UUID required") - } - applySchemaQuery, bindErr = sqlparser.ParseAndBind(`alter vitess_migration %a complete`, sqltypes.StringBindVariable(arg)) - case "cancel": - if arg == "" { - return fmt.Errorf("UUID required") - } - applySchemaQuery, bindErr = sqlparser.ParseAndBind(`alter vitess_migration %a cancel`, sqltypes.StringBindVariable(arg)) - case "cancel-all": - if arg != "" { - return fmt.Errorf("UUID not allowed in %s", command) - } - applySchemaQuery = `alter vitess_migration cancel all` + case + "retry", + "cleanup": + // Do not support 'ALL' argument + applySchemaQuery, err = generateOnlineDDLQuery(command, arg, false) + case + "launch", + "launch-all", + "complete", + "complete-all", + "cancel", + "cancel-all", + "throttle", + "throttle-all", + "unthrottle", + "unthrottle-all": + // Support 'ALL' argument + applySchemaQuery, err = generateOnlineDDLQuery(command, arg, true) default: return fmt.Errorf("Unknown OnlineDDL command: %s", command) } - if bindErr != nil { - return fmt.Errorf("Error generating OnlineDDL query: %+v", bindErr) + if err != nil { + return fmt.Errorf("Error generating OnlineDDL query: %+v", err) } if applySchemaQuery != "" { diff --git a/go/vt/vtctl/vtctl_test.go b/go/vt/vtctl/vtctl_test.go index 5424c124382..0ee4096a333 100644 --- a/go/vt/vtctl/vtctl_test.go +++ b/go/vt/vtctl/vtctl_test.go @@ -26,6 +26,7 @@ import ( "time" "github.com/spf13/pflag" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" @@ -360,3 +361,109 @@ func TestMoveTables(t *testing.T) { }) } } + +func TestGenerateOnlineDDLQuery(t *testing.T) { + tcases := []struct { + cmd string + arg string + allSupported bool + expectError bool + expectQuery string + }{ + { + "launch", + "all", + true, + false, + "alter vitess_migration launch all", + }, + { + "launch-all", + "", + true, + false, + "alter vitess_migration launch all", + }, + { + "launch", + "718169cc_1fea_11ee_82b1_0a43f95f28a3", + true, + false, + "alter vitess_migration '718169cc_1fea_11ee_82b1_0a43f95f28a3' launch", + }, + { + "cancel", + "718169cc_1fea_11ee_82b1_0a43f95f28a3", + true, + false, + "alter vitess_migration '718169cc_1fea_11ee_82b1_0a43f95f28a3' cancel", + }, + { + "unthrottle", + "718169cc_1fea_11ee_82b1_0a43f95f28a3", + true, + false, + "alter vitess_migration '718169cc_1fea_11ee_82b1_0a43f95f28a3' unthrottle", + }, + { + "unthrottle", + "", + true, + true, + "", + }, + { + "unthrottle-all", + "all", + true, + true, + "", + }, + { + "unthrottle-all", + "718169cc_1fea_11ee_82b1_0a43f95f28a3", + true, + true, + "", + }, + { + "retry", + "718169cc_1fea_11ee_82b1_0a43f95f28a3", + false, + false, + "alter vitess_migration '718169cc_1fea_11ee_82b1_0a43f95f28a3' retry", + }, + { + "retry-all", + "718169cc_1fea_11ee_82b1_0a43f95f28a3", + false, + true, + "", + }, + { + "retry-all", + "", + false, + true, + "", + }, + { + "retry", + "all", + false, + true, + "", + }, + } + for _, tcase := range tcases { + t.Run(fmt.Sprintf("%s %s", tcase.cmd, tcase.arg), func(t *testing.T) { + query, err := generateOnlineDDLQuery(tcase.cmd, tcase.arg, tcase.allSupported) + if tcase.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tcase.expectQuery, query) + } + }) + } +} diff --git a/go/vt/vttablet/onlineddl/schema.go b/go/vt/vttablet/onlineddl/schema.go index 39288248cdb..5e48cc8e1cf 100644 --- a/go/vt/vttablet/onlineddl/schema.go +++ b/go/vt/vttablet/onlineddl/schema.go @@ -568,13 +568,6 @@ const ( sqlFindProcess = "SELECT id, Info as info FROM information_schema.processlist WHERE id=%a AND Info LIKE %a" ) -const ( - retryMigrationHint = "retry" - cancelMigrationHint = "cancel" - cancelAllMigrationHint = "cancel-all" - completeMigrationHint = "complete" -) - var ( sqlCreateOnlineDDLUser = []string{ `CREATE USER IF NOT EXISTS %s IDENTIFIED BY '%s'`,