feat(storage): add EitherWriter methods for RocksDB history operations#21067
Closed
yongkangc wants to merge 11 commits intojoshie/par-save-blocks-rocksdbfrom
Closed
feat(storage): add EitherWriter methods for RocksDB history operations#21067yongkangc wants to merge 11 commits intojoshie/par-save-blocks-rocksdbfrom
yongkangc wants to merge 11 commits intojoshie/par-save-blocks-rocksdbfrom
Conversation
This was referenced Jan 14, 2026
yongkangc
force-pushed
the
yk/rocksdb-either-writer-ext
branch
from
9428451 to
071cd7b
Compare
joshieDo
force-pushed
the
joshie/par-save-blocks-rocksdb
branch
2 times, most recently
from
fff917c to
d8a600c
Compare
This was referenced Jan 14, 2026
yongkangc
force-pushed
the
yk/rocksdb-either-writer-ext
branch
from
071cd7b to
8910828
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
yongkangc
force-pushed
the
yk/rocksdb-either-writer-ext
branch
from
8910828 to
126621c
Compare
Member
Author
|
Closing: this PR is empty (no commits between base and head). The EitherWriter extension functionality was merged into adjacent PRs. |
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
2 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 EitherWriter methods for history index operations with RocksDB support.
Changes
unwind_account_history_shards/unwind_storage_history_shardsappend_account_history_shard/append_storage_history_shardTesting
History operations verified.
PR Stack