Online DDL: ALTER VIEW to respect --postpone-completion strategy flag

go/test/endtoend/onlineddl/revert/onlineddl_revert_test.go

@@ -347,6 +347,7 @@ func TestSchemaChange(t *testing.T) {
  // ALTER VIEW
  t.Run("ALTER VIEW where view exists", func(t *testing.T) {
  // The view exists
+ checkTable(t, viewName, true)
  uuid := testOnlineDDLStatementForView(t, alterViewStatement, ddlStrategy, "vtgate", "success_alter")
  uuids = append(uuids, uuid)
  onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
@@ -627,7 +628,8 @@ func TestSchemaChange(t *testing.T) {
  checkMigratedTable(t, tableName, alterHints[0])
  testSelectTableMetrics(t)
  })
- t.Run("postponed revert", func(t *testing.T) {
+ testPostponedRevert := func(t *testing.T, expectStatuses ...schema.OnlineDDLStatus) {
+ require.NotEmpty(t, expectStatuses)
  ctx, cancel := context.WithCancel(context.Background())
  defer cancel()
  var wg sync.WaitGroup
@@ -636,22 +638,56 @@ func TestSchemaChange(t *testing.T) {
  defer wg.Done()
  runMultipleConnections(ctx, t)
  }()
- uuid := testRevertMigration(t, uuids[len(uuids)-1], ddlStrategy+" -postpone-completion")
+ uuid := testRevertMigration(t, uuids[len(uuids)-1], ddlStrategy+" --postpone-completion")
  uuids = append(uuids, uuid)
  // Should be still running!
- onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusRunning)
+ onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, expectStatuses...)
  // Issue a complete and wait for successful completion
  onlineddl.CheckCompleteMigration(t, &vtParams, shards, uuid, true)
- // This part may take a while, because we depend on vreplicatoin polling
+ // This part may take a while, because we depend on vreplication polling
  status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, uuid, 60*time.Second, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed)
  fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
  onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
  cancel() // will cause runMultipleConnections() to terminate
  wg.Wait()
+ }
+ t.Run("postponed revert", func(t *testing.T) {
+ testPostponedRevert(t, schema.OnlineDDLStatusRunning)
  checkMigratedTable(t, tableName, alterHints[1])
  testSelectTableMetrics(t)
  })
 
+ t.Run("postponed revert view", func(t *testing.T) {
+ t.Run("CREATE VIEW again", func(t *testing.T) {
+ // The view does not exist
+ uuid := testOnlineDDLStatementForView(t, createViewStatement, ddlStrategy, "vtgate", "success_create")
+ uuids = append(uuids, uuid)
+ onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
+ checkTable(t, viewName, true)
+ testRevertedUUID(t, uuid, "")
+ })
+ t.Run("ALTER VIEW, postpone completion", func(t *testing.T) {
+ // Technically this test better fits in `onlineddl_scheduler_test.go`, but since we've already laid the grounds here, this is where it landed.
+ // The view exists
+ checkTable(t, viewName, true)
+ uuid := testOnlineDDLStatementForView(t, alterViewStatement, ddlStrategy+" --postpone-completion", "vtgate", "success_create")
+ uuids = append(uuids, uuid)
+
+ onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusQueued, schema.OnlineDDLStatusReady)
+ // Issue a complete and wait for successful completion
+ onlineddl.CheckCompleteMigration(t, &vtParams, shards, uuid, true)
+ // This part may take a while, because we depend on vreplication polling
+ status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, uuid, 60*time.Second, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed)
+ fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
+ onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
+ checkTable(t, viewName, true)
+ testRevertedUUID(t, uuid, "")
+ })
+ // now verify that the revert for ALTER VIEW respects `--postpone-completion`
+ testPostponedRevert(t, schema.OnlineDDLStatusQueued, schema.OnlineDDLStatusReady)
+ checkTable(t, viewName, true)
+ })
+
  // INSTANT DDL
  t.Run("INSTANT DDL: add column", func(t *testing.T) {
  uuid := testOnlineDDLStatementForTable(t, "alter table stress_test add column i_instant int not null default 0", ddlStrategy+" --prefer-instant-ddl", "vtgate", "i_instant")

go/vt/vttablet/onlineddl/executor.go

@@ -2145,25 +2145,36 @@ func (e *Executor) scheduleNextMigration(ctx context.Context) error {
  postponeCompletion := row.AsBool("postpone_completion", false)
  readyToComplete := row.AsBool("ready_to_complete", false)
  ddlAction := row["ddl_action"].ToString()
+ isView := row.AsBool("is_view", false)
 
  if postponeLaunch {
  // We don't even look into this migration until its postpone_launch flag is cleared
  continue
  }
 
+ isImmediateDDL := false
+ switch ddlAction {
+ case sqlparser.CreateStr, sqlparser.DropStr:
+ isImmediateDDL = true
+ case sqlparser.AlterStr:
+ if isView {
+ isImmediateDDL = true
+ }
+ }
  if !readyToComplete {
- // Whether postponsed or not, CREATE and DROP operations are inherently "ready to complete"
- // because their operation is instantaneous.
- switch ddlAction {
- case sqlparser.CreateStr, sqlparser.DropStr:
+ // see if we need to update ready_to_complete
+ if isImmediateDDL {
+  // Whether postponsed or not, CREATE and DROP operations, as well as VIEW operations,
+  // are inherently "ready to complete" because their operation is immediate.
  if err := e.updateMigrationReadyToComplete(ctx, uuid, true); err != nil {
  return err
  }
  }
  }
- if ddlAction == sqlparser.AlterStr || !postponeCompletion {
+
+ if !(isImmediateDDL && postponeCompletion) {
  // Any non-postponed migration can be scheduled
- // postponed ALTER can be scheduled
+ // postponed ALTER ALTER can be scheduled (because gh-ost or vreplication will postpone the cut-over)
  // We only schedule a single migration in the execution of this function
  onlyScheduleOneMigration.Do(func() {
  err = e.updateMigrationStatus(ctx, uuid, schema.OnlineDDLStatusReady)

go/vt/vttablet/onlineddl/schema.go

@@ -111,6 +111,7 @@ const (
  sqlSelectQueuedMigrations = `SELECT
  migration_uuid,
  ddl_action,
+ is_view,
  postpone_launch,
  postpone_completion,
  ready_to_complete