From c544e8ad8feb05f8ff460662cc63d2f0efc96a9c Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 23 Jan 2023 11:25:49 +0200 Subject: [PATCH] Cherry pick 15.0: OnlineDDL: 'mysql' strategy, managed by the scheduler, 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): - https://github.com/vitessio/vitess/commit/673573ab5097eb48289ce32d3093fb3d6d2b027a - https://github.com/vitessio/vitess/commit/836b3c1e7df43c1474f0686363a824e7aeefdecc The first commit changed the function signature for testOnlineDDLStatement(), while the later commit used the old signature. Signed-off-by: Matt Lord Signed-off-by: Matt Lord Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Signed-off-by: Matt Lord Co-authored-by: Matt Lord --- doc/releasenotes/16_0_0_release_notes.md | 11 +++++ .../scheduler/onlineddl_scheduler_test.go | 41 +++++++++++++++++++ go/vt/schema/ddl_strategy.go | 8 ++-- go/vt/schema/ddl_strategy_test.go | 5 +++ go/vt/vttablet/onlineddl/executor.go | 26 +++++++++++- 5 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 doc/releasenotes/16_0_0_release_notes.md diff --git a/doc/releasenotes/16_0_0_release_notes.md b/doc/releasenotes/16_0_0_release_notes.md new file mode 100644 index 00000000000..c66877e8ca2 --- /dev/null +++ b/doc/releasenotes/16_0_0_release_notes.md @@ -0,0 +1,11 @@ +# Release of Vitess v16.0.0 +## Major Changes + +### Online DDL + +Introducing 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](https://dev.mysql.com/doc/refman/8.0/en/innodb-online-ddl-operations.html). In particular, consider that for non-`INSTANT` DDLs, replicas will accumulate substantial lag. + diff --git a/go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go b/go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go index f7cfc0f6fd3..73e1fa5bb50 100644 --- a/go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go +++ b/go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go @@ -763,6 +763,47 @@ func TestSchemaChange(t *testing.T) { }) }) } + // 'mysql' strategy + t.Run("mysql strategy", func(t *testing.T) { + t.Run("declarative", func(t *testing.T) { + t1uuid = testOnlineDDLStatement(t, createParams(createT1Statement, "mysql --declarative", "vtgate", "just-created", "", false)) + + status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, t1uuid, normalWaitTime, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed) + fmt.Printf("# Migration status (for debug purposes): <%s>\n", status) + onlineddl.CheckMigrationStatus(t, &vtParams, shards, t1uuid, schema.OnlineDDLStatusComplete) + checkTable(t, t1Name, true) + }) + + t.Run("fail postpone-completion", func(t *testing.T) { + t1uuid := testOnlineDDLStatement(t, createParams(trivialAlterT1Statement, "mysql --postpone-completion", "vtgate", "", "", true)) + + // --postpone-completion not supported in mysql strategy + time.Sleep(ensureStateNotChangedTime) + onlineddl.WaitForMigrationStatus(t, &vtParams, shards, t1uuid, normalWaitTime, schema.OnlineDDLStatusFailed) + onlineddl.CheckMigrationStatus(t, &vtParams, shards, t1uuid, schema.OnlineDDLStatusFailed) + }) + t.Run("trivial", func(t *testing.T) { + t1uuid := testOnlineDDLStatement(t, createParams(trivialAlterT1Statement, "mysql", "vtgate", "", "", true)) + + status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, t1uuid, normalWaitTime, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed) + fmt.Printf("# Migration status (for debug purposes): <%s>\n", status) + onlineddl.CheckMigrationStatus(t, &vtParams, shards, t1uuid, schema.OnlineDDLStatusComplete) + + rs := onlineddl.ReadMigrations(t, &vtParams, t1uuid) + require.NotNil(t, rs) + for _, row := range rs.Named().Rows { + artifacts := row.AsString("artifacts", "-") + assert.Empty(t, artifacts) + } + }) + t.Run("instant", func(t *testing.T) { + t1uuid := testOnlineDDLStatement(t, createParams(instantAlterT1Statement, "mysql", "vtgate", "", "", true)) + + status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, t1uuid, normalWaitTime, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed) + fmt.Printf("# Migration status (for debug purposes): <%s>\n", status) + onlineddl.CheckMigrationStatus(t, &vtParams, shards, t1uuid, schema.OnlineDDLStatusComplete) + }) + }) } func TestSingleton(t *testing.T) { diff --git a/go/vt/schema/ddl_strategy.go b/go/vt/schema/ddl_strategy.go index c5b38a6fb03..247a0c8d364 100644 --- a/go/vt/schema/ddl_strategy.go +++ b/go/vt/schema/ddl_strategy.go @@ -46,7 +46,7 @@ const ( type DDLStrategy string const ( - // DDLStrategyDirect means not an online-ddl migration. Just a normal MySQL ALTER TABLE + // DDLStrategyDirect means not an online-ddl migration; unmanaged. Just a normal MySQL `ALTER TABLE` DDLStrategyDirect DDLStrategy = "direct" // DDLStrategyVitess requests vreplication to run the migration; new name for DDLStrategyOnline DDLStrategyVitess DDLStrategy = "vitess" @@ -56,13 +56,15 @@ const ( DDLStrategyGhost DDLStrategy = "gh-ost" // DDLStrategyPTOSC requests pt-online-schema-change to run the migration DDLStrategyPTOSC DDLStrategy = "pt-osc" + // DDLStrategyMySQL is a managed migration (queued and executed by the scheduler) but runs through a MySQL `ALTER TABLE` + DDLStrategyMySQL DDLStrategy = "mysql" ) // IsDirect returns true if this strategy is a direct strategy // A strategy is direct if it's not explciitly one of the online DDL strategies func (s DDLStrategy) IsDirect() bool { switch s { - case DDLStrategyVitess, DDLStrategyOnline, DDLStrategyGhost, DDLStrategyPTOSC: + case DDLStrategyVitess, DDLStrategyOnline, DDLStrategyGhost, DDLStrategyPTOSC, DDLStrategyMySQL: return false } return true @@ -94,7 +96,7 @@ func ParseDDLStrategy(strategyVariable string) (*DDLStrategySetting, error) { switch strategy := DDLStrategy(strategyName); strategy { case "": // backward compatiblity and to handle unspecified values setting.Strategy = DDLStrategyDirect - case DDLStrategyVitess, DDLStrategyOnline, DDLStrategyGhost, DDLStrategyPTOSC, DDLStrategyDirect: + case DDLStrategyVitess, DDLStrategyOnline, DDLStrategyGhost, DDLStrategyPTOSC, DDLStrategyMySQL, DDLStrategyDirect: setting.Strategy = strategy default: return nil, fmt.Errorf("Unknown online DDL strategy: '%v'", strategy) diff --git a/go/vt/schema/ddl_strategy_test.go b/go/vt/schema/ddl_strategy_test.go index 7ca13b7e5e4..7aab517d846 100644 --- a/go/vt/schema/ddl_strategy_test.go +++ b/go/vt/schema/ddl_strategy_test.go @@ -34,6 +34,7 @@ func TestIsDirect(t *testing.T) { assert.False(t, DDLStrategy("online").IsDirect()) assert.False(t, DDLStrategy("gh-ost").IsDirect()) assert.False(t, DDLStrategy("pt-osc").IsDirect()) + assert.False(t, DDLStrategy("mysql").IsDirect()) assert.True(t, DDLStrategy("something").IsDirect()) } @@ -73,6 +74,10 @@ func TestParseDDLStrategy(t *testing.T) { strategyVariable: "pt-osc", strategy: DDLStrategyPTOSC, }, + { + strategyVariable: "mysql", + strategy: DDLStrategyMySQL, + }, { strategy: DDLStrategyDirect, }, diff --git a/go/vt/vttablet/onlineddl/executor.go b/go/vt/vttablet/onlineddl/executor.go index 76a75a538e1..483aebb9ec1 100644 --- a/go/vt/vttablet/onlineddl/executor.go +++ b/go/vt/vttablet/onlineddl/executor.go @@ -1088,8 +1088,8 @@ func (e *Executor) initMigrationSQLMode(ctx context.Context, onlineDDL *schema.O conn.ExecuteFetch(restoreSQLModeQuery, 0, false) } // Change sql_mode - changeSSQLModeQuery := fmt.Sprintf("set @@session.sql_mode=REPLACE(REPLACE('%s', 'NO_ZERO_DATE', ''), 'NO_ZERO_IN_DATE', '')", sqlMode) - if _, err := conn.ExecuteFetch(changeSSQLModeQuery, 0, false); err != nil { + changeSQLModeQuery := fmt.Sprintf("set @@session.sql_mode=REPLACE(REPLACE('%s', 'NO_ZERO_DATE', ''), 'NO_ZERO_IN_DATE', '')", sqlMode) + if _, err := conn.ExecuteFetch(changeSQLModeQuery, 0, false); err != nil { return deferFunc, err } return deferFunc, nil @@ -2355,12 +2355,25 @@ func (e *Executor) reviewQueuedMigrations(ctx context.Context) error { return err } } + // Find conditions where the migration cannot take place: + switch onlineDDL.Strategy { + case schema.DDLStrategyMySQL: + strategySetting := onlineDDL.StrategySetting() + if strategySetting.IsPostponeCompletion() { + e.failMigration(ctx, onlineDDL, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "--postpone-completion not supported in 'mysql' strategy")) + } + if strategySetting.IsAllowZeroInDateFlag() { + e.failMigration(ctx, onlineDDL, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "--allow-zero-in-date not supported in 'mysql' strategy")) + } + } + // The review is complete. We've backfilled details on the migration row. We mark // the migration as having been reviewed. The function scheduleNextMigration() will then // have access to this row. if err := e.updateMigrationTimestamp(ctx, "reviewed_timestamp", uuid); err != nil { return err } + } return nil } @@ -3063,6 +3076,15 @@ func (e *Executor) executeAlterDDLActionMigration(ctx context.Context, onlineDDL failMigration(err) } }() + case schema.DDLStrategyMySQL: + go func() { + e.migrationMutex.Lock() + defer e.migrationMutex.Unlock() + + if _, err := e.executeDirectly(ctx, onlineDDL); err != nil { + failMigration(err) + } + }() default: { return failMigration(fmt.Errorf("Unsupported strategy: %+v", onlineDDL.Strategy))