Support for ALTER VITESS_MIGRATION statements by shlomi-noach · Pull Request #7663 · vitessio/vitess

shlomi-noach · 2021-03-11T06:39:09Z

Description

This PR introduces the following queries:

ALTER VITESS_MIGRATION '8c096d5e_81c0_11eb_a8d2_f875a4d24e90' RETRY
ALTER VITESS_MIGRATION '8c096d5e_81c0_11eb_a8d2_f875a4d24e90' CANCEL
ALTER VITESS_MIGRATION CANCEL ALL

The above map to these vtctl commands, respectively:

vtctl OnlineDDL <keyspace> retry 8c096d5e_81c0_11eb_a8d2_f875a4d24e90
vtctl OnlineDDL <keyspace> cancel 8c096d5e_81c0_11eb_a8d2_f875a4d24e90
vtctl OnlineDDL <keyspace> cancel all

Also, reserved but not implemented yet:

ALTER VITESS_MIGRATION '8c096d5e_81c0_11eb_a8d2_f875a4d24e90' COMPLETE

Returned result set for these queries is always empty, but AffectedRows indicates how many migrations were actually affected by the command.

The various endtoend/onlineddl tests now use these queries.

Related Issue(s)

Checklist

Should this PR be backported?
Tests were added or are not required
Documentation was added or is not required

Deployment Notes

Impacted Areas in Vitess

Components that this PR will affect:

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

…or RETRY alter type Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2021-03-11T06:40:10Z

This PR extends #7656 ; it makes sense to review this PR after #7656 is merged.

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2021-03-11T06:49:07Z

Documentation to follow

shlomi-noach · 2021-03-11T06:52:44Z

This PR also refactors endtoend/onlineddl tests by consolidating much of the shared logic and refactoring functions into endtoend/onlineddl/vtgate_utils.go

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2021-03-11T11:44:06Z

@systay points out that this PR has commits from merged PRs. It seems to be a rebase gone wrong. However, code-wise and diff-wise there doesn't seem to be a risk here.

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2021-03-14T09:16:52Z

Confused about endtoend / End-to-End Test failures, e.g.: https://github.com/vitessio/vitess/pull/7663/checks?check_run_id=2105797750

They're consistently failing after 5m. It seems like a timeout issue. But I can't find what times out after 5m. The test itself (https://github.com/vitessio/vitess/blob/master/.github/workflows/endtoend.yml) is set with timeout-minutes: 30

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2021-03-16T07:40:36Z

Kind request for review 🙏 (reminder: this PR extends #7656)

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

The old test had hardcoded values for the expected sizes of internal data structures. This makes for very brittle tests whenever we change the AST or introduce new query types. Instead of checking for the exact sizes of the data structures, check that they're being stored and whether they're larger than than an empty plan. Signed-off-by: Vicent Marti <vmg@strn.cat>

shlomi-noach · 2021-03-16T11:00:57Z

will resolve conflicts after #7656 is merged. The conflicts are due to #7669. See d0c741f for how conflict was resolved in #7656

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2021-03-17T13:57:51Z

Sheesh. GitHub Actions is having another bad day.

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

systay · 2021-03-17T14:33:48Z

go/vt/vtgate/planbuilder/migration.go

 )

-func buildRevertPlan(query string, stmt *sqlparser.RevertMigration, vschema ContextVSchema) (engine.Primitive, error) {
+func buildAlterMigrationPlan(query string, stmt *sqlparser.AlterMigration, vschema ContextVSchema) (engine.Primitive, error) {


nit: why ask for stmt and then not use it?

Nice catch! Removed.

systay · 2021-03-17T14:38:46Z

go/vt/vttablet/onlineddl/executor.go

 	e.migrationMutex.Lock()
 	defer e.migrationMutex.Unlock()
-	parsed := sqlparser.BuildParsedQuery(sqlRetryMigration, ":tablet", whereExpr)
+	parsed := sqlparser.BuildParsedQuery(sqlRetryMigrationWhere, ":tablet", whereExpr)


just noticed that three lines down, on line 2247, we are never checking err from parsed.GenerateQuery

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

…terface-alter Signed-off-by: Andres Taylor <andres@planetscale.com>

systay

LGTM!

shlomi-noach added 16 commits March 10, 2021 12:34

parser support for ALTER VITESS_MIGRATION

dfb2a2f

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

ALTER VITESS_MIGRATION plan is Send

f956f72

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

rename migration plan file

365cc5d

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Format; test parsing of ALTER VITESS_MIGRATION statements

fde7f83

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Support for ALTER VITESS_MIGRATION plan in vttablet; implementation f…

0e9566c

…or RETRY alter type Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

using ALTER VITESS_MIGRATION ... RETRY in endtoend/onlineddl/vrepl

785d1ac

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

using ALTER VITESS_MIGRATION ... RETRY in endtoend/onlineddl/ghost

ffb2972

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

CancelMigration and CancelAllMigrations are public functions

90ba772

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

using ALTER VITESS_MIGRATION ... CANCEL in endtoend/onlineddl/ghost

c77e01d

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

using ALTER VITESS_MIGRATION ... CANCEL in endtoend/onlineddl/vrepl

9a55059

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

using ALTER VITESS_MIGRATION ... CANCEL ALL in endtoend/onlineddl/

d45b7cb

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

using ALTER VITESS_MIGRATION ... CANCEL ALL in endtoend/onlineddl/

9671f3e

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

using ALTER VITESS_MIGRATION ... CANCEL ALL in endtoend/onlineddl/

5be1535

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

refactor: CheckMigrationStatus in endtoend/onlineddl

b86b21e

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

refactor: VtgateExecDDL in endtoend/onlineddl

384088b

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

fix expect number of migrations to cancel

b127d93

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach requested review from GuptaManan100, harshit-gangal and systay as code owners March 11, 2021 06:39

shlomi-noach requested a review from deepthi March 11, 2021 06:39

merge master, resolve conflicts

9aa7fd0

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach mentioned this pull request Mar 11, 2021

Suggestions for online DDL query syntax #7604

Closed

shlomi-noach added 2 commits March 11, 2021 10:22

fix test assertion

542c415

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

make sizegen

b6fb4ee

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach added 2 commits March 14, 2021 07:20

merge master, resolve conflict

72211cd

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

sizegen

1e54273

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach added 3 commits March 14, 2021 19:36

copyright year

5b882d4

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

merge master, resolve conflicts

9561fce

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Merge branch 'master' into online-ddl-sql-interface-alter

ca6eb9a

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach mentioned this pull request Mar 16, 2021

VTGate: support -enable_online_ddl flag #7694

Merged

8 tasks

shlomi-noach and others added 2 commits March 16, 2021 09:40

Merge branch 'master' into online-ddl-sql-interface-alter

89cb0bf

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach added 2 commits March 17, 2021 13:08

merge master, resolve conflicts

1000527

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

merge master, resolve conflicts

4db306a

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

merge master, resolve conflicts

3b5f9cd

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

systay reviewed Mar 17, 2021

View reviewed changes

shlomi-noach and others added 2 commits March 17, 2021 17:12

fixes per review

4b64dbf

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Merge remote-tracking branch 'upstream/master' into online-ddl-sql-in…

5a66255

…terface-alter Signed-off-by: Andres Taylor <andres@planetscale.com>

systay approved these changes Mar 18, 2021

View reviewed changes

shlomi-noach merged commit 31e437d into vitessio:master Mar 18, 2021

shlomi-noach deleted the online-ddl-sql-interface-alter branch March 18, 2021 14:23

askdba added the Component: Query Serving label Mar 23, 2021

askdba added this to the v10.0 milestone Mar 23, 2021

shlomi-noach mentioned this pull request Mar 25, 2021

Online DDL: revertible migrations, declarative migrations, SQL commands vitessio/website#733

Merged

shlomi-noach mentioned this pull request Apr 5, 2021

Online DDL plan via Send; "singleton" migrations on tablets #7785

Merged

9 tasks

ajm188 mentioned this pull request Jul 15, 2021

slack vitess v10.pre tinyspeck/vitess#228

Merged

Conversation

shlomi-noach commented Mar 11, 2021

Description

Related Issue(s)

Checklist

Deployment Notes

Impacted Areas in Vitess

Uh oh!

shlomi-noach commented Mar 11, 2021

Uh oh!

shlomi-noach commented Mar 11, 2021

Uh oh!

shlomi-noach commented Mar 11, 2021

Uh oh!

shlomi-noach commented Mar 11, 2021

Uh oh!

shlomi-noach commented Mar 14, 2021

Uh oh!

shlomi-noach commented Mar 16, 2021

Uh oh!

shlomi-noach commented Mar 16, 2021

Uh oh!

shlomi-noach commented Mar 17, 2021

Uh oh!

systay Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

shlomi-noach Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

systay Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

shlomi-noach Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

systay left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants