Skip to content

Fixes the cutover rename+gtid selection and drain logic#1707

Merged
zacharysierakowski merged 3 commits into
womoruyi/move-tables-1.5-cutoverfrom
move-tables-1.5-cutover-improvements
Jun 12, 2026
Merged

Fixes the cutover rename+gtid selection and drain logic#1707
zacharysierakowski merged 3 commits into
womoruyi/move-tables-1.5-cutoverfrom
move-tables-1.5-cutover-improvements

Conversation

@zacharysierakowski

@zacharysierakowski zacharysierakowski commented Jun 11, 2026

Copy link
Copy Markdown

re: https://github.com/github/database-infrastructure/issues/8209
targets: #1704

Description

Applies some suggestions and fixes on top of #1704.

Details

  • fixes the rename+gtid selection so that it's on the same connection as mentioned in the spec doc
  • fixes drain logic to also check the queue so it doesn't exit the cutover too early
  • reuses CutOverLockTimeoutSeconds instead of using a new, hard-coded constant (defaults to 60 seconds still)
  • makes --postpone-cut-over-flag-file required for --move-tables
  • adds helper script for inserting rows to the source continuously

@zacharysierakowski zacharysierakowski self-assigned this Jun 11, 2026
@zacharysierakowski zacharysierakowski added the feature-move-tables PRs that are associated with the new move-tables feature label Jun 11, 2026
@zacharysierakowski zacharysierakowski marked this pull request as ready for review June 12, 2026 01:06
Copilot AI review requested due to automatic review settings June 12, 2026 01:06
@zacharysierakowski zacharysierakowski merged commit 732581d into womoruyi/move-tables-1.5-cutover Jun 12, 2026
6 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens correctness and operator-safety of move-tables cooperative cutover by ensuring rename+GTID capture are connection-pinned, drain completion also accounts for queued work, and move-tables runs are gated by an explicit postpone flag file (with a move-tables-appropriate cutover timeout default).

Changes:

  • Pin T1 (RENAME) + T2 (GTID capture) to a single DB connection and improve T3 drain completion logic by also checking streamer/apply backlogs.
  • Make --postpone-cut-over-flag-file required for --move-tables, and default --cut-over-lock-timeout-seconds to 60s in move-tables mode (while preserving explicit user overrides).
  • Update local move-tables scripts/docs and add a helper script to continuously insert rows on the source primary.
Show a summary per file
File Description
script/move-tables/setup Forces local replicas to be read-only to prevent accidental replica-only mutations and to fail cutover rename when pointed at a replica.
script/move-tables/README.md Updates example invocation for move-tables to use primary and require postpone flag file.
script/move-tables/insert-source-primary-loop Adds a helper script to generate continuous source-side writes for testing drain/cutover behavior.
go/logic/migrator.go Updates move-tables orchestration: earlier flag-file creation, T1/T2 pinning, drain timeout reuse, and backlog-aware drain completion.
go/logic/migrator_move_tables_cutover_test.go Adjusts drain-timeout test to use CutOverLockTimeoutSeconds and adds coverage for “queued DML prevents premature drain success”.
go/cmd/gh-ost/main.go Enforces required postpone flag for move-tables and sets move-tables default cutover-lock-timeout to 60s unless user specified.
go/base/context.go Allows CutOverLockTimeoutSeconds up to 60s in move-tables mode.
go/base/context_test.go Adds tests asserting timeout range differs by mode (standard vs move-tables).

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 8/8 changed files
  • Comments generated: 1

Comment thread go/logic/migrator.go
Comment on lines +998 to +1002
pinnedConn, err := mgtr.inspector.db.Conn(context.Background())
if err != nil {
return fmt.Errorf("failed to pin connection for T1/T2: %w", err)
}
defer pinnedConn.Close()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature-move-tables PRs that are associated with the new move-tables feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants