Fix race in MissionDatabaseInplaceUpgrade to wait for late watchers to catchup prior to consistency checks#416
Conversation
…ke the sibling MissionProtocolUpgradeWithLoad that has 1k tx rate
There was a problem hiding this comment.
Pull request overview
Improves mission reliability under heavy load and during database upgrades.
Changes:
- Allocates upgrade-grade resources for the transaction-set-limit mission.
- Waits for watcher nodes before final consistency checks.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
MissionLoadGenerationWithTxSetLimit.fs |
Increases core resources. |
MissionDatabaseInplaceUpgrade.fs |
Adds watcher catch-up waits. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| peer.WaitUntilCaughtUpWith(corePeer) | ||
| upgradedPeer.WaitUntilCaughtUpWith(corePeer)) |
There was a problem hiding this comment.
Good catch. WaitUntilCaughtUpWith allows a 10-ledger gap (maxLedgerGap = 10). Updating the patch to tighten the fix.
There was a problem hiding this comment.
Fixed by using WaitForLedgerNum. @copilot, please re-review?
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/FSLibrary/MissionDatabaseInplaceUpgrade.fs:97
WaitForLedgerNum(finalLedger)only waits for each watcher to pass a historical threshold; it does not reduce its gap from the still-advancingcorePeer. A watcher steadily 10 ledgers behind will eventually passfinalLedgerwhile remaining 10 behind, after which the five-sample consistency check can still find no overlapping ledger. Please poll each watcher's current gap tocorePeeruntil it is smaller than the consistency window (for example, parameterizeWaitUntilCaughtUpWithwith a maximum gap belowConsistencyCheckIterationCount) before returning from the mission.
let finalLedger = corePeer.GetLedgerNum()
peer.WaitForLedgerNum(finalLedger)
upgradedPeer.WaitForLedgerNum(finalLedger))
…tcher is caught up before the automatic consistency check runs Tighten the check since WaitUntilCaughtUpWith allows a 10-ledger gap revert revert MissionDatabaseInplaceUpgrade changes
f212c5c to
e160792
Compare
|
Closing this as root cause has been identified in stellar/stellar-core#5378 |
|
I was repurposing this PR to only do the second change but after force pushing to revert MissionDatabaseInplaceUpgrade changes, I realized we have another PR #415 for attempting to fix MissionLoadGenerationWithTxSetLimit. |
Two fixes here from Claude on flaky tests in MissionDatabaseInplaceUpgrade and MissionLoadGenerationWithTxSetLimit:
Exception thrown. Message = MaybeInconsistentPeersExceptionThere is a race and after the final loadgen on both after-upgrade and core, we should wait for both non-validator watcher nodes to catch up to core-0 before the automatic pairwise consistency check runs.