[d-bytebase]

Related issue: #1795

Description

A migration that --attempt-instant-ddl completes never returns. The ALTER is
applied, Success! table ... migrated instantly is logged, and then gh-ost
hangs until it is killed.

Migrate() short-circuits on that path — on success it calls finalCleanup(),
runs the OnSuccess hook and returns, so it never reads
<-mgtr.ghostTableMigrated
(migrator.go#L534-L552).
Three goroutines then form a cycle:

1. The changelog listener parks on the GhostTableMigrated send in
   onChangelogStateEvent. ghostTableMigrated is unbuffered and this path
   never reads it. The send is wrapped in base.SendWithContext, which only
   unblocks on context cancellation — and nothing cancels the context here,
   precisely because Migrate() has not returned. The listener runs from
   notifyListeners, which held listenersMutex across that synchronous call,
   so the mutex stayed held for as long as the send was parked.

2. The binlog parser goroutine (go-mysql's BinlogSyncer.onStream) calls
   EventsStreamer.shouldDecodeRowsEvent through RowsEventDecodeFunc and
   blocks taking that same listenersMutex.

3. BinlogSyncer.Close(), reached from finalCleanup() →
   EventsStreamer.Close(), ends in b.wg.Wait() and waits for that parser
   goroutine, which is never coming back.

Step 2 is what made the cycle reachable. shouldDecodeRowsEvent was introduced
by #1699 ("Don't parse binlog events of unrelated tables", first released in
v1.1.11); it put listenersMutex on the binlog parser's goroutine, so a listener
parked while holding that mutex now stalls the parser directly, and with it
BinlogSyncer.Close().

--attempt-instant-ddl defaults to off and the short-circuit had no test
coverage, which is presumably why this has gone unnoticed. It was found by a
downstream consumer that passes the flag for every migration, where every ALTER
MySQL could apply instantly hung.

The change

• go/logic/streamer.go: notifyListeners collects the matching listeners under
  listenersMutex and invokes them after releasing it. A synchronous listener can
  block indefinitely; the mutex is only needed for the lookup.
• go/logic/migrator.go: ghostTableMigrated gets a one-slot buffer. The
  changelog carries exactly one GhostTableMigrated state per migration — it is
  written once, in initiateApplier — so one slot is enough, and the send no
  longer parks when nobody reads it.

Either of those two on its own clears the hang. Both are kept because they fix
different halves of it: the buffer stops the listener parking at all, and scoping
the mutex stops any blocked listener from stalling the binlog parser and
BinlogSyncer.Close().

• go/logic/migrator.go: the instant-DDL path also sets CutOverCompleteFlag
  before finalCleanup(), as the revert path already does. canStopStreaming
  reads that flag, so without it StreamEvents treated the close that
  finalCleanup performs as an unexpected error and reconnected after its sleep,
  leaking a replication connection and goroutine past the point where Migrate()
  had already returned nil. This only became reachable once the migration stopped
  hanging; it was caught in review.

Test

TestMigrator/TestMigrateInstantDDLReturns sets AttemptInstantDDL with an
ADD COLUMN alter and fails if Migrate() has not returned within 60s, then
asserts the column is actually there. On timeout it cancels the migration context
and waits for the worker first, so a regression does not leave a blocked
Migrate() holding connections and locks for the rest of the suite.

With the two production changes reverted and only the test applied:

    migrator_test.go:1337:
        	Error:      	Migrate did not return after the instant DDL succeeded
        	Test:       	TestMigrator/TestMigrateInstantDDLReturns
--- FAIL: TestMigrator (82.11s)
    --- FAIL: TestMigrator/TestMigrateInstantDDLReturns (60.11s)

The subtest takes 60.11s rather than the full 60s + 30s bound, i.e. the cancel
unwinds the cycle and the worker returns in ~0.1s.

With the full change:

--- PASS: TestMigrator/TestMigrateInstantDDLReturns (0.36s)

go test -count=1 -v -race -covermode=atomic ./go/... passes in all six packages
with no data races, and script/cibuild is clean.

In case this PR introduced Go code changes:

• contributed code is using same conventions as original code
• script/cibuild returns with no formatting errors, build errors or unit test errors.

[Copilot]

🟡 Changes recommended

Instant-migration cleanup can leak replication connections because the cutover flag remains unset, while the deadlock regression test can leave a blocked worker holding locks and connections that disrupt subsequent tests.

2 open issues

• Set the cutover flag before instant-migration cleanup · New
• Cancel and join the migration goroutine before failing · New

What changed in this PR

Fixes a deadlock that prevents gh-ost from returning after a successful instant-DDL migration.

Changes:

• Invoke matching binlog listeners after releasing the listener mutex.
• Buffer the migration-completion signal so an unread notification does not block the listener.
• Add an integration test that checks migration completion within a timeout and verifies the schema change.

File	Description
go/​logic/​streamer.go	Separates listener lookup under the mutex from callback execution.
go/​logic/​migrator.go	Adds a one-slot buffer to the migration-completion channel.
go/​logic/​migrator_test.go	Adds regression coverage for instant-DDL migration completion.

_{Review effort: Max (auto)}