Skip to content

Commit 4aecc93

Browse files
committed
fix(move-tables): address Copilot review comments on #1704 (#8209)
- Add mgtr.checkAbort() at top of T3 drain poll loop so abort/panic stops the drain immediately instead of waiting the full 60s timeout - Assert NoError on CREATE DATABASE IF NOT EXISTS in applier_test.go SetupSuite (was silently discarding the error with _, _ =) - Rewrite T2 comment to frame @@GLOBAL.gtid_executed as a readability divergence (making scope unambiguous in SQL) rather than asserting session-vs-global semantics that may not hold in MySQL 8.0 Ref: database-infrastructure#8209
1 parent 54652ab commit 4aecc93

2 files changed

Lines changed: 8 additions & 4 deletions

File tree

go/logic/applier_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,8 @@ func (suite *ApplierTestSuite) SetupSuite() {
301301
suite.Require().NoError(err)
302302

303303
// Second database & connection for move-tables tests:
304-
_, _ = suite.db.ExecContext(ctx, fmt.Sprintf("CREATE DATABASE IF NOT EXISTS %s", testMysqlDatabaseOther))
304+
_, err = suite.db.ExecContext(ctx, fmt.Sprintf("CREATE DATABASE IF NOT EXISTS %s", testMysqlDatabaseOther))
305+
suite.Require().NoError(err)
305306
otherConf := drivermysql.NewConfig()
306307
otherConf.DBName = testMysqlDatabaseOther
307308
otherConf.User = testMysqlUser

go/logic/migrator.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -989,9 +989,9 @@ func (mgtr *Migrator) moveTablesCutOver() (err error) {
989989
}
990990

991991
// ----- 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).
992+
// The design doc specifies @@gtid_executed. We query @@GLOBAL.gtid_executed explicitly rather than the unqualified
993+
// @@gtid_executed form to make the global server-wide scope unambiguous
994+
// in the SQL itself.
995995
// Design: https://github.com/github/gh-ost-tablemove-poc/blob/9dc6df75c4c88ff473906a497836c7518f5614ec/design/coop_cutover.md#32-correctness-verification-for-p4
996996
var drainGTIDStr string
997997
if err := mgtr.inspector.db.QueryRow("select @@global.gtid_executed").Scan(&drainGTIDStr); err != nil {
@@ -1022,6 +1022,9 @@ func (mgtr *Migrator) moveTablesCutOver() (err error) {
10221022
ticker := time.NewTicker(moveTablesCutOverDrainPollInterval)
10231023
defer ticker.Stop()
10241024
for {
1025+
if err := mgtr.checkAbort(); err != nil {
1026+
return err
1027+
}
10251028
mgtr.applier.CurrentCoordinatesMutex.Lock()
10261029
applierCoords := mgtr.applier.CurrentCoordinates
10271030
mgtr.applier.CurrentCoordinatesMutex.Unlock()

0 commit comments

Comments
 (0)