Introduce a SourcePrimaryConnectionConfig for primary-required move-table ops#1718
Introduce a SourcePrimaryConnectionConfig for primary-required move-table ops#1718zacharysierakowski wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
random script i've had stashed for a while that's been handy between local testing so i don't have to do a full teardown/setup.
happy to remove from this PR if we don't want it merged.
| // handle uses InspectorConnectionConfig (the source) and stays open in both the | ||
| // normal and the cutover-resume paths (Close() only closes the binlog reader, | ||
| // not `db`). | ||
| func (es *EventsStreamer) DropSourceOldTable() error { |
There was a problem hiding this comment.
This was only added recently as part of #1717. I added it knowing that it probably wasn't the right spot for it, but without having the distinction between the primary vs. replica connections (this PR) it was hard to figure out the best spot for this drop operation to dangle from
…al e2e docker setup fix for primary resolution
There was a problem hiding this comment.
Pull request overview
This PR improves --move-tables correctness and safety by introducing a dedicated source-primary connection for primary-required operations (cutover RENAME, drain-GTID capture, and source __del cleanup), while also adding early guards to prevent accidentally running move-tables against non-writable targets or against the source primary for read-heavy phases.
Changes:
- Add a dedicated
sourcePrimaryDB/SourcePrimaryConnectionConfigfor move-tables primary-required operations, including consolidatedRENAME; SELECT @@global.gtid_executedin a single multi-statement round trip. - Add startup guards to fail fast when the move-tables target is
read_only, and to block running the source read path on the source primary unless--allow-on-source-primaryis set. - Update local move-tables scripts/docs to better support primary auto-resolution from a source replica, including
/etc/hostsalias assistance and a new reset helper.
Show a summary per file
| File | Description |
|---|---|
| script/move-tables/setup | Adds best-effort /etc/hosts aliasing so host-side gh-ost can resolve docker-compose service names used by replication metadata. |
| script/move-tables/reset | New helper script to reset source/target tables for local move-tables testing. |
| script/move-tables/README.md | Updates the example invocation to read from the source replica port and includes --initially-drop-socket-file. |
| go/logic/streamer.go | Removes source-side cleanup responsibility from the streamer. |
| go/logic/migrator.go | Introduces source-primary connection/config, consolidated cutover T1/T2 multi-statement, read-path guard, and source __del drop via primary. |
| go/logic/migrator_move_tables_cutover_test.go | Adds/updates unit+integration coverage for source-primary resolution, writability guard, and source-side drop. |
| go/logic/migrator_move_tables_cleanup_test.go | Adds coverage for nil source-primary connection error on cleanup. |
| go/logic/applier.go | Adds a writability guard for move-tables target connections. |
| go/cmd/gh-ost/main.go | Adds --allow-on-source-primary flag wiring. |
| go/base/context.go | Extends move-tables context with AllowOnSourcePrimary and SourcePrimaryConnectionConfig. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 10/10 changed files
- Comments generated: 1
re: https://github.com/github/database-infrastructure/issues/8275
Description
This PR does a few things that were lingering on our backlog.
--allow-on-source-primaryto bypass this guardDemo(s)
Auto resolve source primary from source replica
[Guard] Early exit if target is a replica
[Guard] Early exit if source is a primary and have not specified the new override arg (--allow-on-source-primary)
script/cibuildreturns with no formatting errors, build errors or unit test errors.