Skip to content

Commit a524303

Browse files
ggilderclaude
andcommitted
Handle context cancellation while waiting for ghost table migration
Migrate() blocks on an unbuffered receive from ghostTableMigrated while waiting for the ghost table to be created. The only sender is onChangelogStateEvent(), which publishes via base.SendWithContext(). If the migration aborts during this window, abort() cancels the migration context, so SendWithContext() takes its ctx.Done() branch and returns without ever sending. Nothing else writes to the channel, so Migrate() blocks forever: it never returns, its deferred teardown() never runs, finishedMigrating is never set, and the status and throttler tickers -- which exit on finishedMigrating rather than on the context -- keep looping. The process stays alive indefinitely, logging a frozen status line, until it is killed externally. Extract the wait into waitForGhostTableMigrated() and select on the migration context alongside the channel, returning checkAbort() so the original abort error is surfaced rather than a bare context error. The extraction mirrors consumeRowCopyComplete() and makes the behaviour testable without a database. This is the same deadlock, and the same fix, as #1677 applied to consumeRowCopyComplete; ghostTableMigrated is its remaining sibling. TestAbort_DuringGhostTableWait follows the existing TestAbort_* pattern and fails (blocking until its timeout) without this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 2ed192c commit a524303

2 files changed

Lines changed: 76 additions & 2 deletions

File tree

go/logic/migrator.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,22 @@ func (mgtr *Migrator) consumeRowCopyComplete() {
267267
}()
268268
}
269269

270+
// waitForGhostTableMigrated blocks until the ghost table has been migrated, or
271+
// until the migration context is cancelled by an abort. The only sender on
272+
// ghostTableMigrated publishes via base.SendWithContext, which stops sending
273+
// once the context is cancelled, so waiting on the channel alone would block
274+
// forever after an abort.
275+
func (mgtr *Migrator) waitForGhostTableMigrated() error {
276+
select {
277+
case <-mgtr.ghostTableMigrated:
278+
mgtr.migrationContext.Log.Debugf("ghost table migrated")
279+
return nil
280+
case <-mgtr.migrationContext.GetContext().Done():
281+
// Abort cancelled the context
282+
return mgtr.checkAbort()
283+
}
284+
}
285+
270286
func (mgtr *Migrator) canStopStreaming() bool {
271287
return atomic.LoadInt64(&mgtr.migrationContext.CutOverCompleteFlag) != 0
272288
}
@@ -554,8 +570,9 @@ func (mgtr *Migrator) Migrate() (err error) {
554570
initialLag, _ := mgtr.inspector.getReplicationLag()
555571
if !mgtr.migrationContext.Resume {
556572
mgtr.migrationContext.Log.Infof("Waiting for ghost table to be migrated. Current lag is %+v", initialLag)
557-
<-mgtr.ghostTableMigrated
558-
mgtr.migrationContext.Log.Debugf("ghost table migrated")
573+
if err := mgtr.waitForGhostTableMigrated(); err != nil {
574+
return err
575+
}
559576
}
560577
// Yay! We now know the Ghost and Changelog tables are good to examine!
561578
// When running on replica, this means the replica has those tables. When running

go/logic/migrator_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1566,6 +1566,63 @@ func TestAbort_DuringInspection(t *testing.T) {
15661566
}
15671567
}
15681568

1569+
func TestAbort_DuringGhostTableWait(t *testing.T) {
1570+
migrationContext := base.NewMigrationContext()
1571+
migrator := NewMigrator(migrationContext, "1.0.0")
1572+
1573+
// Start listenOnPanicAbort
1574+
go migrator.listenOnPanicAbort()
1575+
1576+
// Give listenOnPanicAbort time to start
1577+
time.Sleep(20 * time.Millisecond)
1578+
1579+
// Simulate an abort raised while Migrate() waits for the ghost table
1580+
testErr := errors.New("ghost table wait aborted")
1581+
go func() {
1582+
time.Sleep(10 * time.Millisecond)
1583+
select {
1584+
case migrationContext.PanicAbort <- testErr:
1585+
case <-migrationContext.GetContext().Done():
1586+
}
1587+
}()
1588+
1589+
// Nothing sends on ghostTableMigrated, mirroring an abort that cancels the
1590+
// context before the changelog event arrives: the real sender publishes via
1591+
// base.SendWithContext, which stops sending once the context is cancelled.
1592+
// Waiting on the channel alone would block here forever.
1593+
done := make(chan error, 1)
1594+
go func() {
1595+
done <- migrator.waitForGhostTableMigrated()
1596+
}()
1597+
1598+
select {
1599+
case err := <-done:
1600+
if err == nil {
1601+
t.Fatal("Expected an error once the abort cancelled the context")
1602+
}
1603+
if err.Error() != "ghost table wait aborted" {
1604+
t.Errorf("Expected 'ghost table wait aborted', got %v", err)
1605+
}
1606+
case <-time.After(5 * time.Second):
1607+
t.Fatal("Expected waitForGhostTableMigrated to return after the abort cancelled the context")
1608+
}
1609+
}
1610+
1611+
func TestWaitForGhostTableMigrated(t *testing.T) {
1612+
migrationContext := base.NewMigrationContext()
1613+
migrator := NewMigrator(migrationContext, "1.0.0")
1614+
1615+
// ghostTableMigrated is unbuffered, so the send must be async
1616+
go func() {
1617+
time.Sleep(10 * time.Millisecond)
1618+
migrator.ghostTableMigrated <- true
1619+
}()
1620+
1621+
if err := migrator.waitForGhostTableMigrated(); err != nil {
1622+
t.Fatalf("Expected no error, got %v", err)
1623+
}
1624+
}
1625+
15691626
func TestAbort_DuringStreaming(t *testing.T) {
15701627
migrationContext := base.NewMigrationContext()
15711628
migrator := NewMigrator(migrationContext, "1.0.0")

0 commit comments

Comments
 (0)