Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

vtctl OnlineDDL: complete command set #12963

Merged
merged 5 commits into from
Jul 13, 2023
Merged

vtctl OnlineDDL: complete command set #12963

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
039acd8
vtctl OnlineDDL: complete commands
shlomi-noach Apr 25, 2023
e970022
cleanup
shlomi-noach Apr 25, 2023
b0bb74f
refactor, normalize all commands in a single parsing function
shlomi-noach Jul 12, 2023
1357cc0
Merge branch 'main' into vtctl-onlineddl-throttle
shlomi-noach Jul 12, 2023
27d4aae
better error message
shlomi-noach Jul 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 52 additions & 1 deletion go/vt/vtctl/vtctl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,8 +607,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" +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO cancel all is more intuitive, but it's a matter of taste and don't feel strongly about it. The benefit is that it's a single command/signature with the second argument being UUID/all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cancel all (and likewise complete all, throttle all, etc.) are now supported. Refactored the code a bit to generalize the parsing of the command.

" \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",
Expand Down Expand Up @@ -2987,11 +2998,31 @@ func commandOnlineDDL(ctx context.Context, wr *wrangler.Wrangler, subFlags *pfla
return fmt.Errorf("UUID required")
}
applySchemaQuery, bindErr = sqlparser.ParseAndBind(`alter vitess_migration %a retry`, sqltypes.StringBindVariable(arg))
case "cleanup":
if arg == "" {
return fmt.Errorf("UUID required")
}
applySchemaQuery, bindErr = sqlparser.ParseAndBind(`alter vitess_migration %a cleanup`, sqltypes.StringBindVariable(arg))
case "launch":
if arg == "" {
return fmt.Errorf("UUID required")
}
applySchemaQuery, bindErr = sqlparser.ParseAndBind(`alter vitess_migration %a launch`, sqltypes.StringBindVariable(arg))
case "launch-all":
if arg != "" {
return fmt.Errorf("UUID not allowed in %s", command)
}
applySchemaQuery = `alter vitess_migration launch all`
case "complete":
if arg == "" {
return fmt.Errorf("UUID required")
}
applySchemaQuery, bindErr = sqlparser.ParseAndBind(`alter vitess_migration %a complete`, sqltypes.StringBindVariable(arg))
case "complete-all":
if arg != "" {
return fmt.Errorf("UUID not allowed in %s", command)
}
applySchemaQuery = `alter vitess_migration complete all`
case "cancel":
if arg == "" {
return fmt.Errorf("UUID required")
Expand All @@ -3002,6 +3033,26 @@ func commandOnlineDDL(ctx context.Context, wr *wrangler.Wrangler, subFlags *pfla
return fmt.Errorf("UUID not allowed in %s", command)
}
applySchemaQuery = `alter vitess_migration cancel all`
case "throttle":
if arg == "" {
return fmt.Errorf("UUID required")
}
applySchemaQuery, bindErr = sqlparser.ParseAndBind(`alter vitess_migration %a throttle`, sqltypes.StringBindVariable(arg))
case "unthrottle":
if arg == "" {
return fmt.Errorf("UUID required")
}
applySchemaQuery, bindErr = sqlparser.ParseAndBind(`alter vitess_migration %a unthrottle`, sqltypes.StringBindVariable(arg))
case "throttle-all":
if arg != "" {
return fmt.Errorf("UUID not allowed in %s", command)
}
applySchemaQuery = `alter vitess_migration throttle all`
case "unthrottle-all":
if arg != "" {
return fmt.Errorf("UUID not allowed in %s", command)
}
applySchemaQuery = `alter vitess_migration unthrottle all`
default:
return fmt.Errorf("Unknown OnlineDDL command: %s", command)
}
Expand Down
7 changes: 0 additions & 7 deletions go/vt/vttablet/onlineddl/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'`,
Expand Down