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

declarative Online DDL #7725

Merged
merged 25 commits into from
Apr 5, 2021
Merged

Conversation

shlomi-noach
Copy link
Contributor

@shlomi-noach shlomi-noach commented Mar 21, 2021

Description

This PR introduces (partial) declarative schema changes in Online DDL. In a declarative schema change you tell vitess "this is a table's schema, make it so". If the table does not exist, vitess creates it. If the table exists and with a different schema, vitess evaluates the required ALTER statement and migrates the table.

Declarative changes have the property of being idempotent. You may issue the same migration multiple times with no harm done. If vitess find that no change is required, it implicitly markes the migration as successful.

This PR implements partial declarative changes. In a full declarative schema change, one gives their database a list of all tables in a schema. In a partial declarative schema change, we give vitess a single table at a time.

This means:

  • If you want to create a table, ask vitess to CREATE TABLE
  • If you want to modify a table, ask vitess to CREATE TABLE
  • If you want to drop a table, ask vitess to DROP TABLE.

Vitess supports the -declarative flag in dddl_strategy to enable this functionality.

Usage

consider:

set @@ddl_strategy='online -declarative';
create table t(id int primary key); -- table created
create table t(id int primary key, t text); -- table modified
create table t(id int primary key, t text); -- no changes, implicitly successful
create table t(id int primary key, t text, key t_idx(t(32))); -- table modified
drop table t; -- table dropped
drop table t; -- no changes, implicitly successful

you may use -declarative in all online DDL strategies: online, gh-ost, pt-osc.

Documentation to follow.

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

Deployment Notes

Impacted Areas in Vitess

Components that this PR will affect:

  • Query Serving
  • VReplication
  • Cluster Management
  • Build/CI
  • VTAdmin

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

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>
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>
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>
…into online-ddl-idempotent-tests

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
…t for the new table. Some minor refactor of reading MySQL variables

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
Copy link
Contributor Author

shlomi-noach commented Mar 21, 2021

Reviewers: the main changes to review are in executor.go:

  • new evaluateDeclarativeDiff() function
  • and, within executeMigration() function

when we handle a new migration request, we now first check to see if it's declarative. If so, we may end up actually running a modified migration than defined. We break down into the following cases:

  • it's a revert (no special behavior)
  • alter is not allowed
  • drop: silently mark as successful if the table does not exist. Otherwise treat that as a normal drop
  • create: either
    • the table does not exist -> proceed as normal create
    • the table exists -> use tengo to evaluate the diff statement from existing table into desired table. Either:
      • there is no diff (exact same schema): silently mark as successful
      • there is a diff: modify into an alter statement, to be handled later on.

Diffing the table via tengo:

  • tengo is a library which uses sqlx to connect to to the database
  • we have slightly modified tengo, https://github.com/planetscale/tengo
  • we create an account and connection for sqlx & tengo
  • we create a temporary table within the tablet server's schema (keyspace), which has the new design
  • we diff a the specific original table with this temporary table
    • tengo doesn't like diffing tables of different names, because it assumes identical names in different schemas. We hack around by rewriting tengo's analysis of the table
  • the diff is a SQL string, which is the ALTER options (e.g add column i int, drop key k), or empty if the tables are identical

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

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

@shlomi-noach Awesome! I'm not going to review the implementation unless you'd like me to. From a perspective of a user of the functionality, it looks perfect.

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

@rohit-nayak-ps rohit-nayak-ps left a comment

Choose a reason for hiding this comment

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

lgtm. This is really nice!

// - is it CREATE / DROP / ALTER?
// - it is a Revert request?
// - what's the migration strategy?
// The function invokes th eappropriate handlers for each of those cases.
Copy link
Contributor

Choose a reason for hiding this comment

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

th eappropriate handlers => the appropriate handlers

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

if exists {
// table does exist, so this declarative DROP turns out to really be an actual DROP. No further action is needed here
} else {
// table does not exist. We mark this CREATE as implicitly sucessful
Copy link
Contributor

Choose a reason for hiding this comment

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

should be DROP here and not CREATE?

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>
@shlomi-noach shlomi-noach merged commit 95ca0f8 into vitessio:master Apr 5, 2021
@shlomi-noach shlomi-noach deleted the online-ddl-idempotent branch April 5, 2021 05:24
@askdba askdba added this to the v10.0 milestone Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants