Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions go/logic/migrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -3218,13 +3218,18 @@ func (mgtr *Migrator) moveTablesFinalCleanup() error {
targetDatabaseName := mgtr.migrationContext.GetTargetDatabaseName()
checkpointTableName := mgtr.migrationContext.GetCheckpointTableName()

if mgtr.migrationContext.OkToDropTable {
// The source `__del` rollback handle only exists after a real cutover,
// never in Noop runs. It must be dropped on the source primary: the
// inspector/streamer source connections may point at a read replica, so the
// drop goes through the dedicated source-primary handle.
if !mgtr.migrationContext.Noop {
if err := mgtr.retryOperation(mgtr.dropMoveTablesSourceOldTables); err != nil {
// A resumed noop reuses the target tables and checkpoint from the interrupted
// migration, so it must preserve both. A fresh noop creates target tables and
// a checkpoint only for schema validation, so it must remove those artifacts
// regardless of --ok-to-drop-table.
if mgtr.migrationContext.Noop {
if mgtr.migrationContext.Resume {
return nil
}
for _, mt := range mgtr.migrationContext.OrderedMoveTables() {
Comment thread
ericyan marked this conversation as resolved.
if err := mgtr.retryOperation(func() error {
return mgtr.applier.dropTable(mt.TargetTableName)
}); err != nil {
return err
}
}
Expand All @@ -3236,7 +3241,19 @@ func (mgtr *Migrator) moveTablesFinalCleanup() error {
return nil
Comment thread
ericyan marked this conversation as resolved.
}

if mgtr.migrationContext.Noop {
if mgtr.migrationContext.OkToDropTable {
// The source `__del` rollback handle only exists after a real cutover,
// never in Noop runs. It must be dropped on the source primary: the
// inspector/streamer source connections may point at a read replica, so the
// drop goes through the dedicated source-primary handle.
if err := mgtr.retryOperation(mgtr.dropMoveTablesSourceOldTables); err != nil {
return err
}
if mgtr.migrationContext.Checkpoint {
if err := mgtr.retryOperation(mgtr.applier.DropCheckpointTable); err != nil {
return err
}
}
return nil
}

Expand Down
76 changes: 76 additions & 0 deletions go/logic/migrator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,82 @@ func (suite *MigratorTestSuite) TestMoveTablesStateInitializesColumnMetadata() {
})
}

func (suite *MigratorTestSuite) TestMoveTablesNoopDropsTargetTables() {
ctx := context.Background()
targetTableNames := []string{testMysqlTableName, "test_noop_target"}

migrationContext := newTestMigrationContext()
migrationContext.Noop = true
migrationContext.Checkpoint = true
migrationContext.MoveTables.TableNames = targetTableNames
migrationContext.MoveTables.TargetDatabase = testMysqlDatabase
migrationContext.InitMoveTableContainers()

migrator := NewMigrator(migrationContext, "test")
migrator.applier = NewApplier(migrationContext)
migrator.applier.moveTablesTargetDB = suite.db
suite.Require().NoError(migrator.applier.CreateCheckpointTable())

for _, tableName := range targetTableNames {
_, err := suite.db.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s.%s (id INT PRIMARY KEY)",
sql.EscapeName(testMysqlDatabase), sql.EscapeName(tableName)))
suite.Require().NoError(err)
}

suite.Require().NoError(migrator.moveTablesFinalCleanup())

for _, tableName := range targetTableNames {
exists, err := migrator.applier.targetTableExists(tableName)
suite.Require().NoError(err)
suite.Require().False(exists, "noop cleanup must drop target table %s", tableName)
}
checkpointExists, err := migrator.applier.targetTableExists(migrationContext.GetCheckpointTableName())
suite.Require().NoError(err)
suite.Require().False(checkpointExists, "fresh noop cleanup must drop its checkpoint table")
}

func (suite *MigratorTestSuite) TestMoveTablesResumedNoopPreservesTargetTables() {
ctx := context.Background()
targetTableNames := []string{testMysqlTableName, "test_noop_resume_target"}

migrationContext := newTestMigrationContext()
migrationContext.Noop = true
migrationContext.Resume = true
migrationContext.Checkpoint = true
migrationContext.MoveTables.TableNames = targetTableNames
migrationContext.MoveTables.TargetDatabase = testMysqlDatabase
migrationContext.InitMoveTableContainers()

migrator := NewMigrator(migrationContext, "test")
migrator.applier = NewApplier(migrationContext)
migrator.applier.moveTablesTargetDB = suite.db
suite.Require().NoError(migrator.applier.CreateCheckpointTable())

for _, tableName := range targetTableNames {
_, err := suite.db.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s.%s (id INT PRIMARY KEY)",
sql.EscapeName(testMysqlDatabase), sql.EscapeName(tableName)))
suite.Require().NoError(err)
}
defer func() {
for _, tableName := range append(targetTableNames, migrationContext.GetCheckpointTableName()) {
_, err := suite.db.ExecContext(ctx, fmt.Sprintf("DROP TABLE IF EXISTS %s.%s",
sql.EscapeName(testMysqlDatabase), sql.EscapeName(tableName)))
suite.Require().NoError(err)
}
}()

suite.Require().NoError(migrator.moveTablesFinalCleanup())

for _, tableName := range targetTableNames {
exists, err := migrator.applier.targetTableExists(tableName)
suite.Require().NoError(err)
suite.Require().True(exists, "resumed noop cleanup must preserve target table %s", tableName)
}
checkpointExists, err := migrator.applier.targetTableExists(migrationContext.GetCheckpointTableName())
suite.Require().NoError(err)
suite.Require().True(checkpointExists, "resumed noop cleanup must preserve its checkpoint table")
}

func (suite *MigratorTestSuite) TestRetryBatchCopyWithHooks() {
ctx := context.Background()

Expand Down
Loading