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.

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

Online DDL: ready_to_complete hint column #9813

Merged

Conversation

shlomi-noach
Copy link
Contributor

Description

This PR adds ready_to_complete column in _vt.schema_migrations which is then visible in SHOW VITESS_MIGRATIONS ... output.

The use case is intended for postponed migrations (although it provides visibility for non-postponed migrations as well): if a migration is postponed, the user wants to know "is now a good time to cut-over" ie run a ALTER VITESS_MIGRATION ... COMPLETE and expect a near-immediate cut-over.

ready_to_complete is a boolean (values 0 or 1) that indicates that the migration is generally capable of cutting over. Specifically:

  • CREATE and DROP migrations will always exhibit the value of 1 (immediately or shortly after submission). Note that this is true even if the migrations are still queued. Once they exhibit 1, they stay with 1.
  • vitess ALTER migrations will indicate 1 when row-copy is complete and vplayer's lag is generally good. this value can drop back to 0 when the migration is postponed and vplayer happens to accumulate lag. Once vplayer is caught up again, it will flip back to 1.
  • A REVERT of any of the above behaves similarly: a CREATE REVERT immediately exhibits the value of 1; an ALTER's REVERT exhibits 1 when vplayer's lag is good and 0 when vplayer accumulates lag
  • All the above is also true for postponed migrations
  • gh-ost ALTER migrations indicate 1 when entering postponed state, and does not have lag indication. It is only applicable to postponed-completion migrations.

Note that all changes to onSchemaMigrationStatus are for supporting gh-ost by adding a hint column to gh-ost callback API; it's not the important part of this PR.

Related Issue(s)

Tracking: #6926

Checklist

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

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

@dbussink dbussink left a comment

Choose a reason for hiding this comment

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

Looks like a pretty straightforward way to solve this issue. One small question and found a typo 😄.

"-online_ddl_check_interval", "3s",
"-schema_change_check_interval", "1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't matter, but any reason this order of arguments was changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bedtime story: it turns out the onlineddl_ghost endtoend tests have not been running in a while. There was an exception thrown in this old TestMain function that was not caught (in itself something to investigate). What I did now was to copy+paste a brand new TestMain function from onlineddl_vrepl tests, which works.

Having said that, I'll reorder the arguments just to keep this PR cleaner.

if ddlAction == sqlparser.AlterStr || !postponeCompletion {
// Any non-postponed migration can be scheduled
// postponed ALTER can be scheduled
// We only schedule a single migration i nthe execution of this function
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo here, i nthe instead of in the.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
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 shlomi-noach merged commit 29be10b into vitessio:main Mar 3, 2022
@shlomi-noach shlomi-noach deleted the online-ddl-ready-to-complete branch March 3, 2022 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Query Serving Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants