-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
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
vtctl ApplySchema accepts 'ALTER VITESS_MIGRATION...' statements #9303
vtctl ApplySchema accepts 'ALTER VITESS_MIGRATION...' statements #9303
Conversation
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
exec.executeOnlineDDL(ctx, execResult, onlineDDL) | ||
exec.executeOnAllTablets(ctx, execResult, onlineDDL.SQL, true) | ||
if len(execResult.SuccessShards) > 0 { | ||
exec.wr.Logger().Printf("%s\n", onlineDDL.UUID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change here is to remove if exec.ddlStrategySetting.IsSkipTopo() {
-- we've already hard coded that as true
is previous releases.
And of course the else
code cannot ever happen, so that's also removed.
Signed-off-by: Shlomi Noach <[email protected]>
exec.executeOnlineDDL(ctx, execResult, onlineDDL) | ||
} | ||
exec.executeOnAllTablets(ctx, execResult, onlineDDL.SQL, true) | ||
exec.wr.Logger().Printf("%s\n", onlineDDL.UUID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change here is to remove if exec.ddlStrategySetting.IsSkipTopo() {
-- we've already hard coded that as true
is previous releases.
And of course the else
code cannot ever happen, so that's also removed.
exec.wr.Logger().Printf("%s\n", onlineDDL.UUID) | ||
return nil | ||
case *sqlparser.AlterMigration: | ||
exec.executeOnAllTablets(ctx, execResult, sql, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the actual change in this PR: direct AlterMigration
statements to the tablets via QueryService
(that's what the last true
argument indicates).
Signed-off-by: Shlomi Noach <[email protected]>
Code commits of this PR were already merged by another PR. All that is left in this PR is release notes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be merged after making the requested change + resolve conflicts.
doc/releasenotes/13_0_0_summary.md
Outdated
@@ -47,13 +47,22 @@ This command indicates that a migration executed with `-postpone-completion` is | |||
- For running `ALTER`s (`online` and `gh-ost`) which are only partly through the migration: they will cut-over automatically when they complete their work, as if `-postpone-completion` wasn't indicated | |||
- For queued `CREATE` and `DROP` migrations: "unblock" them from being scheduled. They'll be scheduled at the scheduler's discretion. there is no guarantee that they will be scheduled to run immediately. | |||
|
|||
### vtctl ApplySchema: ALTER VITESS_MIGRATION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
### vtctl ApplySchema: ALTER VITESS_MIGRATION | |
### vtctl/vtctlclient ApplySchema: ALTER VITESS_MIGRATION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion to keep this consistent with other release notes.
Since the only remaining changes are to the release notes file, I suggest doing either a "squash-merge" or a rebase before merge so that there is only one commit. |
Signed-off-by: Shlomi Noach <[email protected]>
squash-merged. |
Description
The following is now supported:
$ vtctlclient ApplySchema -sql "alter vitess_migration 'dce5f77e_51de_11ec_a049_0a43f95f28a3' cancel" commerce
ApplySchema
now accepts all variants ofALTER VITESS_MIGRATION...
.Why are we doing this?
We long since wanted to deprecate VExec for OnlineDDL. Right now
vtctl OnlineDDL <command>
heavily relies on wrangler/VExec to get the command to the underlying tablets. However,ApplySchema
already has the required logic to get in to theQueryService
. We will reimplementvtctl OnlineDDL
on top ofApplySchema
instead ofVExec
.This PR is a first step towards that re-implementation.
Related Issue(s)
Checklist