Skip to content

Commit c3b1ab6

Browse files
authored
LedgerDB.V1: fix race between chain switch and forker acquisition (#1737)
This fixes a small bug introduced in #1619, which does not affect Praos. ### Explanation Before this PR, it was possible that, when opening a forker, we first would read the `DbChangelog` https://github.com/IntersectMBO/ouroboros-consensus/blob/fd31cf8fd056d5ddee39c79400b3382d93bccb47/ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Storage/LedgerDB/V1.hs#L762 then switch to a different chain in https://github.com/IntersectMBO/ouroboros-consensus/blob/fd31cf8fd056d5ddee39c79400b3382d93bccb47/ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Storage/ChainDB/Impl/ChainSel.hs#L915-L916 and only then read https://github.com/IntersectMBO/ouroboros-consensus/blob/fd31cf8fd056d5ddee39c79400b3382d93bccb47/ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Storage/LedgerDB/V1.hs#L763 With Praos, this doesn't matter, because `getVolatileSuffix` will always return the `k` most recent blocks/states, so there is no observable difference. However, with Peras, it can be the case that the length of the volatile suffix gets longer/shorter than before, leading to a mismatch in the length of the volatile headers and volatile `DbChangelog` states. In practice, the only effect is that a user would erroneously not be able to acquire points in the LocalStateQuery mini-protocol that it should be able to acquire, or vice versa. ### Fix The fix is to simply move these two things into the same `STM` transaction, as is already the case in all other places in LedgerDB.{V1,V2} that involve `ldbGetVolatileSuffix`. A regression test would need to rely on IOSimPOR; therefore, I am deferring this until we have sth like #1494.
2 parents fd31cf8 + f8271b2 commit c3b1ab6

File tree

2 files changed

+10
-6
lines changed

2 files changed

+10
-6
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
### Patch
2+
3+
- Fix a race condition between chain switches and LedgerDB.V1 forker acquisition.

ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Storage/LedgerDB/V1.hs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -759,13 +759,14 @@ acquireAtTarget ::
759759
Either Word64 (Target (Point blk)) ->
760760
ReadLocked m (Either GetForkerError (DbChangelog l))
761761
acquireAtTarget ldbEnv target = readLocked $ runExceptT $ do
762-
dblog <- lift $ readTVarIO (ldbChangelog ldbEnv)
763-
volSuffix <- lift $ atomically $ getVolatileSuffix $ ldbGetVolatileSuffix ldbEnv
764-
-- The DbChangelog might contain more than k states if they have not yet
765-
-- been garbage-collected.
766-
let volStates = volSuffix $ changelogStates dblog
762+
(dblog, volStates) <- lift $ atomically $ do
763+
dblog <- readTVar (ldbChangelog ldbEnv)
764+
-- The DbChangelog might contain more than k states if they have not yet
765+
-- been garbage-collected.
766+
volSuffix <- getVolatileSuffix $ ldbGetVolatileSuffix ldbEnv
767+
pure (dblog, volSuffix $ changelogStates dblog)
767768

768-
immTip :: Point blk
769+
let immTip :: Point blk
769770
immTip = castPoint $ getTip $ AS.anchor volStates
770771

771772
rollbackMax :: Word64

0 commit comments

Comments
 (0)