feat(stages): add RocksDB support for IndexStorageHistoryStage#21175
Merged
feat(stages): add RocksDB support for IndexStorageHistoryStage#21175
Conversation
yongkangc
added
A-staged-sync
yongkangc
force-pushed
the
yk/rocksdb-index-storage-history-v3
branch
from
a104668 to
8d78f7d
Compare
yongkangc
requested review from
joshieDo,
mediocregopher,
rakita and
shekhirin
as code owners
fgimenez
reviewed
Jan 19, 2026
yongkangc
commented
Jan 20, 2026
| let total_entries = collector.len(); | ||
| let interval = (total_entries / 10).max(1); | ||
|
|
||
| for (index, element) in collector.iter()?.enumerate() { |
Member
Author
There was a problem hiding this comment.
we would def want to dedup the logic in the future with account history - but that should be in another pr
This implements RocksDB support for the IndexStorageHistoryStage following the pattern established in #21165 for IndexAccountHistoryStage: - Add RocksDBProviderFactory and StorageSettingsCache trait bounds to Stage impl - Use EitherWriter::new_storages_history with explicit #[cfg] blocks for batch creation - Add helper functions for loading/flushing storage history shards: - load_storage_history_via_writer - iterate collector, merge with existing shards - flush_storage_history_shards_partial - write full shards, keep partial in memory - flush_storage_history_shards - write all remaining shards with u64::MAX for last - Add EitherWriter methods: - append_storage_history, upsert_storage_history, get_last_storage_history_shard - unwind_storage_history_shard - handles RocksDB shard unwinding - Add RocksDBProvider::clear() and RocksDBBatch::get() methods - Route unwind to RocksDB when storages_history_in_rocksdb is enabled - Add rocksdb_tests module with tests for: - execute_writes_to_rocksdb_when_enabled - unwind_deletes_from_rocksdb_when_enabled - unwind_to_zero_keeps_block_zero - execute_incremental_sync Part of #20593 - Move secondary indices to RocksDB
- Add storage_history_shards() to RocksDBProvider for multi-shard enumeration - Add unwind_storage_history_to() and clear_storage_history() to RocksDBBatch - Update DatabaseProvider::unwind_storage_history_indices to route to RocksDB - Simplify stage unwind to delegate to provider (matching account history pattern) - Add multi-shard unwind test - Use checked_sub(1) for block 0 edge case - Remove now-unused unwind_storage_history_shard from EitherWriter
Use is_empty() check instead of separate count() to avoid double iteration, matching the unwind_account_history_to implementation.
- Rename load_storage_history_via_writer to load_storage_history (matching account history pattern) - Fix doc comment: 'account changesets' -> 'storage changesets' - Add inline comments throughout load_storage_history matching the account history style - Add doc comments for flush helpers referencing LoadMode equivalents - Add inline comments explaining shard flushing logic and u64::MAX invariant
- Add with_rocksdb_batch method to RocksDBProviderFactory trait - Refactor storage history stage to use with_rocksdb_batch like account history - This removes explicit #[cfg] blocks in the stage for cleaner code
- Add RocksDB clear() safety comment in storage history stage - Remove blank line between first_sync and use_rocksdb declarations - Add append_account_history, upsert_account_history, get_last_account_history_shard to EitherWriter for parity with account history PR - Update test to use upsert_account_history instead of put_account_history
- Remove unused load_history_indices, load_indices, and LoadMode (replaced by EitherWriter-based load_account_history and load_storage_history) - Fix storage_history_shards iterator to use self.0 instead of self.0.db - Update doc comments to remove references to deleted functions
yongkangc
force-pushed
the
yk/rocksdb-index-storage-history-v3
branch
from
afe3f7b to
17f7cbf
Compare
yongkangc
commented
Jan 20, 2026
| /// `Address.StorageKey`). It flushes indices to disk when reaching a shard's max length | ||
| /// (`NUM_OF_INDICES_IN_SHARD`) or when the partial key changes, ensuring the last previous partial | ||
| /// key shard is stored. | ||
| pub(crate) fn load_history_indices<Provider, H, P>( |
Member
Author
There was a problem hiding this comment.
we are removing this because it is not rocksdb generic. We now have
• load_account_history() for AccountsHistory
• load_storage_history() for StoragesHistory
Member
Author
There was a problem hiding this comment.
Also this implementation has been flagged by audit as bugged. Cc @klkvr slack thread on 154
Match the same pattern as unwind_account_history_indices - move the HashMap grouping and batch logic from DatabaseProvider into RocksDBProvider for consistency.
Member
Author
|
@joshieDo i plan to have a trait HistoryShardWriter to consolidate the functionality. but that has alot of changes so i just want to get indexhistory in first and send a pr to refactor when we know the functionality /logic is gucci |
joshieDo
approved these changes
Jan 21, 2026
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
3 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.
Part of #20390
Add RocksDB support to the storage history index stage using the
EitherWriterabstractionChanges
RocksDBProviderFactoryandStorageSettingsCachetrait bounds to Stage implEitherWriter::new_storages_history()to route writes to MDBX or RocksDBload_storage_history_via_writer, etc.)EitherWritermethods:append_storage_history,upsert_storage_history,get_last_storage_history_shardRocksDBProvider::clear()andRocksDBBatch::get()methodsCloses RETH-172
PR Stack