Cherry pick 15.0: OnlineDDL: 'mysql' strategy, managed by the schedul…

…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>

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.
+

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) {

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)

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,
  },

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))