strong guards (some panics) against any calls to .OriginalTableName or things like "GetTargetTableName" in move-tables mode

Author: zacharysierakowski

go/base/context.go

@@ -554,6 +554,9 @@ func getSafeTableName(baseName string, suffix string) string {
 // GetGhostTableName generates the name of ghost table, based on original table name
 // or a given table name
 func (mctx *MigrationContext) GetGhostTableName() string {
+	if mctx.IsMoveTablesMode() {
+		panic("GetGhostTableName() must not be called in move-tables mode; there is no ghost table (the target keeps each migrated table's name)")
+	}
 	if mctx.Revert {
 		// When reverting the "ghost" table is the _del table from the original migration.
 		return mctx.OldTableName
@@ -567,8 +570,12 @@ func (mctx *MigrationContext) GetGhostTableName() string {
 
 // GetTargetTableName generates the name of the target table. In move-tables mode
 // each table keeps its own name on the target, so there is no single target
-// table name; per-table code uses MoveTable.TargetTableName instead.
+// table name; per-table code uses MoveTable.TargetTableName instead, and calling
+// this is a programmer error that panics to fail fast.
 func (mctx *MigrationContext) GetTargetTableName() string {
+	if mctx.IsMoveTablesMode() {
+		panic("GetTargetTableName() must not be called in move-tables mode; use MoveTable.TargetTableName")
+	}
 	return mctx.GetGhostTableName()
 }
 
@@ -583,6 +590,9 @@ func (mctx *MigrationContext) GetTargetDatabaseName() string {
 
 // GetOldTableName generates the name of the "old" table, into which the original table is renamed.
 func (mctx *MigrationContext) GetOldTableName() string {
+	if mctx.IsMoveTablesMode() {
+		panic("GetOldTableName() must not be called in move-tables mode; use MoveTableDelName(tableName) for each migrated table's `_<table>_del` rollback handle")
+	}
 	var tableName string
 	if mctx.ForceTmpTableName != "" {
 		tableName = mctx.ForceTmpTableName
@@ -624,6 +634,9 @@ func (mctx *MigrationContext) MoveTableDelName(tableName string) string {
 // GetChangelogTableName generates the name of changelog table, based on original table name
 // or a given table name.
 func (mctx *MigrationContext) GetChangelogTableName() string {
+	if mctx.IsMoveTablesMode() {
+		panic("GetChangelogTableName() must not be called in move-tables mode; there is no changelog table (§1.2)")
+	}
 	if mctx.ForceTmpTableName != "" {
 		return getSafeTableName(mctx.ForceTmpTableName, "ghc")
 	} else {
@@ -644,12 +657,6 @@ func (mctx *MigrationContext) GetCheckpointTableName() string {
 	return getSafeTableName(mctx.OriginalTableName, "ghk")
 }
 
-// GetVoluntaryLockName returns a name of a voluntary lock to be used throughout
-// the swap-tables process.
-func (mctx *MigrationContext) GetVoluntaryLockName() string {
-	return fmt.Sprintf("%s.%s.lock", mctx.DatabaseName, mctx.OriginalTableName)
-}
-
 // RequiresBinlogFormatChange is `true` when the original binlog format isn't `ROW`
 func (mctx *MigrationContext) RequiresBinlogFormatChange() bool {
 	return mctx.OriginalBinlogFormat != "ROW"

go/logic/applier.go

@@ -280,10 +280,15 @@ func (apl *Applier) AcquireMigrationLock(ctx context.Context) error {
 	// One advisory lock per run. In move-tables mode it is keyed on the
 	// set-derived run token (not any single table) so two processes moving the
 	// same set of tables collide, while a single-table run keeps its table-keyed
-	// lock name.
-	lockTable := apl.originalTableName()
+	// lock name. lockSubject is a human-readable description used in contention
+	// errors; neither branch consults the representative table accessor.
+	var lockTable, lockSubject string
 	if apl.migrationContext.IsMoveTablesMode() {
 		lockTable = "movetables." + apl.migrationContext.MoveTablesRunToken()
+		lockSubject = fmt.Sprintf("tables %v", apl.migrationContext.MoveTables.TableNames)
+	} else {
+		lockTable = apl.originalTableName()
+		lockSubject = fmt.Sprintf("`%s`.`%s`", apl.migrationContext.DatabaseName, apl.originalTableName())
 	}
 	lockName := buildMigrationLockName(apl.migrationContext.GetTargetDatabaseName(), lockTable)
 
@@ -322,11 +327,11 @@ func (apl *Applier) AcquireMigrationLock(ctx context.Context) error {
 		conn.Close()
 		lockDB.Close()
 		if holderID.Valid {
-			return fmt.Errorf("another gh-ost process is already migrating `%s`.`%s`: migration lock %s held by connection id %d",
-				apl.migrationContext.DatabaseName, apl.originalTableName(), lockName, holderID.Int64)
+			return fmt.Errorf("another gh-ost process is already migrating %s: migration lock %s held by connection id %d",
+				lockSubject, lockName, holderID.Int64)
 		}
-		return fmt.Errorf("another gh-ost process is already migrating `%s`.`%s`: migration lock %s is held",
-			apl.migrationContext.DatabaseName, apl.originalTableName(), lockName)
+		return fmt.Errorf("another gh-ost process is already migrating %s: migration lock %s is held",
+			lockSubject, lockName)
 	}
 
 	apl.migrationLockConn = conn
@@ -598,7 +603,14 @@ func (apl *Applier) tableExists(tableName string) (tableFound bool) {
 	return (m != nil)
 }
 
+// originalTableName returns the single migrated table. It is a representative
+// accessor that has no meaning in move-tables mode (every table is handled
+// through its own MoveTable container), so calling it there is a programmer
+// error and panics to fail fast rather than silently operate on the wrong table.
 func (apl *Applier) originalTableName() string {
+	if apl.migrationContext.IsMoveTablesMode() {
+		panic("applier.originalTableName() must not be called in move-tables mode; use the per-table MoveTable container instead")
+	}
 	return apl.migrationContext.OriginalTableName
 }
 

go/logic/applier_test.go

@@ -481,19 +481,31 @@ func (suite *ApplierTestSuite) TestFinalCleanupMoveTablesMode_SkipsDrops() {
 }
 
 // initiateStreaming() requires a binlog-capable MySQL connection to call directly.
-// This test verifies IsMoveTablesMode() and that GetChangelogTableName() returns
-// a derivable name. A new streamer always starts with zero listeners; the real
-// proof that no changelog listener is registered comes from the full run not
-// failing on a nonexistent _ghc table.
+// This test verifies IsMoveTablesMode() and that no changelog table is referenced
+// in move-tables mode (§1.2): no `_ghc` table exists on the source or target
+// database. A new streamer always starts with zero listeners; the real proof that
+// no changelog listener is registered comes from the full run not failing on a
+// nonexistent _ghc table.
 func (suite *ApplierTestSuite) TestInitiateStreamingMoveTablesMode_NoChangelogListener() {
+	ctx := context.Background()
 	migrationContext := newTestMigrationContext()
 	migrationContext.MoveTables.TableNames = []string{testMysqlTableName}
 	migrationContext.MoveTables.TargetDatabase = testMysqlDatabaseOther
 
 	suite.Require().True(migrationContext.IsMoveTablesMode())
 
-	changelogTableName := migrationContext.GetChangelogTableName()
-	suite.Require().NotEmpty(changelogTableName, "changelog table name should be derivable")
+	// In move-tables mode there is no changelog table. Verify none exists on
+	// either the source or target database (LIKE '%\_ghc' matches a literal
+	// trailing "_ghc").
+	for _, schema := range []string{testMysqlDatabase, testMysqlDatabaseOther} {
+		var count int
+		err := suite.db.QueryRowContext(ctx,
+			`SELECT COUNT(*) FROM information_schema.tables WHERE table_schema = ? AND table_name LIKE '%\_ghc'`,
+			schema,
+		).Scan(&count)
+		suite.Require().NoError(err)
+		suite.Require().Equal(0, count, "no changelog (_ghc) table should exist in move-tables mode in schema %s", schema)
+	}
 
 	streamer := NewEventsStreamer(migrationContext)
 	suite.Require().Empty(streamer.listeners, "new streamer should have no listeners")

go/logic/hooks.go

@@ -221,15 +221,32 @@ func NewHooksExecutor(migrationContext *base.MigrationContext) *HooksExecutor {
 func (he *HooksExecutor) applyEnvironmentVariables(extraVariables ...string) []string {
 	env := os.Environ()
 	env = append(env, fmt.Sprintf("GH_OST_DATABASE_NAME=%s", he.migrationContext.DatabaseName))
-	tableNameEnv := he.migrationContext.OriginalTableName
+
+	var tableNameEnv string
 	if he.migrationContext.IsMoveTablesMode() {
-		// No representative table: report the full migrated set (target names equal
-		// source names in move-tables mode).
 		tableNameEnv = strings.Join(he.migrationContext.MoveTables.TableNames, ",")
+	} else {
+		tableNameEnv = he.migrationContext.OriginalTableName
 	}
 	env = append(env, fmt.Sprintf("GH_OST_TABLE_NAME=%s", tableNameEnv))
-	env = append(env, fmt.Sprintf("GH_OST_GHOST_TABLE_NAME=%s", he.migrationContext.GetGhostTableName()))
-	env = append(env, fmt.Sprintf("GH_OST_OLD_TABLE_NAME=%s", he.migrationContext.GetOldTableName()))
+	var ghostTableNameEnv string
+	var oldTableNameEnv string
+	if he.migrationContext.IsMoveTablesMode() {
+		// No ghost or old tables in move-tables mode: the destination keeps each
+		// source table's name, and the rollback handles are the per-table
+		// `_<table>_del` tables produced by the atomic cutover RENAME.
+		ghostTableNameEnv = strings.Join(he.migrationContext.MoveTables.TableNames, ",")
+		delNames := make([]string, 0, len(he.migrationContext.MoveTables.TableNames))
+		for _, tableName := range he.migrationContext.MoveTables.TableNames {
+			delNames = append(delNames, he.migrationContext.MoveTableDelName(tableName))
+		}
+		oldTableNameEnv = strings.Join(delNames, ",")
+	} else {
+		ghostTableNameEnv = he.migrationContext.GetGhostTableName()
+		oldTableNameEnv = he.migrationContext.GetOldTableName()
+	}
+	env = append(env, fmt.Sprintf("GH_OST_GHOST_TABLE_NAME=%s", ghostTableNameEnv))
+	env = append(env, fmt.Sprintf("GH_OST_OLD_TABLE_NAME=%s", oldTableNameEnv))
 	env = append(env, fmt.Sprintf("GH_OST_DDL=%s", he.migrationContext.AlterStatement))
 	env = append(env, fmt.Sprintf("GH_OST_ELAPSED_SECONDS=%f", he.migrationContext.ElapsedTime().Seconds()))
 	env = append(env, fmt.Sprintf("GH_OST_ELAPSED_COPY_SECONDS=%f", he.migrationContext.ElapsedRowCopyTime().Seconds()))
@@ -266,13 +283,14 @@ func (he *HooksExecutor) applyEnvironmentVariables(extraVariables ...string) []s
 		env = append(env, fmt.Sprintf("GH_OST_TABLES=%s", strings.Join(he.migrationContext.MoveTables.TableNames, ",")))
 	}
 	env = append(env, fmt.Sprintf("GH_OST_TARGET_DATABASE_NAME=%s", he.migrationContext.GetTargetDatabaseName()))
-	targetTableNameEnv := he.migrationContext.GetTargetTableName()
+
+	var targetTableNameEnv string
 	if he.migrationContext.IsMoveTablesMode() {
-		// Target tables keep their source names; there is no single ghost table.
 		targetTableNameEnv = strings.Join(he.migrationContext.MoveTables.TableNames, ",")
+	} else {
+		targetTableNameEnv = he.migrationContext.GetTargetTableName()
 	}
 	env = append(env, fmt.Sprintf("GH_OST_TARGET_TABLE_NAME=%s", targetTableNameEnv))
-
 	env = append(env, extraVariables...)
 	return env
 }

go/logic/inspect.go

@@ -130,6 +130,9 @@ func (isp *Inspector) InspectOriginalTable() (err error) {
 }
 
 func (isp *Inspector) originalTableName() string {
+	if isp.migrationContext.IsMoveTablesMode() {
+		panic("inspector.originalTableName() must not be called in move-tables mode; inspect each table via its name (e.g. validateTableFor/InspectMoveTable)")
+	}
 	return isp.migrationContext.OriginalTableName
 }
 

go/logic/server.go

@@ -192,6 +192,33 @@ func (srv *Server) onServerCommand(command string, writer *bufio.Writer) (err er
 	return srv.migrationContext.Log.Errore(err)
 }
 
+// commandArgMatchesMigration reports whether a table-name argument supplied with
+// an interactive command refers to this migration. The argument is optional and
+// acts as a courtesy safety check, so an operator who is connected to the wrong
+// gh-ost socket is rejected. In standard mode it must equal the single migrated
+// table; in move-tables mode it may be any one of the migrated tables.
+func (srv *Server) commandArgMatchesMigration(arg string) bool {
+	if srv.migrationContext.IsMoveTablesMode() {
+		for _, tableName := range srv.migrationContext.MoveTables.TableNames {
+			if arg == tableName {
+				return true
+			}
+		}
+		return false
+	}
+	return arg == srv.migrationContext.OriginalTableName
+}
+
+// migrationTargetDescription returns a human-readable description of the migrated
+// table(s), used in interactive-command messages. In move-tables mode it is the
+// comma-joined list of migrated tables; otherwise the single table name.
+func (srv *Server) migrationTargetDescription() string {
+	if srv.migrationContext.IsMoveTablesMode() {
+		return strings.Join(srv.migrationContext.MoveTables.TableNames, ",")
+	}
+	return srv.migrationContext.OriginalTableName
+}
+
 // applyServerCommand parses and executes commands by user
 func (srv *Server) applyServerCommand(command string, writer *bufio.Writer) (printStatusRule PrintStatusRule, err error) {
 	tokens := strings.SplitN(command, "=", 2)
@@ -387,9 +414,9 @@ help                                 # This message
 		}
 	case "throttle", "pause", "suspend":
 		{
-			if arg != "" && arg != srv.migrationContext.OriginalTableName {
+			if arg != "" && !srv.commandArgMatchesMigration(arg) {
 				// User explicitly provided table name. This is a courtesy protection mechanism
-				err := fmt.Errorf("user commanded 'throttle' on %s, but migrated table is %s; ignoring request", arg, srv.migrationContext.OriginalTableName)
+				err := fmt.Errorf("user commanded 'throttle' on %s, but migrated table is %s; ignoring request", arg, srv.migrationTargetDescription())
 				return NoPrintStatusRule, err
 			}
 			atomic.StoreInt64(&srv.migrationContext.ThrottleCommandedByUser, 1)
@@ -398,9 +425,9 @@ help                                 # This message
 		}
 	case "no-throttle", "unthrottle", "resume", "continue":
 		{
-			if arg != "" && arg != srv.migrationContext.OriginalTableName {
+			if arg != "" && !srv.commandArgMatchesMigration(arg) {
 				// User explicitly provided table name. This is a courtesy protection mechanism
-				err := fmt.Errorf("user commanded 'no-throttle' on %s, but migrated table is %s; ignoring request", arg, srv.migrationContext.OriginalTableName)
+				err := fmt.Errorf("user commanded 'no-throttle' on %s, but migrated table is %s; ignoring request", arg, srv.migrationTargetDescription())
 				return NoPrintStatusRule, err
 			}
 			atomic.StoreInt64(&srv.migrationContext.ThrottleCommandedByUser, 0)
@@ -425,9 +452,9 @@ help                                 # This message
 				err := fmt.Errorf("user commanded 'unpostpone' without specifying table name, but --force-named-cut-over is set")
 				return NoPrintStatusRule, err
 			}
-			if arg != "" && arg != srv.migrationContext.OriginalTableName {
+			if arg != "" && !srv.commandArgMatchesMigration(arg) {
 				// User explicitly provided table name. This is a courtesy protection mechanism
-				err := fmt.Errorf("user commanded 'unpostpone' on %s, but migrated table is %s; ignoring request", arg, srv.migrationContext.OriginalTableName)
+				err := fmt.Errorf("user commanded 'unpostpone' on %s, but migrated table is %s; ignoring request", arg, srv.migrationTargetDescription())
 				return NoPrintStatusRule, err
 			}
 			if atomic.LoadInt64(&srv.migrationContext.IsPostponingCutOver) > 0 {
@@ -444,9 +471,9 @@ help                                 # This message
 				err := fmt.Errorf("user commanded 'panic' without specifying table name, but --force-named-panic is set")
 				return NoPrintStatusRule, err
 			}
-			if arg != "" && arg != srv.migrationContext.OriginalTableName {
+			if arg != "" && !srv.commandArgMatchesMigration(arg) {
 				// User explicitly provided table name. This is a courtesy protection mechanism
-				err := fmt.Errorf("user commanded 'panic' on %s, but migrated table is %s; ignoring request", arg, srv.migrationContext.OriginalTableName)
+				err := fmt.Errorf("user commanded 'panic' on %s, but migrated table is %s; ignoring request", arg, srv.migrationTargetDescription())
 				return NoPrintStatusRule, err
 			}
 			err := fmt.Errorf("user commanded 'panic'. The migration will be aborted without cleanup. Please drop the gh-ost tables before trying again")

go/logic/throttler.go

@@ -193,12 +193,15 @@ func (thlr *Throttler) collectControlReplicasLag() {
 		return
 	}
 
-	replicationLagQuery := fmt.Sprintf(`
-		select value from %s.%s where hint = 'heartbeat' and id <= 255
-		`,
-		sql.EscapeName(thlr.migrationContext.DatabaseName),
-		sql.EscapeName(thlr.migrationContext.GetChangelogTableName()),
-	)
+	var replicationLagQuery string
+	if !thlr.migrationContext.IsMoveTablesMode() {
+		replicationLagQuery = fmt.Sprintf(`
+			select value from %s.%s where hint = 'heartbeat' and id <= 255
+			`,
+			sql.EscapeName(thlr.migrationContext.DatabaseName),
+			sql.EscapeName(thlr.migrationContext.GetChangelogTableName()),
+		)
+	}
 
 	readReplicaLag := func(connectionConfig *mysql.ConnectionConfig) (lag time.Duration, err error) {
 		dbUri := connectionConfig.GetDBUri("information_schema")