github / github/gh-ost

Deadlock during cleanup when --attempt-instant-ddl succeeds (GhostTableMigrated signal never drained)

Open
#1,735 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
13.6k
Forks
1.4k
Avg merge
2h 31m
Merged PRs (30d)
4

Description

### Overview

When `--attempt-instant-ddl` is used and the instant `ALTER` succeeds, gh-ost can deadlock during final cleanup and hang forever instead of exiting.

### Root cause

`initiateApplier` writes a `GhostTableMigrated` changelog row. The streamer's changelog listener callback (`Migrator.onChangelogStateEvent`) publishes that signal synchronously via `base.SendWithContext(ctx, mgtr.ghostTableMigrated, true)` **while holding `EventsStreamer.listenersMutex`** (the send happens inside `notifyListeners`, which holds the mutex for the duration of the callback).

On the normal migration path there is a dedicated receiver:

```go
if !mgtr.migrationContext.Resume {
<-mgtr.ghostTableMigrated
}
```

But on the instant-DDL success path (`go/logic/migrator.go`), `Migrate()` returns early right after `finalCleanup()` and **never receives** from `ghostTableMigrated`:

```go
if err := mgtr.applier.AttemptInstantDDL(); err == nil {
if err := mgtr.finalCleanup(); err != nil {
return nil
}
...
return nil
}
```

So the changelog send blocks forever, keeping `listenersMutex` held. `finalCleanup()` then closes the binlog reader, whose rows-event decode callback (`EventsStreamer.shouldDecodeRowsEvent`) needs the **same** mutex, and `BinlogSyncer.Close()` waits (via `WaitGroup`) for that goroutine to exit → permanent deadlock. gh-ost hangs and never completes the migration.

### Reproduction

Run an instant-DDL-eligible migration against MySQL 8.0 with `--attempt-instant-ddl`, e.g. adding a column with a default:

```
gh-ost --attempt-instant-ddl --execute \
--alter="ADD COLUMN c INT NOT NULL DEFAULT 1" \
--host=127.0.0.1 --port=3306 --database=db --table=t ...
```

gh-ost applies the instant DDL, logs the "migrated instantly" success, but then hangs in cleanup instead of exiting.

### Proposed fix

Drain the `GhostTableMigrated` signal on the instant-DDL success path before `finalCleanup()`, mirroring the receive already present on the normal path, guarded by `!Resume` (resume migrations never emit the signal). I have a PR ready with the fix plus regression tests.

Contributor guide

Open the contributing guide

Research direction

Start in go/logic/migrator.go, comparing the instant-DDL success path with the normal migration path that receives GhostTableMigrated. Trace initiateApplier, onChangelogStateEvent, and finalCleanup to understand the mutex and channel interaction. Done means instant-DDL migrations exit without hanging, with regression tests covering the signal drain and resume behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, mysql
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.