vtctl OnlineDDL: complete command set (#12963)

* vtctl OnlineDDL: complete commands Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * cleanup Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * refactor, normalize all commands in a single parsing function Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * better error message Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
vitessio · Jul 13, 2023 · 5cffc8b · 5cffc8b
1 parent 6e77777
commit 5cffc8b
Show file tree

Hide file tree

Showing 3 changed files with 171 additions and 34 deletions.
diff --git 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", <uuid>
+ // "launch-all", <empty>
+ 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
@@ -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
@@ -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'`,