Skip to content

Add cutover concurrent test#545

Merged
morgo merged 6 commits intoblock:mainfrom
morgo:mtocker-add-cutover-concurrent-test
Jan 9, 2026
Merged

Add cutover concurrent test#545
morgo merged 6 commits intoblock:mainfrom
morgo:mtocker-add-cutover-concurrent-test

Conversation

@morgo
Copy link
Collaborator

@morgo morgo commented Dec 21, 2025

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

This is a pretty isolated/easy way to test the cutover better. It is inspired by one of the tests added to MoveTables.

Also fixes #548 again, since there was a second issue here. It ensures that the buffered position is always going forward. There is a case where FormatDescriptionEvent events could prevent BlockWait from completing.

@morgo morgo force-pushed the mtocker-add-cutover-concurrent-test branch from addbd77 to f67d8f0 Compare December 21, 2025 19:51
@morgo morgo requested a review from Copilot December 21, 2025 19:51
Copy link
Contributor

Copilot AI left a comment

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 adds a concurrent test for cutover atomicity to verify that the migration system correctly maintains consistency between the old and new tables during the cutover process. The test uses an intentionally partial cutover that only renames the original table to _old (skipping the rename of _new to original), creating a failure scenario that allows verification of data consistency under concurrent write load.

  • Adds a test-only cutover mechanism (useTestCutover) that performs a partial rename for failure injection testing
  • Implements TestCutoverAtomicityWithConcurrentWrites to verify consistency between _old and _new tables with concurrent writes
  • Refactors cutover logic to extract executeRenameUnderLock as a shared implementation for both normal and test cutovers

Reviewed changes

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

File Description
pkg/migration/migration.go Adds private useTestCutover field to control test-only cutover behavior
pkg/migration/runner.go Passes useTestCutover flag from Migration to cutover configuration
pkg/migration/cutover.go Adds partialRenameForTest method and refactors rename logic into shared executeRenameUnderLock function
pkg/migration/cutover_test.go Adds comprehensive concurrent test with helper functions to simulate write load during migration

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

@morgo morgo requested a review from kolbe December 22, 2025 14:12
@morgo morgo enabled auto-merge January 9, 2026 17:52
@morgo morgo force-pushed the mtocker-add-cutover-concurrent-test branch from 00c6cab to 8d2e327 Compare January 9, 2026 18:01
@morgo morgo disabled auto-merge January 9, 2026 18:35
@morgo morgo enabled auto-merge January 9, 2026 18:49
@morgo morgo disabled auto-merge January 9, 2026 18:50
@morgo morgo merged commit 4737179 into block:main Jan 9, 2026
7 of 8 checks passed
@morgo morgo deleted the mtocker-add-cutover-concurrent-test branch January 9, 2026 18:52
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.

spirit incorrectly handles replication.RotateEvent

2 participants