Skip to content

Prevent race condition on cutover completion#1664

Merged
meiji163 merged 2 commits intomasterfrom
jakubpliszka/fix-heartbeat-race
Apr 29, 2026
Merged

Prevent race condition on cutover completion#1664
meiji163 merged 2 commits intomasterfrom
jakubpliszka/fix-heartbeat-race

Conversation

@jakubpliszka
Copy link
Copy Markdown
Contributor

Description

This PR fixes a race between the heartbeat goroutine and finalCleanup() that can cause gh-ost to panic with a misleading FATAL after a successful migration:

FATAL injectHeartbeat writing failed 121 times, last error: Error 1146 (42S02): Table '<schema>._<table>_ghc' doesn't exist

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.

Copilot AI review requested due to automatic review settings April 29, 2026 16:56
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

This PR addresses a race where the heartbeat goroutine may continue writing to the changelog table while cleanup is dropping it, leading to a post-success panic.

Changes:

  • Stop heartbeat injection when migrationContext.CutOverCompleteFlag indicates cutover has completed.
Show a summary per file
File Description
go/logic/applier.go Adds a guard in the heartbeat loop to exit once cutover is marked complete.

Copilot's findings

Tip

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

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

Comment thread go/logic/applier.go
Comment on lines 749 to +755
if atomic.LoadInt64(&this.finishedMigrating) > 0 {
return
}

if atomic.LoadInt64(&this.migrationContext.CutOverCompleteFlag) > 0 {
return
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

@meiji163 meiji163 merged commit a6a5f49 into master Apr 29, 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