fix(l1): bound RocksDB index and filter block memory#6735
Conversation
… instead of pinning them per open file. With max_open_files(-1) every SST stays open, and the RocksDB default (cache_index_and_filter_blocks = false) keeps each file's index and filter blocks in heap for the reader's lifetime, so table memory grows without bound with the number of SST files. On a 490 GB mainnet DB this reached ~8 GB of pinned index/filter blocks (~6 GB of it bloom filters), driving resident memory to ~20 GB. Enabling cache_index_and_filter_blocks moves index and filter blocks into the bounded block cache, capping total table memory at the cache size. pin_l0_filter_and_index_blocks_in_cache keeps the hottest level's metadata resident to avoid a read-latency cliff on the cache.
|
Lines of code reportTotal lines added: Detailed view |
(--rocksdb.block-cache-size, env ETHREX_ROCKSDB_BLOCK_CACHE_SIZE) with a default
of 20 GiB. Because the previous commit moved index and bloom-filter blocks into
the bounded block cache, the cache size now governs total RocksDB resident memory
and significantly influences block-import throughput. Measured on a synced mainnet
node: at a 4 GiB cache, filter blocks monopolize the cache and block exec is ~76%
slower than the unbounded baseline; at 20 GiB the cache comfortably holds the
filter + index working set plus the EVM's hot data and exec is at parity. The
help text spells the trade-off out explicitly and only recommends lowering it on
resource-constrained hosts.
Plumbed through a new StoreConfig struct (exposed from ethrex-storage) and
Store::new_with_config / new_from_genesis_with_config /
{init,load,open}_store_with_config variants. The existing zero-config
constructors continue to use the default and remain unchanged for tests and
tools, so callers that don't need to override the cache size are unaffected.
…ndex-filter-memory # Conflicts: # cmd/ethrex/initializers.rs
iovoid
force-pushed
the
fix/rocksdb-bounded-index-filter-memory
branch
2 times, most recently
from
2f10db8 to
b1851f2
Compare
iovoid
force-pushed
the
fix/rocksdb-bounded-index-filter-memory
branch
from
b1851f2 to
f235e8e
Compare
🤖 Kimi Code ReviewOverall Assessment: Well-structured PR that introduces configurable RocksDB block cache sizing with sensible defaults and good documentation. The change from hardcoded 4GB to configurable 12GB with proper index/filter block caching is a significant improvement for mainnet stability. Critical Issue: 32-bit Compatibility
Code Quality & Safety
Performance & Observability
Nits
Security: No vulnerabilities introduced. The cache size is purely a local performance tunable and does not affect consensus or network validation. Recommendation: Address Item 1 (32-bit compatibility) before merge, or explicitly document that ethrex requires 64-bit architectures. The rest of the PR is solid and ready for merge. Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Claude Code ReviewNow I have all the information needed for a thorough review. Here is the complete analysis: PR #6735 —
|
🤖 Codex Code Review
Open question:
Aside from that, I did not find consensus/EVM/security-impacting issues in this PR; the storage change itself looks internally consistent. I could not run Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Greptile SummaryThis PR fixes an unbounded memory growth issue in the RocksDB backend where index and bloom-filter blocks were pinned in heap per open SST file (outside the bounded LRU), growing proportionally with database size. The fix routes all index/filter blocks through the shared block cache via
Confidence Score: 4/5Safe to merge; the change correctly bounds RocksDB resident memory and is backward-compatible. The core fix is sound and well-validated with extensive benchmark data. Both comments relate to the same gap in configure_block_cache: without cache_index_and_filter_blocks_with_high_priority and a high_pri_pool_ratio on the cache, non-L0 filter/index blocks share equal eviction priority with data blocks. The 12 GiB default keeps this from mattering in the mainnet sweep, but a workload with heavy data-block traffic could push filter/index blocks for deeper LSM levels out of cache. crates/storage/backend/rocksdb.rs — the configure_block_cache closure and the cache construction line above it.
|
| Filename | Overview |
|---|---|
| crates/storage/backend/rocksdb.rs | Core fix: routes index/filter blocks through bounded LRU via cache_index_and_filter_blocks(true) + pin_l0_filter_and_index_blocks_in_cache(true) on every CF; cache size is now caller-supplied. Missing cache_index_and_filter_blocks_with_high_priority means non-L0 filter/index blocks compete equally with data blocks under cache pressure. |
| crates/storage/store.rs | Adds StoreConfig, DEFAULT_ROCKSDB_BLOCK_CACHE_SIZE_BYTES, and new_with_config/new_from_genesis_with_config variants; existing new/new_from_genesis correctly delegate to the new variants with defaults. |
| crates/storage/lib.rs | Re-exports DEFAULT_ROCKSDB_BLOCK_CACHE_SIZE_BYTES and StoreConfig at the crate root; clean and minimal change. |
| cmd/ethrex/cli.rs | Adds --rocksdb.block-cache-size CLI arg with env var ETHREX_ROCKSDB_BLOCK_CACHE_SIZE, default from the storage constant, and a well-written help string explaining the trade-off. |
| cmd/ethrex/initializers.rs | Adds _with_config variants for all store init helpers; legacy helpers delegate to them with StoreConfig::default(); init_l1 threads the CLI-supplied cache size through correctly. |
| cmd/ethrex/l2/initializers.rs | L2 init path switched to init_store_with_config, picking up the CLI-supplied block cache size from L1Options; correct plumbing. |
| tooling/migrations/src/bin/bench_migration.rs | Updated call site for the new RocksDBBackend::open signature; uses the public default constant. |
| docs/CLI.md | Auto-generated CLI help updated to include the new --rocksdb.block-cache-size option; trailing whitespace in existing entries is a side-effect of regeneration. |
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
crates/storage/backend/rocksdb.rs:113-117
Without `set_cache_index_and_filter_blocks_with_high_priority(true)` (paired with a `high_pri_pool_ratio` on the cache), all non-L0 filter/index blocks compete equally with data blocks in the LRU. Under a read-heavy workload that touches many L1+ SST files (e.g. state queries or a fast-sync replay), data blocks can displace filter/index blocks from the cache, increasing filter misses and triggering extra disk reads — the same class of performance degradation the PR fixes for the baseline. Adding high-priority routing for index/filter blocks gives the eviction policy a knob to keep them resident without growing the cache.
```suggestion
let configure_block_cache = |block_opts: &mut BlockBasedOptions| {
block_opts.set_block_cache(&block_cache);
block_opts.set_cache_index_and_filter_blocks(true);
block_opts.set_cache_index_and_filter_blocks_with_high_priority(true);
block_opts.set_pin_l0_filter_and_index_blocks_in_cache(true);
};
```
### Issue 2 of 2
crates/storage/backend/rocksdb.rs:102
**Cache `high_pri_pool_ratio` not set**
`pin_l0_filter_and_index_blocks_in_cache` pins L0 filter/index blocks via a ref-count, but the effectiveness of `set_cache_index_and_filter_blocks_with_high_priority` (if added) depends on the cache having a non-zero high-priority pool. Without `Cache::new_lru_cache` accepting a `high_pri_pool_ratio`, or using `LruCacheOptions` to set it, the high-priority designation is a no-op. The RocksDB recommendation for this configuration is to use a 10% high-priority pool (e.g. `0.1`) so filter/index blocks are protected from eviction by data blocks when the cache is under pressure.
Reviews (1): Last reviewed commit: "fix(ci): sync CLI.md help, fix rocksdb t..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let configure_block_cache = |block_opts: &mut BlockBasedOptions| { | ||
| block_opts.set_block_cache(&block_cache); | ||
| block_opts.set_cache_index_and_filter_blocks(true); | ||
| block_opts.set_pin_l0_filter_and_index_blocks_in_cache(true); | ||
| }; |
There was a problem hiding this comment.
set_cache_index_and_filter_blocks_with_high_priority(true) (paired with a high_pri_pool_ratio on the cache), all non-L0 filter/index blocks compete equally with data blocks in the LRU. Under a read-heavy workload that touches many L1+ SST files (e.g. state queries or a fast-sync replay), data blocks can displace filter/index blocks from the cache, increasing filter misses and triggering extra disk reads — the same class of performance degradation the PR fixes for the baseline. Adding high-priority routing for index/filter blocks gives the eviction policy a knob to keep them resident without growing the cache.
| let configure_block_cache = |block_opts: &mut BlockBasedOptions| { | |
| block_opts.set_block_cache(&block_cache); | |
| block_opts.set_cache_index_and_filter_blocks(true); | |
| block_opts.set_pin_l0_filter_and_index_blocks_in_cache(true); | |
| }; | |
| let configure_block_cache = |block_opts: &mut BlockBasedOptions| { | |
| block_opts.set_block_cache(&block_cache); | |
| block_opts.set_cache_index_and_filter_blocks(true); | |
| block_opts.set_cache_index_and_filter_blocks_with_high_priority(true); | |
| block_opts.set_pin_l0_filter_and_index_blocks_in_cache(true); | |
| }; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/storage/backend/rocksdb.rs
Line: 113-117
Comment:
Without `set_cache_index_and_filter_blocks_with_high_priority(true)` (paired with a `high_pri_pool_ratio` on the cache), all non-L0 filter/index blocks compete equally with data blocks in the LRU. Under a read-heavy workload that touches many L1+ SST files (e.g. state queries or a fast-sync replay), data blocks can displace filter/index blocks from the cache, increasing filter misses and triggering extra disk reads — the same class of performance degradation the PR fixes for the baseline. Adding high-priority routing for index/filter blocks gives the eviction policy a knob to keep them resident without growing the cache.
```suggestion
let configure_block_cache = |block_opts: &mut BlockBasedOptions| {
block_opts.set_block_cache(&block_cache);
block_opts.set_cache_index_and_filter_blocks(true);
block_opts.set_cache_index_and_filter_blocks_with_high_priority(true);
block_opts.set_pin_l0_filter_and_index_blocks_in_cache(true);
};
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Same as the other, those two options have default values
| // the size (see the `--rocksdb.block-cache-size` CLI flag); a value that is too | ||
| // small relative to the filter + working-set size will degrade block-import | ||
| // throughput (filter blocks displace data blocks, EVM reads spill to disk). | ||
| let block_cache = Cache::new_lru_cache(block_cache_size); |
There was a problem hiding this comment.
high_pri_pool_ratio not set
pin_l0_filter_and_index_blocks_in_cache pins L0 filter/index blocks via a ref-count, but the effectiveness of set_cache_index_and_filter_blocks_with_high_priority (if added) depends on the cache having a non-zero high-priority pool. Without Cache::new_lru_cache accepting a high_pri_pool_ratio, or using LruCacheOptions to set it, the high-priority designation is a no-op. The RocksDB recommendation for this configuration is to use a 10% high-priority pool (e.g. 0.1) so filter/index blocks are protected from eviction by data blocks when the cache is under pressure.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/storage/backend/rocksdb.rs
Line: 102
Comment:
**Cache `high_pri_pool_ratio` not set**
`pin_l0_filter_and_index_blocks_in_cache` pins L0 filter/index blocks via a ref-count, but the effectiveness of `set_cache_index_and_filter_blocks_with_high_priority` (if added) depends on the cache having a non-zero high-priority pool. Without `Cache::new_lru_cache` accepting a `high_pri_pool_ratio`, or using `LruCacheOptions` to set it, the high-priority designation is a no-op. The RocksDB recommendation for this configuration is to use a 10% high-priority pool (e.g. `0.1`) so filter/index blocks are protected from eviction by data blocks when the cache is under pressure.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
set_cache_index_and_filter_blocks_with_high_priority is enabled by default and high_pri_pool_ratio has a default value of 0.5
Motivation
ethrex's RocksDB backend ties resident memory to database size with no upper bound. With
max_open_files(-1)every SST stays open, and RocksDB's default (cache_index_and_filter_blocks = false) keeps each open file's index and bloom-filter blocks pinned in heap for the reader's lifetime — outside the bounded LRU. As on-disk state grows, so does the in-heap footprint, with no ceiling: operationally indistinguishable from a memory leak, and on a large enough database it eventually exhausts the host. On a ~500 GB mainnet DB this reached ~8 GB of pinned index/filter blocks (~6 GB of it bloom filters), driving resident memory to ~20 GB.Description
1. Route index and filter blocks through the shared block cache.
Enables
cache_index_and_filter_blocks(true)+pin_l0_filter_and_index_blocks_in_cache(true)on every column family. RocksDB stops pinning every open SST's index/filter blocks in heap and instead routes them through its bounded LRU. Total RocksDB resident memory now tracks the block cache size, not the database size.2. Expose the block cache size as a CLI option.
Adds
--rocksdb.block-cache-size <BYTES>(envETHREX_ROCKSDB_BLOCK_CACHE_SIZE), default 12 GiB, plumbed through aStoreConfigstruct and*_with_configconstructor variants onStoreand theinit_store/load_store/open_storehelpers; the zero-config constructors keep working with the default and are unchanged for tests, tools, and L2 callers.Because filter/index blocks now share one bounded cache with data blocks, the cache size governs a memory vs. block-import-throughput trade-off: a cache too small to hold the filter+index working set plus useful hot data will stall execution. The default is sized to run smoothly on a 32 GB host (see the sweep below); the CLI help text states the trade-off and only recommends lowering it on memory-constrained hosts.
Validation
Heap profile (the unbounded growth)
A jemalloc profile of the unfixed baseline attributed ~92% of resident memory to RocksDB, dominated by ~8 GB of index/bloom-filter blocks. With the fix applied, the
PrefetchIndexAndFilterBlocksallocations drop from ~8 GB to under 1 GB — the rest is demand-loaded into the bounded cache viaGetOrReadFilterBlock.Block-import benchmark (mainnet head-following, 60-block window, same chain segment)
A too-small cache lets filter blocks displace data and slows import; a sufficiently large cache is at parity with the unbounded baseline:
Cache-size sweep (32 GB host) — picking the default
To choose a default that runs smoothly on memory-constrained hosts, swept the cache under a 32 GiB cgroup cap on a synced mainnet node, 45-min steady-state windows:
All three keep up with head-following: filters stay resident, disk is near-idle, zero slow blocks, no memory pressure or OOM under the 32 GB cap.
filter.missfirst starts rising (the high-priority pool nears the hot filter working set).Trade-off
At today's ~500 GB mainnet DB, a 12 GiB cache bounds RocksDB resident memory forever regardless of database growth, versus the unfixed baseline's ~16 GB and climbing (state DBs only grow). Operators who need a lower ceiling can reduce the cache size at some throughput cost (≥ 8 GiB recommended); the help text documents this.