reuse CutOverLockTimeoutSeconds for move-tables

Author: zacharysierakowski

go/base/context.go

@@ -508,8 +508,12 @@ func (mctx *MigrationContext) SetCutOverLockTimeoutSeconds(timeoutSeconds int64)
 	if timeoutSeconds < 1 {
 		return fmt.Errorf("minimal timeout is 1sec. Timeout remains at %d", mctx.CutOverLockTimeoutSeconds)
 	}
-	if timeoutSeconds > 10 {
-		return fmt.Errorf("maximal timeout is 10sec. Timeout remains at %d", mctx.CutOverLockTimeoutSeconds)
+	maxTimeout := int64(10)
+	if mctx.IsMoveTablesMode() {
+		maxTimeout = 60
+	}
+	if timeoutSeconds > maxTimeout {
+		return fmt.Errorf("maximal timeout is %dsec. Timeout remains at %d", maxTimeout, mctx.CutOverLockTimeoutSeconds)
 	}
 	mctx.CutOverLockTimeoutSeconds = timeoutSeconds
 	return nil

go/base/context_test.go

@@ -326,3 +326,22 @@ func TestSetAbortError_ThreadSafe(t *testing.T) {
 		t.Errorf("Stored error %v not in list of sent errors", got)
 	}
 }
+
+func TestSetCutOverLockTimeoutSecondsRangeByMode(t *testing.T) {
+	{
+		ctx := NewMigrationContext()
+		require.NoError(t, ctx.SetCutOverLockTimeoutSeconds(10))
+		err := ctx.SetCutOverLockTimeoutSeconds(11)
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "maximal timeout is 10sec")
+	}
+
+	{
+		ctx := NewMigrationContext()
+		ctx.MoveTables.TableNames = []string{"tbl"}
+		require.NoError(t, ctx.SetCutOverLockTimeoutSeconds(60))
+		err := ctx.SetCutOverLockTimeoutSeconds(61)
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "maximal timeout is 60sec")
+	}
+}

go/cmd/gh-ost/main.go

@@ -198,6 +198,12 @@ func main() {
 
 	flag.CommandLine.SetOutput(os.Stdout)
 	flag.Parse()
+	cutOverLockTimeoutUserSpecified := false
+	flag.Visit(func(f *flag.Flag) {
+		if f.Name == "cut-over-lock-timeout-seconds" {
+			cutOverLockTimeoutUserSpecified = true
+		}
+	})
 
 	if *checkFlag {
 		return
@@ -387,6 +393,9 @@ func main() {
 		if migrationContext.MoveTables.TargetDatabase == "" {
 			migrationContext.MoveTables.TargetDatabase = migrationContext.DatabaseName
 		}
+		if !cutOverLockTimeoutUserSpecified {
+			*cutOverLockTimeoutSeconds = 60
+		}
 		migrationContext.MoveTables.ConnectionConfig = mysql.NewConnectionConfig()
 	}
 

go/logic/migrator.go

@@ -28,13 +28,6 @@ var (
 	RetrySleepFn                      = time.Sleep
 	checkpointTimeout                 = 2 * time.Second
 
-	// moveTablesCutOverDrainTimeout caps how long T3 of the move-tables
-	// cooperative cutover (#8209) waits for the applier to catch up to the
-	// drain GTID captured at T2. 60s is a starting default chosen over
-	// CutOverLockTimeoutSeconds (which is capped at 1-10s by
-	// SetCutOverLockTimeoutSeconds and would be too short for a real drain
-	// under load). Up for review with Daniel/Eric in the PR.
-	moveTablesCutOverDrainTimeout = 60 * time.Second
 	// moveTablesCutOverDrainPollInterval is the per-iteration sleep in T3's
 	// drain poll. 100ms per move_table_mode.md §1.5.
 	moveTablesCutOverDrainPollInterval = 100 * time.Millisecond
@@ -1038,15 +1031,10 @@ func (mgtr *Migrator) moveTablesCutOver() (err error) {
 	// drain target (i.e. the applier contains every GTID in drainGTID). Reads
 	// of CurrentCoordinates hold the mutex per applier.go:75. Per-iteration
 	// logging is Debug only to avoid spamming Info on a hot loop.
-	//
-	// Timeout is a named constant (moveTablesCutOverDrainTimeout = 60s). NOTE:
-	// the internalization doc suggested reusing CutOverLockTimeoutSeconds, but
-	// that's capped at 1-10s (context.go:SetCutOverLockTimeoutSeconds) — too
-	// short for a real drain under load. 60s default is up for review with
-	// Daniel/Eric; flagged in the PR description.
+	drainTimeout := time.Duration(mgtr.migrationContext.CutOverLockTimeoutSeconds) * time.Second
 	mgtr.migrationContext.Log.Infof("T3: draining applier to drain GTID (timeout %s, poll %s)",
-		moveTablesCutOverDrainTimeout, moveTablesCutOverDrainPollInterval)
-	drainCtx, cancel := context.WithTimeout(context.Background(), moveTablesCutOverDrainTimeout)
+		drainTimeout, moveTablesCutOverDrainPollInterval)
+	drainCtx, cancel := context.WithTimeout(context.Background(), drainTimeout)
 	defer cancel()
 	ticker := time.NewTicker(moveTablesCutOverDrainPollInterval)
 	defer ticker.Stop()
@@ -1073,7 +1061,7 @@ func (mgtr *Migrator) moveTablesCutOver() (err error) {
 		}
 		select {
 		case <-drainCtx.Done():
-			return fmt.Errorf("drain poll timed out after %s: applier did not catch up to drain GTID", moveTablesCutOverDrainTimeout)
+			return fmt.Errorf("drain poll timed out after %s: applier did not catch up to drain GTID", drainTimeout)
 		case <-ticker.C:
 			// next iteration
 		}

go/logic/migrator_move_tables_cutover_test.go

@@ -281,26 +281,25 @@ func (s *MoveTablesCutOverSuite) TestRenameFailurePropagates() {
 }
 
 // TestDrainTimeoutPropagates maps to T3 timeout handling: applier coordinates
-// never reach the drain GTID -> drain poll bounded by moveTablesCutOverDrainTimeout
+// never reach the drain GTID -> drain poll bounded by CutOverLockTimeoutSeconds
 // returns a wrapped error and the flag is not set.
 func (s *MoveTablesCutOverSuite) TestDrainTimeoutPropagates() {
 	ctx := context.Background()
 	_, err := s.db.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s (id INT PRIMARY KEY)", getTestTableName()))
 	s.Require().NoError(err)
 
-	// Patch the package-level timeout/poll just for this test.
-	origTimeout, origPoll := moveTablesCutOverDrainTimeout, moveTablesCutOverDrainPollInterval
-	moveTablesCutOverDrainTimeout = 200 * time.Millisecond
+	// Patch poll interval and use a 1-second drain timeout for a bounded test.
+	origPoll := moveTablesCutOverDrainPollInterval
 	moveTablesCutOverDrainPollInterval = 50 * time.Millisecond
 	s.T().Cleanup(func() {
-		moveTablesCutOverDrainTimeout = origTimeout
 		moveTablesCutOverDrainPollInterval = origPoll
 	})
 
 	var calls []string
 	fakeHooks := &recordingHooks{name: "fake", calls: &calls}
 	// initialCoords nil - drain comparison never satisfies.
 	m, mc := s.buildMigrator(fakeHooks, nil)
+	mc.CutOverLockTimeoutSeconds = 1
 
 	s.Require().Equal(int64(0), atomic.LoadInt64(&mc.CutOverCompleteFlag), "pre-state: flag must be 0")
 
@@ -327,17 +326,16 @@ func (s *MoveTablesCutOverSuite) TestDrainWaitsForQueuedDML() {
 	_, err := s.db.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s (id INT PRIMARY KEY)", getTestTableName()))
 	s.Require().NoError(err)
 
-	origTimeout, origPoll := moveTablesCutOverDrainTimeout, moveTablesCutOverDrainPollInterval
-	moveTablesCutOverDrainTimeout = 200 * time.Millisecond
+	origPoll := moveTablesCutOverDrainPollInterval
 	moveTablesCutOverDrainPollInterval = 50 * time.Millisecond
 	s.T().Cleanup(func() {
-		moveTablesCutOverDrainTimeout = origTimeout
 		moveTablesCutOverDrainPollInterval = origPoll
 	})
 
 	var calls []string
 	fakeHooks := &recordingHooks{name: "fake", calls: &calls}
 	m, mc := s.buildMigrator(fakeHooks, s.containingDrainGTID())
+	mc.CutOverLockTimeoutSeconds = 1
 	m.applyEventsQueue <- newApplyEventStructByDML(&binlog.BinlogEntry{
 		DmlEvent: &binlog.BinlogDMLEvent{
 			DatabaseName: testMysqlDatabase,