Skip to content

Commit 60acf2c

Browse files
committed
feat(move-tables): implement T0-T6 cooperative cutover protocol (#8209)
Postpone gate (flag file + socket, no heartbeat-lag check), RENAME on the privileged source connection with no retry, drain-GTID capture on the same connection, 100ms GTID-containment drain poll with timeout, CutOverCompleteFlag set before return, on-success hook. Drain GTID persistence is #8210. Hook env vars are #8211. Ref: database-infrastructure#8209
1 parent 2db5332 commit 60acf2c

1 file changed

Lines changed: 129 additions & 51 deletions

File tree

go/logic/migrator.go

Lines changed: 129 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,26 @@ import (
2020
"github.com/github/gh-ost/go/binlog"
2121
"github.com/github/gh-ost/go/mysql"
2222
"github.com/github/gh-ost/go/sql"
23+
24+
"github.com/openark/golib/sqlutils"
2325
)
2426

2527
var (
2628
ErrMigratorUnsupportedRenameAlter = errors.New("alter statement seems to RENAME the table. This is not supported, and you should run your RENAME outside gh-ost")
2729
ErrMigrationNotAllowedOnMaster = errors.New("it seems like this migration attempt to run directly on master. Preferably it would be executed on a replica (this reduces load from the master). To proceed please provide --allow-on-master")
2830
RetrySleepFn = time.Sleep
2931
checkpointTimeout = 2 * time.Second
32+
33+
// moveTablesCutOverDrainTimeout caps how long T3 of the move-tables
34+
// cooperative cutover (#8209) waits for the applier to catch up to the
35+
// drain GTID captured at T2. 60s is a starting default chosen over
36+
// CutOverLockTimeoutSeconds (which is capped at 1-10s by
37+
// SetCutOverLockTimeoutSeconds and would be too short for a real drain
38+
// under load). Up for review with Daniel/Eric in the PR.
39+
moveTablesCutOverDrainTimeout = 60 * time.Second
40+
// moveTablesCutOverDrainPollInterval is the per-iteration sleep in T3's
41+
// drain poll. 100ms per move_table_mode.md §1.5.
42+
moveTablesCutOverDrainPollInterval = 100 * time.Millisecond
3043
)
3144

3245
type ChangelogState string
@@ -893,9 +906,6 @@ func (mgtr *Migrator) MoveTables() (err error) {
893906
if err := mgtr.finalCleanup(); err != nil {
894907
return nil
895908
}
896-
if err := mgtr.hooksExecutor.OnSuccess(false); err != nil {
897-
return err
898-
}
899909
mgtr.migrationContext.Log.Infof("Done moving tables %v from %s to %s (%s)",
900910
mgtr.migrationContext.MoveTables.TableNames, sql.EscapeName(mgtr.migrationContext.DatabaseName),
901911
sql.EscapeName(mgtr.migrationContext.MoveTables.TargetDatabase), mgtr.migrationContext.MoveTables.TargetHost)
@@ -915,69 +925,137 @@ func (mgtr *Migrator) MoveTables() (err error) {
915925
// single-server assumption that no longer holds when the applier writes target
916926
// and the streamer reads source. Each is replaced or dropped here.
917927
//
918-
// This commit (1 of 3) lays out the protocol as labeled comment blocks only.
919-
// The protocol logic (RENAME, GTID capture, drain poll, flag set, hooks) lands
920-
// in commit 2. Tests land in commit 3.
928+
// Crash safety (persisting the drain GTID before T3) is #8210. Enriched hook
929+
// env vars (GH_OST_DRAIN_GTID, GH_OST_TARGET_*) are #8211. Target-side
930+
// throttling is #8212. None of those are wired here.
921931
func (mgtr *Migrator) moveTablesCutOver() (err error) {
922932
if mgtr.migrationContext.Noop {
923933
mgtr.migrationContext.Log.Debugf("Noop operation; not really moving tables")
924934
return nil
925935
}
926936

927937
// ----- Postpone gate (precedes T0) -----
928-
// Reuses sleepWhileTrue with a predicate that KEEPS the postpone-flag-file
929-
// branch from standard cutOver() (migrator.go:965-984) and DROPS the
930-
// heartbeat-lag branch (lines 958-964). The heartbeat-lag check is dropped
931-
// because move-tables mode disables _ghc writes (#8206), so
932-
// TimeSinceLastHeartbeatOnChangelog() returns ~58 years (time.Since(zero))
933-
// and would deadlock the gate forever.
934-
// TODO(#8209 commit 2): implement postpone gate (flag-file + unpostpone
935-
// socket command, with OnBeginPostponed firing once).
938+
// Mirrors standard cutOver()'s sleepWhileTrue postpone structure but DROPS the
939+
// heartbeat-lag branch: move-tables mode disables _ghc heartbeat writes (#8206),
940+
// so TimeSinceLastHeartbeatOnChangelog() returns ~58 years (time.Since(zero))
941+
// and would deadlock the gate forever. KEEPS the postpone-flag-file +
942+
// unpostpone-socket gate because per coop_cutover.md §1.1 P4, operator-removes-
943+
// postpone is the trigger for the entire cutover phase.
944+
mgtr.migrationContext.Log.Debugf("checking for cut-over postpone")
945+
if err := mgtr.sleepWhileTrue(func() (bool, error) {
946+
if mgtr.migrationContext.PostponeCutOverFlagFile == "" {
947+
return false, nil
948+
}
949+
if atomic.LoadInt64(&mgtr.migrationContext.UserCommandedUnpostponeFlag) > 0 {
950+
atomic.StoreInt64(&mgtr.migrationContext.UserCommandedUnpostponeFlag, 0)
951+
return false, nil
952+
}
953+
if base.FileExists(mgtr.migrationContext.PostponeCutOverFlagFile) {
954+
if atomic.LoadInt64(&mgtr.migrationContext.IsPostponingCutOver) == 0 {
955+
if err := mgtr.hooksExecutor.OnBeginPostponed(); err != nil {
956+
return true, err
957+
}
958+
}
959+
atomic.StoreInt64(&mgtr.migrationContext.IsPostponingCutOver, 1)
960+
return true, nil
961+
}
962+
return false, nil
963+
}); err != nil {
964+
return err
965+
}
966+
atomic.StoreInt64(&mgtr.migrationContext.IsPostponingCutOver, 0)
967+
mgtr.migrationContext.Log.Debugf("checking for cut-over postpone: complete")
936968

937969
// ----- T0: on-before-cut-over hook -----
938-
// Fires gh-ost-on-before-cut-over* via mgtr.hooksExecutor.OnBeforeCutOver().
939-
// Non-zero hook exit aborts cutover BEFORE any source DDL.
940-
// TODO(#8209 commit 2): implement T0 hook call with error propagation.
970+
// Non-zero hook exit aborts cutover BEFORE any source DDL fires.
971+
if err := mgtr.hooksExecutor.OnBeforeCutOver(); err != nil {
972+
return fmt.Errorf("on-before-cut-over hook failed: %w", err)
973+
}
941974

942975
// ----- T1: RENAME source table -----
943-
// Connection: mgtr.inspector.db (privileged source connection per
944-
// move_table_mode.md §1.5; same connection as T2 so the RENAME's own GTID
945-
// is visible in @@gtid_executed when T2 reads it).
946-
// Exec: sqlutils.ExecNoPrepare with no retry — RENAME is not idempotent;
947-
// a partial success leaves the table already renamed and a retry would
948-
// fail. On error: return wrapped err; operator re-runs the whole hook.
949-
// SQL shape: RENAME TABLE `<source_db>`.`<table>` TO `<source_db>`.`_<table>_del`
950-
// TODO(#8209 commit 2): implement RENAME via inspector.db.
951-
952-
// ----- T2: capture @@gtid_executed -----
953-
// Same connection as T1. Parse the result into a *mysql.GTIDBinlogCoordinates
954-
// (the drain GTID — a closed superset of every committed source write to
955-
// the migrated table, per coop_cutover.md §3.2 steps 1-3).
956-
// TODO(#8209 commit 2): implement GTID capture on the inspector connection.
976+
// Issued on mgtr.inspector.db (the privileged source connection per
977+
// move_table_mode.md §1.5). No retry: RENAME is not idempotent — a partial
978+
// success leaves the table already renamed and a retry would fail. The
979+
// operator re-runs the whole hook chain on failure.
980+
sourceDB := mgtr.migrationContext.DatabaseName
981+
sourceTable := mgtr.migrationContext.OriginalTableName
982+
delTable := mgtr.migrationContext.GetOldTableName()
983+
renameQuery := fmt.Sprintf("RENAME TABLE %s.%s TO %s.%s",
984+
sql.EscapeName(sourceDB), sql.EscapeName(sourceTable),
985+
sql.EscapeName(sourceDB), sql.EscapeName(delTable))
986+
mgtr.migrationContext.Log.Infof("T1: renaming source table: %s", renameQuery)
987+
if _, err := sqlutils.ExecNoPrepare(mgtr.inspector.db, renameQuery); err != nil {
988+
return fmt.Errorf("RENAME failed: %w", err)
989+
}
990+
991+
// ----- T2: capture @@gtid_executed on the SAME *sql.DB handle as T1 -----
992+
// Design doc specifies @@gtid_executed; using @@global.gtid_executed because
993+
// the unqualified form resolves to session scope (only this session's GTIDs),
994+
// while drain requires the global GTID set (all committed transactions).
995+
// Design: https://github.com/github/gh-ost-tablemove-poc/blob/9dc6df75c4c88ff473906a497836c7518f5614ec/design/coop_cutover.md#32-correctness-verification-for-p4
996+
var drainGTIDStr string
997+
if err := mgtr.inspector.db.QueryRow("select @@global.gtid_executed").Scan(&drainGTIDStr); err != nil {
998+
return fmt.Errorf("drain GTID capture failed: %w", err)
999+
}
1000+
drainGTID, err := mysql.NewGTIDBinlogCoordinates(drainGTIDStr)
1001+
if err != nil {
1002+
return fmt.Errorf("drain GTID parse failed: %w", err)
1003+
}
1004+
mgtr.migrationContext.Log.Infof("T2: captured drain GTID: %s", drainGTID.DisplayString())
9571005

9581006
// ----- T3: drain poll -----
959-
// Loop ~100ms ticks (per move_table_mode.md §1.5) until
960-
// applier.CurrentCoordinates contains the drain GTID, comparing via
961-
// GTIDBinlogCoordinates.SmallerThan (go/mysql/binlog_gtid.go). Reads of
962-
// CurrentCoordinates MUST hold CurrentCoordinatesMutex (applier.go:75).
963-
// Timeout after CutOverLockTimeoutSeconds; on timeout return an error
964-
// naming the drain.
965-
// TODO(#8209 commit 2): implement drain poll with mutex-protected reads.
1007+
// Wait until applier.CurrentCoordinates catches up to drainGTID. The drain
1008+
// is complete when the applier's coords are not strictly smaller than the
1009+
// drain target (i.e. the applier contains every GTID in drainGTID). Reads
1010+
// of CurrentCoordinates hold the mutex per applier.go:75. Per-iteration
1011+
// logging is Debug only to avoid spamming Info on a hot loop.
1012+
//
1013+
// Timeout is a named constant (moveTablesCutOverDrainTimeout = 60s). NOTE:
1014+
// the internalization doc suggested reusing CutOverLockTimeoutSeconds, but
1015+
// that's capped at 1-10s (context.go:SetCutOverLockTimeoutSeconds) — too
1016+
// short for a real drain under load. 60s default is up for review with
1017+
// Daniel/Eric; flagged in the PR description.
1018+
mgtr.migrationContext.Log.Infof("T3: draining applier to drain GTID (timeout %s, poll %s)",
1019+
moveTablesCutOverDrainTimeout, moveTablesCutOverDrainPollInterval)
1020+
drainCtx, cancel := context.WithTimeout(context.Background(), moveTablesCutOverDrainTimeout)
1021+
defer cancel()
1022+
ticker := time.NewTicker(moveTablesCutOverDrainPollInterval)
1023+
defer ticker.Stop()
1024+
for {
1025+
mgtr.applier.CurrentCoordinatesMutex.Lock()
1026+
applierCoords := mgtr.applier.CurrentCoordinates
1027+
mgtr.applier.CurrentCoordinatesMutex.Unlock()
1028+
if applierCoords != nil && !applierCoords.IsEmpty() && !applierCoords.SmallerThan(drainGTID) {
1029+
mgtr.migrationContext.Log.Infof("T3: drain complete; applier caught up to drain GTID")
1030+
break
1031+
}
1032+
mgtr.migrationContext.Log.Debugf("T3: applier still behind drain GTID, polling")
1033+
select {
1034+
case <-drainCtx.Done():
1035+
return fmt.Errorf("drain poll timed out after %s: applier did not catch up to drain GTID", moveTablesCutOverDrainTimeout)
1036+
case <-ticker.C:
1037+
// next iteration
1038+
}
1039+
}
9661040

9671041
// ----- T4: set CutOverCompleteFlag -----
968-
// atomic.StoreInt64(&mgtr.migrationContext.CutOverCompleteFlag, 1).
969-
// MUST be set BEFORE T5 so the streamer's StreamEvents(canStopStreaming)
970-
// loop can wind down in parallel with the (potentially slow) on-success
971-
// hook. Forgetting this flag has no visible failure mode at the call site
972-
// — the run silently hangs after cutover.
973-
// TODO(#8209 commit 2): implement T4 flag set.
974-
975-
// ----- T5: fire on-success hook -----
976-
// mgtr.hooksExecutor.OnSuccess(false). Hook unlocks user_rw@target via
977-
// db-user-management and flips the write_cutover? feature flag.
978-
// Standard env vars only — GH_OST_DRAIN_GTID + GH_OST_TARGET_* are #8211
979-
// (1.7), not this PR.
980-
// TODO(#8209 commit 2): implement T5 hook call with error propagation.
1042+
// MUST be set before T5 so the streamer's canStopStreaming loop (migrator.go:256)
1043+
// can wind down in parallel with the (potentially slow) on-success hook.
1044+
// Forgetting this has no visible failure at the call site — the run silently
1045+
// hangs after cutover because eventsStreamer.StreamEvents() never returns.
1046+
atomic.StoreInt64(&mgtr.migrationContext.CutOverCompleteFlag, 1)
1047+
mgtr.migrationContext.Log.Debugf("T4: CutOverCompleteFlag set")
1048+
1049+
// ----- T5: on-success hook -----
1050+
// Hook unlocks user_rw@target via db-user-management and flips the
1051+
// write_cutover? feature flag. Standard env vars only — GH_OST_DRAIN_GTID +
1052+
// GH_OST_TARGET_* are #8211 (1.7), not this PR. The pre-protocol placeholder
1053+
// OnSuccess call that used to live in MoveTables() (after finalCleanup) has
1054+
// been removed so the hook fires in the order coop_cutover.md §3.2 step 6
1055+
// requires (T5 between T4 and T6, BEFORE finalCleanup).
1056+
if err := mgtr.hooksExecutor.OnSuccess(false); err != nil {
1057+
return fmt.Errorf("on-success hook failed: %w", err)
1058+
}
9811059

9821060
// ----- T6: return nil -----
9831061
return nil

0 commit comments

Comments
 (0)