Skip to content

Commit

Permalink
implement singleton-table restriction logic, add endtoend tests
Browse files Browse the repository at this point in the history 
Signed-off-by: Shlomi Noach <[email protected]>
  • Loading branch information
@shlomi-noach
shlomi-noach committed Nov 7, 2024
1 parent 9cafba3 commit fc64ff9
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 7 deletions.
27 changes: 21 additions & 6 deletions go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1674,7 +1674,7 @@ DROP TABLE IF EXISTS stress_test
t.Run("terminate throttled migration", func(t *testing.T) {
onlineddl.CheckMigrationStatus(t, &vtParams, shards, openEndedUUID, schema.OnlineDDLStatusRunning)
onlineddl.CheckCancelMigration(t, &vtParams, shards, openEndedUUID, true)
status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, openEndedUUID, 20*time.Second, schema.OnlineDDLStatusFailed, schema.OnlineDDLStatusCancelled)
status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, openEndedUUID, normalWaitTime, schema.OnlineDDLStatusFailed, schema.OnlineDDLStatusCancelled)
fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
onlineddl.CheckMigrationStatus(t, &vtParams, shards, openEndedUUID, schema.OnlineDDLStatusCancelled)
})
Expand Down Expand Up @@ -1707,6 +1707,21 @@ DROP TABLE IF EXISTS stress_test
checkTable(t, tableName, true)
})

// singleton-table
t.Run("fail singleton-table same table single submission", func(t *testing.T) {
_ = testOnlineDDLStatement(t, createParams(multiAlterTableThrottlingStatement, "vitess --singleton-table", "vtctl", "", "hint_col", "singleton-table migration rejected", false))
// The first of those migrations will make it, the other two will be rejected
onlineddl.CheckCancelAllMigrations(t, &vtParams, 1)
})
t.Run("fail singleton-table same table multi submission", func(t *testing.T) {
uuid := testOnlineDDLStatement(t, createParams(alterTableThrottlingStatement, "vitess --singleton-table --postpone-completion", "vtctl", "", "hint_col", "", false))
uuids = append(uuids, uuid)
onlineddl.WaitForMigrationStatus(t, &vtParams, shards, uuid, normalWaitTime, schema.OnlineDDLStatusRunning)

_ = testOnlineDDLStatement(t, createParams(alterTableThrottlingStatement, "vitess --singleton-table --postpone-completion", "vtctl", "", "hint_col", "singleton-table migration rejected", false))
onlineddl.CheckCancelAllMigrations(t, &vtParams, 1)
})

var throttledUUIDs []string
// singleton-context
t.Run("postponed migrations, singleton-context", func(t *testing.T) {
Expand Down Expand Up @@ -1763,29 +1778,29 @@ DROP TABLE IF EXISTS stress_test
uuids = append(uuids, uuid)
_ = testOnlineDDLStatement(t, createParams(dropNonexistentTableStatement, "vitess --singleton", "vtgate", "", "hint_col", "rejected", true))
onlineddl.CheckCompleteAllMigrations(t, &vtParams, len(shards))
status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, uuid, 20*time.Second, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed)
status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, uuid, normalWaitTime, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed)
fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
})
t.Run("fail concurrent singleton-context with revert", func(t *testing.T) {
revertUUID := testRevertMigration(t, createRevertParams(uuids[len(uuids)-1], "vitess --allow-concurrent --postpone-completion --singleton-context", "vtctl", "rev:ctx", "", false))
onlineddl.WaitForMigrationStatus(t, &vtParams, shards, revertUUID, 20*time.Second, schema.OnlineDDLStatusRunning)
onlineddl.WaitForMigrationStatus(t, &vtParams, shards, revertUUID, normalWaitTime, schema.OnlineDDLStatusRunning)
// revert is running
_ = testOnlineDDLStatement(t, createParams(dropNonexistentTableStatement, "vitess --allow-concurrent --singleton-context", "vtctl", "migrate:ctx", "", "rejected", true))
onlineddl.CheckCancelMigration(t, &vtParams, shards, revertUUID, true)
status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, revertUUID, 20*time.Second, schema.OnlineDDLStatusFailed, schema.OnlineDDLStatusCancelled)
status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, revertUUID, normalWaitTime, schema.OnlineDDLStatusFailed, schema.OnlineDDLStatusCancelled)
fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
onlineddl.CheckMigrationStatus(t, &vtParams, shards, revertUUID, schema.OnlineDDLStatusCancelled)
})
t.Run("success concurrent singleton-context with no-context revert", func(t *testing.T) {
revertUUID := testRevertMigration(t, createRevertParams(uuids[len(uuids)-1], "vitess --allow-concurrent --postpone-completion", "vtctl", "rev:ctx", "", false))
onlineddl.WaitForMigrationStatus(t, &vtParams, shards, revertUUID, 20*time.Second, schema.OnlineDDLStatusRunning)
onlineddl.WaitForMigrationStatus(t, &vtParams, shards, revertUUID, normalWaitTime, schema.OnlineDDLStatusRunning)
// revert is running but has no --singleton-context. Our next migration should be able to run.
uuid := testOnlineDDLStatement(t, createParams(dropNonexistentTableStatement, "vitess --allow-concurrent --singleton-context", "vtctl", "migrate:ctx", "", "", false))
uuids = append(uuids, uuid)
onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
onlineddl.CheckCancelMigration(t, &vtParams, shards, revertUUID, true)
status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, revertUUID, 20*time.Second, schema.OnlineDDLStatusFailed, schema.OnlineDDLStatusCancelled)
status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, revertUUID, normalWaitTime, schema.OnlineDDLStatusFailed, schema.OnlineDDLStatusCancelled)
fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
onlineddl.CheckMigrationStatus(t, &vtParams, shards, revertUUID, schema.OnlineDDLStatusCancelled)
})
Expand Down
11 changes: 10 additions & 1 deletion go/vt/vttablet/onlineddl/executor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4869,7 +4869,7 @@ func (e *Executor) submitCallbackIfNonConflicting(
) (
result *sqltypes.Result, err error,
) {
if !onlineDDL.StrategySetting().IsSingleton() && !onlineDDL.StrategySetting().IsSingletonContext() {
if !onlineDDL.StrategySetting().IsSingleton() && !onlineDDL.StrategySetting().IsSingletonContext() && !onlineDDL.StrategySetting().IsSingletonTable() {
// not a singleton. No conflict
return callback()
}
Expand Down Expand Up @@ -4915,6 +4915,15 @@ func (e *Executor) submitCallbackIfNonConflicting(
}
// no conflict? continue looking for other pending migrations
}
case onlineDDL.StrategySetting().IsSingletonTable():
// We will reject this migration if there's any pending migration for the same table
for _, row := range rows {
pendingTableName := row["mysql_table"].ToString()
if onlineDDL.Table == pendingTableName {
pendingUUID := row["migration_uuid"].ToString()
return vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, "singleton-table migration rejected: found pending migration: %s for the same table: %s", pendingUUID, onlineDDL.Table)
}
}
}
return nil
}()
Expand Down

0 comments on commit fc64ff9

Please sign in to comment.