Skip to content

feat(move): reverse-window rollback for MoveTables#1051

Merged
morgo merged 3 commits into
block:mainfrom
morgo:feat/reverse-window-move
Jul 9, 2026
Merged

feat(move): reverse-window rollback for MoveTables#1051
morgo merged 3 commits into
block:mainfrom
morgo:feat/reverse-window-move

Conversation

@morgo

@morgo morgo commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

A Pull Request should be associated with an Issue.

We wish to have discussions in Issues. A single issue may be targeted by multiple PRs.
If you're offering a new feature or fixing anything, we'd like to know beforehand in Issues,
and potentially we'll be able to point development in a particular direction.
Further notes in https://github.com/block/spirit/blob/main/.github/CONTRIBUTING.md

Add an optional post-cutover reverse window to move: after a MoveTables cutover, --reverse-window=<duration> keeps the source current in change-only reverse mode (targets → source) so the move can be rolled back before the source is retired. On timeout — or if the reverse feed dies — the move finalizes forward; on an operator revert request it reverse-cuts-over back to the source. 0 (the default) is a normal one-way cutover.

Builds on the reverse-feed foundations merged in #1060 (change.Source.CurrentPosition + move.ReverseFeed); this PR is the integration only.

  • Driver (reversewindow.go) — stands up the change-only reverse feed at cutover, holds the window, and takes a terminal action: complete-forward on timeout or feed death, reverse cutover on a revert request.
  • Revert trigger (revertmarker.go) — the operator creates a _spirit_move_revert table on the first target; the driver polls for it. Pre-flight and pre-cutover refuse to run if a stale marker is already present.
  • Reverse cutover — un-retires the source (_old → real) and retires the former targets to a _revert suffix (distinct from _old); dropStaleRevertTables makes revert→retry idempotent.
  • Checkpoint (checkpoint.go) — adds move_phase and cutover_at columns so a move killed during the window resumes back into it (maybeResumeReverseWindow) rather than re-copying. An interrupted reverse cutover (phase reverting) is intentionally left for manual completion, not auto-resumed.
  • Status — new ReverseWindow state (sorts after CutOver).
  • Runner wiring — postSwitch position capture under the source lock, the --reverse-window gate, the SetReverseCutover hook, and a single-(unsharded)-source guard.
  • Docsdocs/move.md (flag reference), pkg/status/README.md, pkg/move/README.md.

Requires an unsharded (single) source. Tested against MySQL: complete-forward, revert (binlog + GTID), resume-after-kill, idempotent revert→retry, and stale-marker refusal.

Out of scope (separate strata-side PR): the strata revert command and the cutover-func split.

Note: this changes the move checkpoint format (adds move_phase / cutover_at).

@morgo morgo force-pushed the feat/reverse-window-move branch 2 times, most recently from 3b7c70a to db0bbcd Compare July 9, 2026 15:50
@morgo morgo marked this pull request as ready for review July 9, 2026 15:50
@morgo morgo marked this pull request as draft July 9, 2026 15:55
morgo and others added 2 commits July 9, 2026 11:16
After a MoveTables cutover, --reverse-window=<duration> keeps the move alive in
change-only reverse mode (targets→source) for the window, so it can be rolled
back before the source is retired. 0 (the default) is a normal cutover. Builds
on the reverse-feed foundations (block#1060).

- reverseWindow driver (reversewindow.go): stands up the reverse feed at cutover,
  holds the window, and takes a terminal action — complete-forward on timeout or
  feed death, reverse cutover on an operator revert request.
- Revert trigger (revertmarker.go): the operator creates a _spirit_move_revert
  table on the first target; the driver polls for it. Pre-flight and pre-cutover
  refuse to run when a (stale) marker is already present.
- Reverse cutover un-retires the source (_old→real) and retires the former
  targets to _revert (check.RevertRetiredName), distinct from the source's _old;
  dropStaleRevertTables makes revert→retry idempotent.
- Checkpoint gains move_phase/cutover_at (checkpoint.go) so a killed move resumes
  back into the window (runner.maybeResumeReverseWindow); the reverting phase
  requires manual completion.
- New status.ReverseWindow state (after CutOver); TestStateOrder guard updated.
- runner.go wiring: postSwitch position capture, --reverse-window gate,
  SetReverseCutover hook, single-source guard.

Requires an unsharded (single) source. Integration tests cover complete-forward,
revert (binlog + GTID), resume-after-kill, idempotent retry, and stale-marker
refusal.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- docs/move.md: new reverse-window flag section — behavior, ending conditions,
  requesting a rollback via the _spirit_move_revert marker, the unsharded-source
  constraint, the stale-marker guard, and resume semantics — plus the
  Configuration index entry.
- pkg/status/README.md: add the ReverseWindow state to the lifecycle list and
  explain its post-cutover placement.
- pkg/move/README.md: add the reverse-window lifecycle step and a Reverse Window
  design-decisions section.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@morgo morgo force-pushed the feat/reverse-window-move branch from cbcf622 to 93b34ad Compare July 9, 2026 17:22
@morgo morgo marked this pull request as ready for review July 9, 2026 17:24
@morgo morgo requested a review from Copilot July 9, 2026 17:25

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

Adds an optional, durable “reverse window” phase to move runs: after forward cutover, Spirit can keep the retired source tables current via a change-only reverse feed (targets → source _old) for a bounded duration, allowing an operator-triggered rollback before finalizing the move.

Changes:

  • Introduces a new lifecycle state (status.ReverseWindow) and documents/tests the required state ordering invariants.
  • Extends move.Runner with reverse-window orchestration: revert-marker guard, cutover post-switch hook to capture reverse start positions, reverse-window loop, and reverse cutover support.
  • Extends the shared checkpoint schema/record to persist reverse-window phase and forward-cutover timestamp.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/status/state.go Adds ReverseWindow state after CutOver.
pkg/status/state_order_test.go Updates ordering assertions to include ReverseWindow.
pkg/status/README.md Documents the new state and why it sorts after CutOver.
pkg/move/runner.go Adds reverse-window wiring: resume handling, revert-marker guards, stop background dumpers, reverse cutover hook, and support utilities.
pkg/move/revertmarker.go Adds _spirit_move_revert marker existence/drop helpers.
pkg/move/reversewindow.go Implements reverse-window driver: build reverse feed, poll for revert/deadline/feed death, complete-forward or reverse cutover.
pkg/move/reversewindow_test.go End-to-end integration tests for complete-forward, revert, GTID mode, resume-after-kill, and stale marker refusal.
pkg/move/README.md Documents reverse-window behavior, constraints, and resume semantics.
pkg/move/move.go Adds --reverse-window flag + validation.
pkg/move/cutover.go Adds CutOver.postSwitch hook to capture reverse-feed positions/checkpoint phase right after traffic switch.
pkg/move/check/rename_safety.go Adds _revert retired-name helper for rollback artifacts.
pkg/checkpoint/checkpoint.go Extends checkpoint schema + read/write to include move_phase and cutover_at.
docs/move.md Documents --reverse-window, operator rollback trigger, constraints, and resume behavior.

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

Comment thread pkg/move/reversewindow.go
Comment thread pkg/move/runner.go Outdated
Comment thread pkg/move/runner.go
…backstop wording

From Copilot's review of block#1051:
- buildFeed now requires a non-empty captured start position for every target
  before starting the reverse feed. A missing/empty entry (e.g. a corrupted or
  partial checkpoint on resume) previously fell back to the target's current
  head, silently skipping post-cutover writes and making rollback unsafe.
- Corrected the copy-path resume guard's comment/error: a post-cutover
  (reverse-window) checkpoint is resumed earlier by maybeResumeReverseWindow, so
  the guard is a defensive backstop — not "reverse-window resume is unsupported".

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@morgo morgo merged commit ade989b into block:main Jul 9, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants