pr suggestions

Signed-off-by: Tim Vaillancourt <[email protected]>

Author: timvaillancourt

go/base/context.go

@@ -135,7 +135,6 @@ type MigrationContext struct {
 	CriticalLoadHibernateSeconds        int64
 	PostponeCutOverFlagFile             string
 	CutOverLockTimeoutSeconds           int64
-	CutOverIdleTimeoutSeconds           int64
 	CutOverExponentialBackoff           bool
 	ExponentialBackoffMaxInterval       int64
 	ForceNamedCutOverCommand            bool
@@ -278,7 +277,6 @@ func NewMigrationContext() *MigrationContext {
 		ApplierConnectionConfig:             mysql.NewConnectionConfig(),
 		MaxLagMillisecondsThrottleThreshold: 1500,
 		CutOverLockTimeoutSeconds:           3,
-		CutOverIdleTimeoutSeconds:           6,
 		DMLBatchSize:                        10,
 		etaNanoseonds:                       ETAUnknown,
 		maxLoad:                             NewLoadMap(),

go/cmd/gh-ost/main.go

@@ -108,7 +108,6 @@ func main() {
 	dmlBatchSize := flag.Int64("dml-batch-size", 10, "batch size for DML events to apply in a single transaction (range 1-100)")
 	defaultRetries := flag.Int64("default-retries", 60, "Default number of retries for various operations before panicking")
 	cutOverLockTimeoutSeconds := flag.Int64("cut-over-lock-timeout-seconds", 3, "Max number of seconds to hold locks on tables while attempting to cut-over (retry attempted when lock exceeds timeout)")
-	flag.Int64Var(&migrationContext.CutOverIdleTimeoutSeconds, "cut-over-idle-timeout-seconds", *cutOverLockTimeoutSeconds*2, "the idle timeout in seconds for the cut-over operation. The MySQL session performing the cut-over uses this as a wait_timeout.")
 	niceRatio := flag.Float64("nice-ratio", 0, "force being 'nice', imply sleep time per chunk time; range: [0.0..100.0]. Example values: 0 is aggressive. 1: for every 1ms spent copying rows, sleep additional 1ms (effectively doubling runtime); 0.7: for every 10ms spend in a rowcopy chunk, spend 7ms sleeping immediately after")
 
 	maxLagMillis := flag.Int64("max-lag-millis", 1500, "replication lag at which to throttle operation")

go/logic/applier.go

@@ -937,40 +937,14 @@ func (this *Applier) CreateAtomicCutOverSentryTable() error {
 	return nil
 }
 
-// WaitForAtomicCutOverRename waits for the cut-over RENAME operation while periodically
-// pinging the applier connection to avoid the connection becoming idle.
-func (this *Applier) WaitForAtomicCutOverRename(tx *gosql.Tx, okToUnlockTable <-chan bool) {
-	ticker := time.NewTicker(time.Millisecond * 500)
-	defer ticker.Stop()
-	for {
-		select {
-		case <-okToUnlockTable:
-			this.migrationContext.Log.Infof("Will now proceed to drop magic table and unlock tables")
-			return
-		case <-ticker.C:
-			// keep connection alive as we wait for the RENAME.
-			if _, err := tx.Exec(`select /* gh-ost */ 1`); err != nil {
-				this.migrationContext.Log.Errorf("Failed to ping applier connection: %v", err)
-			}
-		}
-	}
-}
-
 // InitAtomicCutOverIdleTimeout sets the cut-over session wait_timeout, returning a func() to rollback
 // to the original wait_timeout and an error, if any.
-func (this *Applier) InitAtomicCutOverIdleTimeout(tx *gosql.Tx) (restoreTimeoutFunc func(), err error) {
-	this.migrationContext.Log.Infof("Setting cut-over idle timeout as %d seconds", this.migrationContext.CutOverIdleTimeoutSeconds)
-	query := fmt.Sprintf(`set /* gh-ost */ session wait_timeout:=%d`, this.migrationContext.CutOverIdleTimeoutSeconds)
-	_, err = tx.Exec(query)
-	return func() {
-		this.migrationContext.Log.Infof("Restoring applier idle timeout as %d seconds", this.migrationContext.ApplierWaitTimeout)
-		query = fmt.Sprintf(`set /* gh-ost */ session wait_timeout:=%d`, this.migrationContext.ApplierWaitTimeout)
-		if _, err := sqlutils.ExecNoPrepare(this.db, query); err != nil {
-			this.migrationContext.Log.Errorf("Failed to restore applier wait_timeout to %d seconds: %v",
-				this.migrationContext.ApplierWaitTimeout, err,
-			)
-		}
-	}, err
+func (this *Applier) InitAtomicCutOverIdleTimeout(tx *gosql.Tx) error {
+	cutOverIdleTimeoutSeconds := this.migrationContext.CutOverLockTimeoutSeconds * 2
+	this.migrationContext.Log.Infof("Setting cut-over idle timeout as %d seconds", cutOverIdleTimeoutSeconds)
+	query := fmt.Sprintf(`set /* gh-ost */ session wait_timeout:=%d`, cutOverIdleTimeoutSeconds)
+	_, err := tx.Exec(query)
+	return err
 }
 
 // AtomicCutOverMagicLock
@@ -1013,13 +987,17 @@ func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocke
 		return err
 	}
 
-	if this.migrationContext.CutOverIdleTimeoutSeconds >= 1 {
-		restoreIdleTimeoutFunc, err := this.InitAtomicCutOverIdleTimeout(tx)
-		if err != nil {
-			tableLocked <- err
-			return err
-		}
-		defer restoreIdleTimeoutFunc()
+	defer func() {
+		if _, err := sqlutils.ExecNoPrepare(this.db, query); err != nil {
+ 			this.migrationContext.Log.Errorf("Failed to restore applier wait_timeout to %d seconds: %v",
+ 				this.migrationContext.ApplierWaitTimeout, err,
+ 			)
+ 		}
+	}()
+
+	if err := this.InitAtomicCutOverIdleTimeout(tx); err != nil {
+		tableLocked <- err
+		return err
 	}
 
 	if err := this.CreateAtomicCutOverSentryTable(); err != nil {
@@ -1052,7 +1030,8 @@ func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocke
 
 	// The cut-over phase will proceed to apply remaining backlog onto ghost table,
 	// and issue RENAME. We wait here until told to proceed.
-	this.WaitForAtomicCutOverRename(tx, okToUnlockTable)
+	<-okToUnlockTable
+	this.migrationContext.Log.Infof("Will now proceed to drop magic table and unlock tables")
 
 	// The magic table is here because we locked it. And we are the only ones allowed to drop it.
 	// And in fact, we will: