LedgerDB.V2: prevent race condition when snapshotting #1555
+107
−86
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hold a read lock while getting and duplicating the anchor handle, such that the handle can not be closed in between. Also make sure to close the duplicated handle.
This reverts commit 7900088. Without the fix in the preceding commit, this would cause test failures, see cabal run ouroboros-consensus-diffusion:mock-test -- \ --quickcheck-replay="(SMGen 15972426649573867634 4412022096428701771,88)" \ -p '/Praos.simple convergence/'
No behavioral change, just a refactoring by extracting common parts; convenient for follow-up changes
amesgen
requested review from
dnadales,
fraser-iohk,
geo2a,
jasagredo and
nfrisby
as code owners
No behavioral change, purely a refactoring
Not necessary given how it is currently being used, but it is also easy enough to make it resilient against the race condition, so why not?
amesgen
force-pushed
the
amesgen/ledgerdb-v2-locking
branch
from
e500584 to
19faf20
Compare
amesgen
force-pushed
the
amesgen/ledgerdb-v2-locking
branch
from
19faf20 to
4010598
Compare
jasagredo
reviewed
Jun 11, 2025
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Storage/LedgerDB/V2.hs
Show resolved
Hide resolved
jasagredo
approved these changes
Jun 11, 2025
amesgen
added a commit
that referenced
this pull request
Jun 30, 2025
Closes #1551 The commits are intended to be reviewed individually. - The first commit fixes the race condition described in #1551 by using the `ldbOpenHandlesLock` to duplicate the handle when snapshotting. - The second commit reverts 7900088 added in #1516, enabled by the first commit. Without the first commit, this would cause test failures, see the commit description. - The third and fourth commits are purely refactorings to be able to reuse the pattern of "duplicating while holding a read lock". - The fifth commit uses the same pattern for `takeSnapshotNOW`, a testing function, see the commit description. - The last commit adds documentation for `ldbOpenHandlesLock`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1551
The commits are intended to be reviewed individually.
LedgerTableHandles #1551 by using theldbOpenHandlesLockto duplicate the handle when snapshotting.takeSnapshotNOW, a testing function, see the commit description.ldbOpenHandlesLock.