Skip to content

Properly skip publishing if checkpoints are present, but publishing is disabled#5378

Merged
marta-lokhova merged 1 commit into
stellar:masterfrom
marta-lokhova:historyArchiveBugfix
Jul 24, 2026
Merged

Properly skip publishing if checkpoints are present, but publishing is disabled#5378
marta-lokhova merged 1 commit into
stellar:masterfrom
marta-lokhova:historyArchiveBugfix

Conversation

@marta-lokhova

Copy link
Copy Markdown
Contributor

The bug showed up in simulations, that kept running into nodes getting delayed by a few ledgers for no good reason.

Summary of the bug (AI-assisted):

  • PutSnapshotFilesWork::doWork() busy-loops WORK_RUNNING forever when there are no writable history archives (Step 1 creates zero children, so it never reaches WORK_SUCCESS).
  • The after-upgrade node hits this condition because it restored before-upgrade's DB, which contained a queued checkpoint publish for ledger 31. Its own config has localHistory = false. WorkScheduler::scheduleOne then hogs the main thread in CRANK_TIME_SLICE = 500 ms bursts, re-posting itself indefinitely.
  • Every cross-thread post waits ~one slice, which is what causes post-on-main-thread.delay (mean 377 / max 530 ms). These delays causes the node to continue staying a few ledgers behind instead of properly catching up to the fast nodes

Copilot AI review requested due to automatic review settings July 24, 2026 20:38

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

Prevents stale publish queues from busy-looping when no writable history archive exists.

Changes:

  • Skip queued-history publication when publishing is unavailable.
  • Fail defensively if snapshot work starts without writable archives.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/history/HistoryManagerImpl.cpp Adds the publication guard.
src/historywork/PutSnapshotFilesWork.cpp Prevents the empty-archive busy loop.
Comments suppressed due to low confidence (1)

src/history/HistoryManagerImpl.cpp:406

  • Add a regression test for the reported restart/restore case: persist a queued checkpoint with publishing configured, reopen the same state without writable archives, call publishQueuedHistory(), and verify that it returns zero without scheduling work or stalling main-thread cranks. The existing history test suite covers persisted publish queues, but neither new no-archive path is exercised, so the busy-loop fix could regress unnoticed.
    if (!mApp.getHistoryArchiveManager().publishEnabled())

Comment thread src/history/HistoryManagerImpl.cpp Outdated
@marta-lokhova
marta-lokhova force-pushed the historyArchiveBugfix branch from 529904d to b95c6e1 Compare July 24, 2026 20:45
@marta-lokhova
marta-lokhova requested a review from sisuresh July 24, 2026 20:48
@marta-lokhova
marta-lokhova enabled auto-merge July 24, 2026 20:52
@marta-lokhova
marta-lokhova added this pull request to the merge queue Jul 24, 2026
Merged via the queue into stellar:master with commit c444c07 Jul 24, 2026
56 checks passed
@marta-lokhova
marta-lokhova deleted the historyArchiveBugfix branch July 24, 2026 22:26
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