-
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
OnlineDDL: Revert for VReplication based migrations #7478
OnlineDDL: Revert for VReplication based migrations #7478
Conversation
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
…evert Signed-off-by: Shlomi Noach <[email protected]>
…cessful migration on the table, and that there's no pending migrations on that table Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Convenience link to track changes on top of |
Signed-off-by: Shlomi Noach <[email protected]>
…tatements Signed-off-by: Shlomi Noach <[email protected]>
…evert Signed-off-by: Shlomi Noach <[email protected]>
…not originally exist Signed-off-by: Shlomi Noach <[email protected]>
…R and DROP Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
…-retries is impractical Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Ready for reviewThis PR offers lossless revert for online DDL. Revert is supported for: CREATE TABLE and DROP TABLE using any online
|
Signed-off-by: Shlomi Noach <[email protected]>
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.
LGTM!! 🚀
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.
this is brilliant :-)
@@ -0,0 +1,776 @@ | |||
/* | |||
Copyright 2019 The Vitess Authors. |
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.
copyright: 2021?
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.
fixed
testSelectTableMetrics(t) | ||
}) | ||
t.Run("fail revert older change", func(t *testing.T) { | ||
// We shouldn't be able to revert one-before-last succcessfulk 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.
succcessfulk=>successful
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.
fixed
fmt.Println("# 'vtctlclient OnlineDDL show recent' output (for debug purposes):") | ||
fmt.Println(result) | ||
assert.Equal(t, len(clusterInstance.Keyspaces[0].Shards), strings.Count(result, uuid)) | ||
// We ensure "full word" regexp becuase some column names may conflict |
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.
becuase=>because
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.
fixed
go/vt/schema/online_ddl.go
Outdated
|
||
// GetRevertUUID works when this migration is a revert for another migration. It returns the UUID | ||
// fo the reverted migration. | ||
// The functio nreturns error when this is not a revert 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.
functio nreturns =>function returns an
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.
fixed
Signed-off-by: Shlomi Noach <[email protected]>
Ready for review
This PR offers lossless revert for online DDL. Revert is supported for:
CREATE TABLE and DROP TABLE using any online
ddl_strategy
(eitheronline
,gh-ost
,pt-osc
):CREATE TABLE
: the revert is toDROP
, but actually implemented byRENAME...
CREATE TABLE IF NOT EXISTS
: if theCREATE
was a noop, then so is the revertDROP TABLE
: online DDL implementsDRPO
viaRENANE
. Revert "creates" the table by renaming it back into placeDROP TABLE IF EXISTS
: if theDROP
was a noop then so is the revertALTER TABLE using
ddl_strategy=online
Only VReplication-based
ALTER
migrations are revertible.Any reverted migration
It is possible to revert reverted migration, reverted-reverted migrations, and it's turtles all the way down.
Eligible migrations
On top of the above constraints, you may only revert a migration if:
At this time we only support revert for the last migration, which means you cannot revert the migrations 2nd before last, and you can't "pop" the stack of migrations. However, notice that when you revert a migraiton, the revert itself is a (hopefully) succesful migration, and you may revert the revert.
What's in a revert?
A revert is an online schema migration of its own. You may submit a revert via
vtctl OnlineDDL <keyspace> revert <uuid>
. This generates a new UUID, which is the job ID for the new revert migration.Revert migrations are queued and scheduled same as any other migration. They can be tracked, cancelled and retried same as any other migration.
A revert migration is indicated by a SQL statement of the form
revert fb697374_7b25_11eb_85f4_f875a4d24e90
. This statement is internal and not supported inVTGate
or in any MySQL connection. Work in progress for specialized SQL syntax.It's important to note that a revert migration does not modify the entry for the migration it reverts. It reads the details of the reverted migration from
_vt.schema_migrations
, and, since it is a migration in itself, has its own entry in that table.Implementation
The revert implementation for
CREATE
andDROP
is relatively simple, in the form of counter-queries that revert the effects of the original migraiton. A revert never actuallyCREATE
s a table, and never actuallyDROP
s a table. It's implemented behind the scenes byRENAME
s. There is special logic to handle the case forCREATE TABLE IF NOT EXISTS
andDROP TABLE IF EXISTS
.The implementation for
ALTER
, withddl_strategy='online'
is more elaborate, and includes:ALTER
migration_vt.vreplication
entry from the to-be-revertedALTER
migrationpos
from the to-be-reverted VReplication stream. Thatpos
was taken while tables were swapped and while no writes took place on the table.We use the
artifacts
value in_vt.schema_migration
to identify what the original table was.Lossless
The user has 24 hours to revert a migration without losing data.
DROP
, the table reappears with all data intact.ALTER
, VReplication makes sure to apply any changes made to the table following the migration, so that you do not lose data.VARCHAR(10)
toVARCHAR(20)
. If the new table was populated with now longer texts, then it is irrevertible.Tests
Online DDL Revert is tested via a specialize
endtoend
test. See below to get a sense of what gets tested:Points of interest for the reviewer
go/test/endtoend/onlineddl_revert/onlineddl_revert_test.go
is theendtoend
test, and should give some clarity on general behavior.go/vt/vtctl/vtctl.go
(https://github.com/vitessio/vitess/pull/7478/files#diff-ad37010accc5a29c6751d18124328bb882457e8b2527db688b854c81fe23861c), you can see how arevert <UUID>
is constructed, and sent just as if it were a normal migration.go/vt/vttablet/onlineddl/executor.go
, https://github.com/vitessio/vitess/pull/7478/files#diff-ad37010accc5a29c6751d18124328bb882457e8b2527db688b854c81fe23861cinitVreplicationOriginalMigration()
, https://github.com/vitessio/vitess/pull/7478/files#diff-059c9f46e8d270d9c5514ef2b08679035eb0daaa8d95074e34ef43a81d50dc37R584 is where we init a vreplication DDL for a newALTER
(this code existsed before but now refactored into a separate function), and:initVreplicationRevertMigration()
, https://github.com/vitessio/vitess/pull/7478/files#diff-059c9f46e8d270d9c5514ef2b08679035eb0daaa8d95074e34ef43a81d50dc37R605 is where we inita vreplication DDL which is a revert for a previous VReplication migration. This is where we read the details of the soon-to-be-reverted migration, and speicifcally table & pos.validateMigrationRevertible()
, https://github.com/vitessio/vitess/pull/7478/files#diff-059c9f46e8d270d9c5514ef2b08679035eb0daaa8d95074e34ef43a81d50dc37R1310, is th elogic where we verify the migration we revert is the last successful one on the given table, etc.executeRevert()
, https://github.com/vitessio/vitess/pull/7478/files#diff-059c9f46e8d270d9c5514ef2b08679035eb0daaa8d95074e34ef43a81d50dc37R1376, is where all reverts happen. ForALTER
, the function quickly delegates to vreplication-specific handling. ForCREATE
andDROP
the logic is within this function. There's a lot of logic to handle edge cases,IF EXISTS
,IF NOT EXISTS
etc.Original comment, when this PR was just WIP:
This PR extends #7419 . It only makes sense to review it after #7419 is reviewed & merged.
This experimental PR introduces revert for Online DDL via VReplicaiton. With this PR it is possible to revert a successfully completed DDL performed by VReplication on a table (where this was tha last migration executed on said table). The revert operation may take place for as long as the old table is still available, which is now set to
24h
.This is still evolving, more writeup coming later.
Related Issue(s)
Checklist
Deployment Notes
Impacted Areas in Vitess
Components that this PR will affect: