Skip to content

Add GET_LOCK to prevent concurrent migrations of the same table#1693

Merged
meiji163 merged 2 commits into
masterfrom
jakubpliszka/prevent-concurrent-migrations
May 28, 2026
Merged

Add GET_LOCK to prevent concurrent migrations of the same table#1693
meiji163 merged 2 commits into
masterfrom
jakubpliszka/prevent-concurrent-migrations

Conversation

@jakubpliszka
Copy link
Copy Markdown
Contributor

Description

This PR prevents two gh-ost processes from migrating the same table concurrently on the same MySQL server.

The existing table existence check runs only on fresh start, and is defeated by the dummy-drop-and-retry recovery flow when the previous gh-ost is still alive.

initiateApplier now takes GET_LOCK('gh-ost::<db>.<table>', 0) on a pinned connection. The lock survives table cleanup, auto-releases on TCP close so retries work, and a keepalive triggers PanicAbort if the connection dies.

In case this PR introduced Go code changes:

  • contributed code is using same conventions as original code
  • script/cibuild returns with no formatting errors, build errors or unit test errors.

@jakubpliszka jakubpliszka requested a review from rashiq as a code owner May 28, 2026 16:09
Copilot AI review requested due to automatic review settings May 28, 2026 16:09
Copy link
Copy Markdown
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

Adds a MySQL user-level GET_LOCK taken on a dedicated pinned connection at the start of initiateApplier() to prevent two gh-ost processes from concurrently migrating the same table on the same MySQL server. A keepalive goroutine pings the pinned connection and triggers PanicAbort if the connection dies; the lock is released in Applier.Teardown().

Changes:

  • New AcquireMigrationLock/releaseMigrationLock plus a keepalive goroutine in Applier, called from Migrator.initiateApplier() and released via Applier.Teardown().
  • Lock name gh-ost::<db>.<table> (SHA1-hashed if it exceeds MySQL's 64-char limit) computed by buildMigrationLockName.
  • New testcontainer-backed tests for acquire-when-free and acquire-when-held, a unit test for buildMigrationLockName, and added defer migrator.applier.Teardown() in migrator copier tests.
Show a summary per file
File Description
go/logic/applier.go Adds migration-wide GET_LOCK infrastructure (pinned conn, keepalive, release) and wires release into Teardown.
go/logic/migrator.go Acquires the migration lock immediately after InitDBConnections in initiateApplier.
go/logic/applier_test.go Adds integration tests for lock acquisition success/failure and a unit test for the lock-name builder.
go/logic/migrator_test.go Ensures applier is torn down after initiateApplier() in copier tests so the new lock connection is released.

Copilot's findings

Tip

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

  • Files reviewed: 4/4 changed files
  • Comments generated: 0

@meiji163 meiji163 merged commit c636347 into master May 28, 2026
9 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