feat(cli): add --storage.rocksdb flag for RocksDB history indices#21070
Closed
feat(cli): add --storage.rocksdb flag for RocksDB history indices#21070
Conversation
Co-authored-by: Sergei Shulepov <pep@tempo.xyz>
joshieDo
force-pushed
the
joshie/par-save-blocks-rocksdb
branch
from
e042e16 to
fff917c
Compare
joshieDo
force-pushed
the
joshie/par-save-blocks-rocksdb
branch
from
fff917c to
d8a600c
Compare
This was referenced Jan 14, 2026
yongkangc
force-pushed
the
yk/rocksdb-cli
branch
from
9ab5a43 to
d29b718
Compare
This wires RocksDB into the history lookup paths: - Adds account_history_info and storage_history_info methods to EitherReader - Updates HistoricalStateProviderRef to use EitherReader for lookups - Adds RocksDBProviderFactory trait bounds to provider impls - Uses the rank/select pattern for efficient binary search in shards
**Problem** - EitherReader had unnecessary PhantomData markers - RocksDB transaction setup was duplicated in historical.rs with cfg-gated blocks - Addressed joshieDo's feedback about RocksDB logic leaking into historical provider **Solution** - Remove PhantomData from EitherReader enum variants (lifetime already captured by RocksDB reference) - Add with_rocksdb_tx helper method to RocksDBProviderFactory trait - Refactor historical.rs to use trait method instead of duplicated cfg-gated blocks **Changes** - Remove PhantomData from EitherReader enum and all constructors/match arms - Add with_rocksdb_tx to RocksDBProviderFactory trait with default implementation - Refactor account_history_lookup and storage_history_lookup to use with_rocksdb_tx helper - Make RocksTxRefArg type alias public for trait method **Expected Impact** - Cleaner EitherReader API without unnecessary PhantomData - RocksDB transaction setup encapsulated in trait method - Reduced cfg-gated block duplication in historical.rs - No behavioral changes, all existing tests pass (96/97)
When rocksdb feature is disabled, the RocksDB variant is compiled out, leaving the lifetime 'a unused and causing E0392 error. Add PhantomData<&'a ()> to StaticFile variant to ensure the lifetime is always used regardless of feature flags.
…lication - Add compute_history_rank() function for shared rank/select logic - Simplify EitherReader::storage_history_info and account_history_info - Simplify RocksTx::history_info by using the shared helper - Import PhantomData directly instead of using std::marker::PhantomData - Fix clippy doc_markdown warnings for RocksDB in stub module - Make PendingRocksDBBatches pub(crate) to fix unreachable_pub warning Reduces duplicated rank/select code across 3 locations while preserving comments.
Previously, when RocksDB tables were empty but MDBX had checkpoints > 0, the consistency check would return Some(0), triggering an assertion failure because unwinding to block 0 is considered destructive. This is the expected state when RocksDB is enabled for the first time alongside existing MDBX data. The fix treats empty RocksDB tables as a first-run/migration scenario, logging a warning instead of requesting an unwind. The pipeline will naturally populate the tables during sync.
When RocksDB history tables have entries but ALL entries are sentinel entries (highest_block_number == u64::MAX), the consistency check was incorrectly returning Some(0) as the unwind target. This would trigger an assertion failure during node startup. Sentinel entries represent "open" shards that haven't been completed yet, meaning no actual history has been indexed. This is equivalent to the empty table case and should be treated as a first-run scenario. Added tests to verify this edge case is handled correctly.
…ks_data Add append_account_history_shard and append_storage_history_shard methods to RocksDBBatch that properly handle shard boundaries. Update write_blocks_data to use these methods instead of the naive approach that just wrote to u64::MAX shard without checking existing shards.
Add utility functions for RocksDB integration in stages: - make_rocksdb_provider() - creates RocksDB provider from UnifiedStorageWriter - make_rocksdb_batch_arg() - creates RocksDB batch for EitherWriter - register_rocksdb_batch() - registers batch with pending batches - collect_shards_for_unwind() - shared logic for history shard unwinding - Add RocksDBIntegrity enum variant for metadata
Add RocksDB unwind support for IndexAccountHistory and IndexStorageHistory stages: - Create RocksDB batch during unwind when storage settings enabled - Use EitherWriter for history shard operations - Register batches with pending_batches for atomic commit
Add RocksDB support for: - Genesis history initialization via EitherWriter - TransactionLookup stage unwind operations - RocksDB provider append_*_history_shard methods - Metrics for RocksDB write operations
Add a single CLI flag to enable RocksDB for all history tables: - --storage.rocksdb enables AccountsHistory, StoragesHistory, and TransactionHashNumbers to be stored in RocksDB instead of MDBX Also adds: - `reth db settings set` subcommands for individual table control - Updated CLI documentation
yongkangc
force-pushed
the
yk/rocksdb-cli
branch
from
d29b718 to
477d1da
Compare
gakonst
reviewed
Jan 16, 2026
Member
gakonst
left a comment
gakonst
left a comment
There was a problem hiding this comment.
Code Review: PR #21070
CI Status
- ❌ 18 failures (cascading from upstream)
Changes Summary
Adds --storage.rocksdb CLI flag for RocksDB history indices.
Review Notes
User-facing CLI change. Once upstream PRs pass:
- Verify flag parsing and validation
- Check default behavior (should be off by default for backwards compat)
- Ensure help text is clear
Review by @tempo_ai
Update various CLI commands and test utilities for RocksDB: - Stage drop command handles RocksDB table clearing - Stage dump commands pass storage settings - Test utilities provide RocksDB stubs - Add PendingRocksDBBatches export
yongkangc
force-pushed
the
yk/rocksdb-cli
branch
from
477d1da to
4f34746
Compare
Add clear() method to RocksDBProvider and its stub implementation to support clearing all entries from a RocksDB table. This is needed by the stage drop CLI command to clear RocksDB history tables. - Adds clear<T: Table>() method that iterates and deletes all entries - Stub implementation returns Ok(()) since stub behaves as empty DB
The stage/drop.rs was trying to call rocksdb.clear::<T>() but this method doesn't exist on RocksDBProvider. Remove the dead code blocks that would have caused compilation failures with the edge feature.
yongkangc
force-pushed
the
yk/rocksdb-cli
branch
from
d10a480 to
7477c97
Compare
Member
Author
|
Closing in favor of new approach using --rocksdb.* prefix. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary
Add --storage.rocksdb CLI flag to enable RocksDB storage for history indices.
Changes
Testing
Node starts with RocksDB flag.
PR Stack