LedgerDB: prune on garbage collection instead of on every change #1513
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
amesgen
force-pushed
the
amesgen/ledgerdb-garbage-collect-states
branch
2 times, most recently
from
8b48bb3 to
045f1cc
Compare
amesgen
force-pushed
the
amesgen/ledgerdb-garbage-collect-states
branch
4 times, most recently
from
13e5533 to
68402ed
Compare
jasagredo
approved these changes
May 21, 2025
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Storage/LedgerDB/V2.hs
Show resolved
Hide resolved
amesgen
force-pushed
the
amesgen/v2-ledgerseq-close
branch
from
981971e to
0c5b137
Compare
amesgen
force-pushed
the
amesgen/ledgerdb-garbage-collect-states
branch
from
68402ed to
4d6fd67
Compare
amesgen
force-pushed
the
amesgen/v2-ledgerseq-close
branch
from
0c5b137 to
7900088
Compare
amesgen
force-pushed
the
amesgen/ledgerdb-garbage-collect-states
branch
from
4d6fd67 to
7049fd4
Compare
|
Sync benchmarks are looking good (mainnet, first 1e6 slots/blocks):
LMDB benchmark (of course, this is a bit degenerate as Byron doesn't have tables, but this still serves as a regression test for the
Note that d1b6215 is crucial; otherwise, there is a significant (2x) regression in max heap size. |
amesgen
force-pushed
the
amesgen/ledgerdb-garbage-collect-states
branch
2 times, most recently
from
2e01b1c to
b9e25f5
Compare
amesgen
force-pushed
the
amesgen/ledgerdb-v2-locking
branch
from
19faf20 to
4010598
Compare
amesgen
force-pushed
the
amesgen/ledgerdb-garbage-collect-states
branch
from
b9e25f5 to
894940c
Compare
amesgen
force-pushed
the
amesgen/ledgerdb-garbage-collect-states
branch
from
894940c to
a8fa7e2
Compare
amesgen
force-pushed
the
amesgen/ledgerdb-garbage-collect-states
branch
from
b503dc3 to
6c78fad
Compare
amesgen
changed the base branch from
main
to
amesgen/ledgerdb-state-machine-precondition-bug
|
Thanks for the great reviews, I hope I addressed your comments. Interesting changes:
|
Base automatically changed from
amesgen/ledgerdb-state-machine-precondition-bug
to
main
July 2, 2025 14:48
amesgen
force-pushed
the
amesgen/ledgerdb-garbage-collect-states
branch
2 times, most recently
from
48bb1fe to
ad7acfa
Compare
amesgen
force-pushed
the
amesgen/ledgerdb-garbage-collect-states
branch
from
ad7acfa to
d791dfb
Compare
amesgen
force-pushed
the
amesgen/ledgerdb-garbage-collect-states
branch
from
d791dfb to
247c489
Compare
It is not necessary to perform the garbage collection of the LedgerDB and the map of invalid blocks in the same STM transaction. In the past, this was important, but it is not anymore, see #1507.
Primarily, this is an optimization to reduce the maximum memory usage (more relevant with the in-memory backend) when pruning happens on garbage collection instead of while adding new blocks to the LedgerDB, see the added commit and the benchmark in the pull request. Previously, LedgerDB garbage collection happened as part of VolatileDB garbage collection, which was intentionally rate-limited. Also, it resolves the current (somewhat weird) behavior that we do not copy any blocks to the ImmutableDB when we are taking a snapshot (which can take >2 minutes), and consequently also not garbage-collecting the VolatileDB. It also synergizes with the planned feature to add a random delay when taking snapshots.
Also make sure to account for the fact that the DbChangelog might have gotten pruned between opening and committing the forker.
regarding the previous few commits
amesgen
force-pushed
the
amesgen/ledgerdb-garbage-collect-states
branch
from
247c489 to
08bda65
Compare
It was already superseded in the most important places due to `LedgerDbPruneBeforeSlot`. Its remaining use cases are non-essential: - Replay on startup. In this case, we never roll back, so not maintaining k states is actually an optimization here. We can also remove the now-redundant `InitDB.pruneDb` function. - Internal functions used for db-analyser. Here, we can just as well use `LedgerDbPruneAll` (which is used by `pruneToImmTipOnly`) as we never need to roll back. - Testing. In particular, we remove some DbChangelog tests that previously ensured that only at most @k@ states are kept. This is now no longer true; that property is instead enforced by the LedgerDB built on top of the DbChangelog. A follow-up commit in this PR enriches the LedgerDB state machine test to make sure that the public API functions behave appropriately, ensuring that we don't lose test coverage (and also testing V2, which previously didn't have any such tests).
Make sure that we correctly fail when trying to roll back too far.
amesgen
force-pushed
the
amesgen/ledgerdb-garbage-collect-states
branch
from
08bda65 to
dec284f
Compare
jasagredo
approved these changes
Sep 2, 2025
github-project-automation
bot
moved this from 👀 In review
to ✅ Done
in Consensus Team Backlog
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.
This is in preparation for #1424
This PR is intended to be reviewed commit-by-commit.
Currently, we prune the LedgerDB (ie remove all but the last
k+1states) every time we adopt a longer chain. This means that we can not rely on the fact that other threads (like thecopyAndSnapshotChainDB background) actually observe all immutable ledger states, just as described in the caveats of ourWatcherabstraction.However, a predictable ledger snapshotting rule (#1424) requires this property; otherwise, when the node is under high load and/or we are adopting multiple blocks in quick succession, the node might not be able to create a snapshot for its desired block.
This PR changes this fact: Now, when adopting new blocks, the LedgerDB is not immediately pruned. Instead, the a new dedicated background thread for ledger maintenance tasks (flushing/snapshotting/garbage collection) in the ChainDB will periodically (on every new immutable block) wake up and (in particular) garbage collect the LedgerDB based on a slot number.
Also, this makes the semantics more consistent with the existing garbage collection of previously-applied blocks in the LedgerDB, and also with how the ChainDB works, where we also don't immediately delete blocks from the VolatileDB once they are buried beneath
k+1blocks.See #1513 (comment) for benchmarks demonstrating that the peak memory usage does not increase while syncing (where we now briefly might hold more than
k+1ledger states in memory).