OnlineDDL: 'mysql' strategy, managed by the scheduler, but executed via normal MySQL statements #12027

shlomi-noach · 2023-01-02T08:50:58Z

Description

This PR introduces a new DDL strategy: mysql. This strategy is a hybrid between direct (which is completely non-Online) and the various online strategies.

A migration submitted with mysql strategy is managed. That is, it gets a migration UUID. The scheduler queues it, reviews it, runs it. The user may cancel or retry it, much like all Online DDL migrations. The difference is that when the scheduler runs the migration via normal MySQL CREATE/ALTER/DROP TABLE statements.

The user may add ALGORITHM=INPLACE or ALGORITHM=INSTANT as they please, and the scheduler will attempt to run those as is. Some migrations will be completely blocking. See the MySQL documentation. In particular, consider that for non-INSTANT DDLs, replicas will accumulate substantial lag.

This was requested and discussed by @derekperkins in a recent Vitess Open Hour call.

Related Issue(s)

Tracking: #6926

Checklist

"Backport to:" labels have been added if this change should be back-ported
Tests were added or are not required
Documentation was added or is not required

Deployment Notes

…ia normal MySQL 'ALTER TABLE' Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

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

vitess-bot · 2023-01-02T08:51:00Z

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

Ensure that the Pull Request has a descriptive title.
If this is a change that users need to know about, please apply the release notes (needs details) label so that merging is blocked unless the summary release notes document is included.
If a test is added or modified, there should be a documentation on top of the test to explain what the expected behavior is what the test does.

If a new flag is being introduced:

Is it really necessary to add this flag?
Flag names should be clear and intuitive (as far as possible)
Help text should be descriptive.
Flag names should use dashes (-) as word separators rather than underscores (_).

If a workflow is added or modified:

Each item in Jobs should be named in order to mark it as required.
If the workflow should be required, the maintainer team should be notified.

Bug fixes

There should be at least one unit or end-to-end test.
The Pull Request description should include a link to an issue that describes the bug.

Non-trivial changes

There should be some code comments as to why things are implemented the way they are.

New/Existing features

Should be documented, either by modifying the existing documentation or creating new documentation.
New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

Backward compatibility

Protobuf changes should be wire-compatible.
Changes to _vt tables and RPCs need to be backward compatible.
vtctl command output order should be stable and awk-able.
RPC changes should be compatible with vitess-operator
If a flag is removed, then it should also be removed from VTop, if used there.

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

mattlord · 2023-01-03T14:32:38Z

go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go

+ t.Run("declarative", func(t *testing.T) {
+ t1uuid = testOnlineDDLStatement(t, createT1Statement, "mysql --declarative", "vtgate", "just-created", "", false)
+
+ status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, t1uuid, normalWaitTime, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed)


Curious why we specify multiple statuses in WaitForMigrationStatus but then only a single one in CheckMigrationStatus?

The reason is that we wait till either status is seen (the function will otherwise give some 30sec for the migration to "evolve"). So there are terminal states for the function, so that it does not need to wait after meeting these states. This just reduces test time. Then, we really want to validate that we reached a particular result: typically we look for a success, ie OnlineDDLStatusComplete. But sometimes we intentionally seek a failure.

derekperkins

This is awesome, thanks for pushing this through!

deepthi

This is great. I like how you keep improving online DDL to take advantage of upstream MySQL improvements.
I think this is significant enough to either warrant a new issue altogether, or at least a specific comment in the tracking issue as to why we want to introduce this option. Currently there's nothing in the tracking issue that tells us anything about why this PR is being proposed.
Also website docs 😄

shlomi-noach · 2023-01-10T06:26:28Z

This is great. I like how you keep improving online DDL to take advantage of upstream MySQL improvements.
I think this is significant enough to either warrant a new issue altogether, or at least a specific comment in the tracking issue as to why we want to introduce this option. Currently there's nothing in the tracking issue that tells us anything about why this PR is being proposed.

👍

Also website docs smile

👍

Let me first create all the necessary docs before we merge this PR

shlomi-noach · 2023-01-11T09:41:50Z

Website docs: vitessio/website#1342

shlomi-noach · 2023-01-11T09:59:13Z

Fixes #12075

I've put #12075 up to explain why we're doing this.

shlomi-noach · 2023-01-11T10:00:16Z

@deepthi for final approval + merge, as there's now a tracking issue as well as website docs

…er, but executed via normal MySQL statements (#1499) * OnlineDDL: 'mysql' strategy, managed by the scheduler, but executed via normal MySQL statements (#12027) * OnlineDDL: 'mysql' strategy: managed by the scheduler, but executed via normal MySQL 'ALTER TABLE' Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * use 'mysql' strategy Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * fail mysql strategy on incompatible strategy flags Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * endtoend tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * release notes Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Fix bad merge. (#12087) GitHub did not seem to pick up the merge conflict between these two commits (older to newer): - 673573a - 836b3c1 The first commit changed the function signature for testOnlineDDLStatement(), while the later commit used the old signature. Signed-off-by: Matt Lord <mattalord@gmail.com> Signed-off-by: Matt Lord <mattalord@gmail.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Signed-off-by: Matt Lord <mattalord@gmail.com> Co-authored-by: Matt Lord <mattalord@gmail.com>

shlomi-noach added 4 commits January 2, 2023 07:01

OnlineDDL: 'mysql' strategy: managed by the scheduler, but executed v…

cad6798

…ia normal MySQL 'ALTER TABLE' Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

use 'mysql' strategy

21474d2

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

fail mysql strategy on incompatible strategy flags

4c20a95

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

endtoend tests

4502d82

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

shlomi-noach added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: Query Serving labels Jan 2, 2023

shlomi-noach requested a review from derekperkins January 2, 2023 08:50

shlomi-noach requested review from mattlord and rohit-nayak-ps as code owners January 2, 2023 08:50

release notes

009ed9f

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

shlomi-noach requested review from deepthi, frouioui, GuptaManan100 and rsajwani as code owners January 2, 2023 09:41

mattlord approved these changes Jan 3, 2023

View reviewed changes

derekperkins approved these changes Jan 3, 2023

View reviewed changes

deepthi added the NeedsWebsiteDocsUpdate What it says label Jan 9, 2023

deepthi reviewed Jan 9, 2023

View reviewed changes

shlomi-noach mentioned this pull request Jan 11, 2023

Documenting new 'mysql' DDL strategy vitessio/website#1342

Merged

shlomi-noach removed the NeedsWebsiteDocsUpdate What it says label Jan 11, 2023

deepthi approved these changes Jan 13, 2023

View reviewed changes

deepthi merged commit 836b3c1 into vitessio:main Jan 13, 2023

deepthi deleted the onlineddl-mysql-strategy branch January 13, 2023 00:20

mattlord mentioned this pull request Jan 13, 2023

Internal: Fix Bad Merge #12087

Merged

3 tasks

shlomi-noach mentioned this pull request Feb 21, 2023

Feature Request: introduce mysql DDL strategy, managed, but executed with MySQL DDL #12075

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OnlineDDL: 'mysql' strategy, managed by the scheduler, but executed via normal MySQL statements #12027

OnlineDDL: 'mysql' strategy, managed by the scheduler, but executed via normal MySQL statements #12027

shlomi-noach commented Jan 2, 2023 •

edited

Loading

vitess-bot bot commented Jan 2, 2023 •

edited by deepthi

Loading

mattlord Jan 3, 2023

shlomi-noach Jan 3, 2023

derekperkins left a comment

deepthi left a comment

shlomi-noach commented Jan 10, 2023

shlomi-noach commented Jan 11, 2023

shlomi-noach commented Jan 11, 2023

shlomi-noach commented Jan 11, 2023

OnlineDDL: 'mysql' strategy, managed by the scheduler, but executed via normal MySQL statements #12027

OnlineDDL: 'mysql' strategy, managed by the scheduler, but executed via normal MySQL statements #12027

Conversation

shlomi-noach commented Jan 2, 2023 • edited Loading

Description

Related Issue(s)

Checklist

Deployment Notes

vitess-bot bot commented Jan 2, 2023 • edited by deepthi Loading

Review Checklist

General

If a new flag is being introduced:

If a workflow is added or modified:

Bug fixes

Non-trivial changes

New/Existing features

Backward compatibility

mattlord Jan 3, 2023

Choose a reason for hiding this comment

shlomi-noach Jan 3, 2023

Choose a reason for hiding this comment

derekperkins left a comment

Choose a reason for hiding this comment

deepthi left a comment

Choose a reason for hiding this comment

shlomi-noach commented Jan 10, 2023

shlomi-noach commented Jan 11, 2023

shlomi-noach commented Jan 11, 2023

shlomi-noach commented Jan 11, 2023

shlomi-noach commented Jan 2, 2023 •

edited

Loading

vitess-bot bot commented Jan 2, 2023 •

edited by deepthi

Loading