[ritojoao]

Related issue: #1773

Description

This PR drops the _gho table when --attempt-instant-ddl succeeds.

initiateApplier creates and alters the ghost table before Migrate() tries the instant DDL. When the instant ALTER works, the ghost table is never swapped in, and finalCleanup() only drops it on --noop, so _gho was left behind after every successful instant migration. The fix drops it on the instant path, with retryOperation like the other cleanup steps, before finalCleanup().

It adds TestMigrateInstantDDLDropsGhostTable, which runs an instant migration and checks that both _gho and _ghc are gone.

Note on testing: on current master the instant DDL path hangs (#1795), so the new test times out there before reaching its assertions. I verified this change on top of #1796:

• without the fix, the new test fails on the _gho check;
• with the fix, the whole TestMigrator suite passes (go test -run TestMigrator ./go/logic, MySQL 8.0.42 via testcontainers);
• gofmt -l, go vet ./go/logic and go build ./... are clean.

The change itself does not touch the code #1796 changes, so the two merge independently; this PR's CI will hang until #1796 lands. Happy to rebase once it does.

• contributed code is using same conventions as original code
• script/cibuild returns with no formatting errors, build errors or unit test errors (verified on top of Do not deadlock when instant DDL completes the migration #1796, see above).

[Copilot]

🟡 Changes recommended

The new test hits a known instant-DDL deadlock on the current base, causing CI to time out before cleanup assertions or remaining tests can run; the deadlock fix must be incorporated before merging.

1 open issue

• Fix instant-DDL deadlock before merging the test · New

What changed in this PR

Fixes #1773 by removing the unused ghost table after a successful instant DDL migration.

Changes:

• Drops _gho with retry handling before final cleanup on the instant-DDL success path.
• Adds a regression test checking the added column and removal of both _gho and _ghc.

File	Description
go/​logic/​migrator.go	Adds ghost-table cleanup after successful instant DDL.
go/​logic/​migrator_test.go	Adds coverage for instant-DDL table cleanup.

_{Review effort: Balanced (auto)}

[ritojoao]

Thanks for the review, and good catch. I reproduced it: on this branch alone, TestMigrateInstantDDLDropsGhostTable hangs on the instant-DDL success path until the go test timeout, which is the deadlock from #1795. With the fix from #1796 applied underneath, the test passes (0.3s, with -race).

The change in Migrate() itself is independent of that fix, so I'd like to keep the test, since it covers the real path. I'm moving this PR to draft until #1796 is merged, then I'll rebase and re-run the suite. If you'd rather merge this one first, I'm happy to restructure the test, just let me know. Thanks!

[ritojoao]

Hi, thanks for your time. I rebased this onto the current master and reworked the test.

The test calls Migrate() end to end on the --attempt-instant-ddl path, against a real MySQL 8.0.42. On that path nothing receives from ghostTableMigrated, so the streamer's send in onChangelogStateEvent blocks and finalCleanup hangs when it closes the streamer. This happens with and without this change, so I kept it out of scope here. The test drains that channel in a small goroutine so it can reach the assertion.

With the fix, the test passes in about 0.25s. With the DropTable call removed, it fails on the assertion that _testing_gho should not exist.

Happy to open a separate issue or PR for the ghostTableMigrated hang if you would like. Please tell me if you would rather handle it differently here.