diff --git a/go/test/endtoend/onlineddl/revert/onlineddl_revert_test.go b/go/test/endtoend/onlineddl/revert/onlineddl_revert_test.go index ff65afb3d58..bc94c6d6450 100644 --- a/go/test/endtoend/onlineddl/revert/onlineddl_revert_test.go +++ b/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") diff --git a/go/vt/vttablet/onlineddl/executor.go b/go/vt/vttablet/onlineddl/executor.go index d9421009b37..67e88917673 100644 --- a/go/vt/vttablet/onlineddl/executor.go +++ b/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 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) diff --git a/go/vt/vttablet/onlineddl/schema.go b/go/vt/vttablet/onlineddl/schema.go index d2887851398..8f110965551 100644 --- a/go/vt/vttablet/onlineddl/schema.go +++ b/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