Skip to content

feat(stages): add RocksDB helper functions and unwind for index history stages#21120

Closed
yongkangc wants to merge 18 commits intoyk/rocksdb-either-readerfrom
yk/rocksdb-stage-utils-combined
Closed

feat(stages): add RocksDB helper functions and unwind for index history stages#21120
yongkangc wants to merge 18 commits intoyk/rocksdb-either-readerfrom
yk/rocksdb-stage-utils-combined

Conversation

@yongkangc
Copy link
Member

Summary
Add RocksDB integration for staged sync operations, combining helper utilities with index history stage unwind support.

Changes

Helper functions:

  • 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

Index history stage unwind:

  • IndexAccountHistory: RocksDB batch creation during unwind
  • IndexStorageHistory: RocksDB batch creation during unwind
  • EitherWriter for history shard operations
  • Register batches with pending_batches for atomic commit

Also fixes:

  • Register AccountsHistory and StoragesHistory tables in RocksDB metrics
  • Fix unresolved RocksDBProviderFactory link in either_writer docs

Testing
Stage operations and unwind verified.


PR Stack

main
  ↑
#21063 (rocksdb-either-reader)
  ↑
#THIS (rocksdb-stage-utils-combined) ◀ you are here
  ↑
#21068 (rocksdb-init-txlookup)
  ↑
#21069 (rocksdb-cli-testutils)
  ↑
#21070 (rocksdb-cli)
  ↑
#21071 (rocksdb-docs)

joshieDo and others added 18 commits January 13, 2026 16:48
Co-authored-by: Sergei Shulepov <pep@tempo.xyz>
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.
…ex history stages

Add RocksDB integration for staged sync operations:

Helper functions:
- 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

Index history stage unwind:
- IndexAccountHistory: RocksDB batch creation during unwind
- IndexStorageHistory: RocksDB batch creation during unwind
- EitherWriter for history shard operations
- Register batches with pending_batches for atomic commit

Also fixes:
- Register AccountsHistory and StoragesHistory tables in RocksDB metrics
- Fix unresolved RocksDBProviderFactory link in either_writer docs
@yongkangc yongkangc marked this pull request as draft January 16, 2026 13:51
@yongkangc yongkangc changed the base branch from yk/rocksdb-either-reader to main January 16, 2026 14:04
@yongkangc yongkangc added the A-rocksdb Related to rocksdb integration label Jan 16, 2026
@yongkangc yongkangc changed the base branch from main to yk/rocksdb-either-reader January 16, 2026 14:28
@yongkangc
Copy link
Member Author

Superseded by stacked PRs #21124 and #21125

@yongkangc yongkangc closed this Jan 16, 2026
@github-project-automation github-project-automation bot moved this from Backlog to Done in Reth Tracker Jan 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-rocksdb Related to rocksdb integration

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants