Skip to content

perf(provider): optimize RocksDB history pruning with changeset-based approach#21358

Closed
gakonst wants to merge 13 commits intomainfrom
yk/rocksdb-sf-prune
Closed

perf(provider): optimize RocksDB history pruning with changeset-based approach#21358
gakonst wants to merge 13 commits intomainfrom
yk/rocksdb-sf-prune

Conversation

@gakonst
Copy link
Member

@gakonst gakonst commented Jan 23, 2026 •
edited
Loading

Summary

Optimizes RocksDB history pruning by using a changeset-based approach instead of full table scans.

Changes

  • Query changesets via provider abstraction (changed_storages_with_range / changed_accounts_with_range) which routes to static files or MDBX based on storage settings
  • Use changesets to identify affected (address, storage_key) pairs instead of iterating the entire StoragesHistory/AccountsHistory tables
  • Seek directly to relevant history entries using new iter_from() method
  • Add defensive check to verify no excess entries remain after optimized pruning
  • Fall back to full scan if changesets are incomplete

This significantly improves pruning performance for mainnet-scale databases.

Closes #20417

Based on PR #20674 by @meetrick

@meetrick @gakonst
perf(provider): optimize RocksDB history pruning with changeset-based…
d818329 
… approach

Use MDBX changesets to identify affected (address, storage_key) pairs
instead of iterating the entire StoragesHistory/AccountsHistory tables.

This significantly improves pruning performance for mainnet-scale databases.

Closes #20417

Signed-off-by: Hwangjae Lee <meetrick@gmail.com>
@meetrick @gakonst
fix(provider): add defensive check to RocksDB history pruning
3c25d43 
Verify no excess entries remain after optimized pruning via `last()`.
Falls back to full scan if entries are missed due to incomplete changesets.

Signed-off-by: Hwangjae Lee <meetrick@gmail.com>
@meetrick @gakonst
test(provider): add defensive check coverage for incomplete changesets
0e33590 
Add tests verifying the defensive check catches RocksDB entries
missed by changeset-based pruning when MDBX data is incomplete.
- test_storages_history_defensive_check_catches_missed_entries
- test_accounts_history_defensive_check_catches_missed_entries

Signed-off-by: Hwangjae Lee <meetrick@gmail.com>
@meetrick @gakonst
fixed clippy error for CI
2761024 
Signed-off-by: Hwangjae Lee <meetrick@gmail.com>
@meetrick @gakonst
Code clean: unnecessary comments
45a7829 
Signed-off-by: Hwangjae Lee <meetrick@gmail.com>
@meetrick @gakonst
fix(rocksdb): use iterator_cf method on RocksDBProviderInner enum
4afabe8 
The iter_from method was incorrectly accessing `.db` field directly on
RocksDBProviderInner, which is an enum and doesn't expose db as a public
field. This was introduced during merge conflict resolution. Use the
existing iterator_cf() helper method that properly handles both ReadWrite
and ReadOnly variants.

Signed-off-by: Hwangjae Lee <meetrick@gmail.com>
@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Jan 23, 2026
@github-actions github-actions bot added A-db Related to the database C-enhancement New feature or request labels Jan 23, 2026
@yongkangc yongkangc added the A-rocksdb Related to rocksdb integration label Jan 23, 2026
yongkangc
yongkangc reviewed Jan 23, 2026
View reviewed changes
yongkangc
yongkangc reviewed Jan 23, 2026
View reviewed changes
yongkangc
yongkangc reviewed Jan 23, 2026
View reviewed changes
crates/storage/provider/src/providers/rocksdb/invariants.rs
/// excess block range, then only deletes History entries for those specific accounts.
/// This is more efficient than iterating the whole table.
///
/// TODO(<https://github.com/paradigmxyz/reth/issues/20417>): this iterates the whole table,
Copy link
Member

@yongkangc yongkangc Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also can u verify logical equivalence?

Copy link
Member Author

@gakonst gakonst Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logical equivalence verified:

Before (original):

for result in self.iter::<tables::StoragesHistory>()? {
    let (key, _) = result?;
    let highest_block = key.sharded_key.highest_block_number;
    if max_block == 0 || (highest_block != u64::MAX && highest_block > max_block) {
        to_delete.push(key);
    }
}

After (optimized path with changeset-based approach + fallback):

Case Original Behavior New Behavior
max_block == 0 Deletes all except sentinels Calls prune_*_all() which deletes all except sentinels ✅
highest_block > max_block (with changesets) Full scan, delete matching Changeset lookup → seek → delete matching ✅
highest_block > max_block (no changesets) Full scan Falls back to full scan ✅
Sentinel entries (u64::MAX) Preserved Preserved ✅
highest_block <= max_block Not deleted Not deleted ✅

The defensive check after the optimized path ensures any entries missed by changesets are still caught via full scan fallback.

@gakonst
fix: address review comments
261db5a 
- Restore accidentally deleted methods in provider.rs (table_stats, raw_iter, account_history_shards, storage_history_shards, unwind_account_history_indices, unwind_storage_history_indices)
- Move imports to top of file in invariants.rs
- Remove redundant comments
- Simplify variable patterns (use !is_empty() instead of len() > 0)
- Use HashSet for O(1) deduplication in defensive check
yongkangc
yongkangc reviewed Jan 23, 2026
View reviewed changes
@gakonst
docs: clarify that changeset queries route to static files when enabled
d8d7da0 
Update docstrings to reflect that provider.changed_storages_with_range() and
provider.changed_accounts_with_range() correctly route to static files when
storage_changesets_in_static_files / account_changesets_in_static_files are
enabled, rather than always querying MDBX.
@gakonst
docs: fix documentation drift in pruning functions
9667546 
- Clarify that changeset queries route to static files or MDBX based on storage settings
- Clarify that _all() functions preserve sentinel entries (u64::MAX)
- Update 'clearing all entries' to 'clearing all non-sentinel entries'
@gakonst gakonst force-pushed the yk/rocksdb-sf-prune branch from 24d2326 to 9667546 Compare January 23, 2026 12:52
yongkangc
yongkangc reviewed Jan 23, 2026
View reviewed changes
@yongkangc yongkangc marked this pull request as ready for review January 23, 2026 13:30
@yongkangc yongkangc requested a review from joshieDo as a code owner January 23, 2026 13:30
@yongkangc yongkangc requested a review from shekhirin as a code owner January 23, 2026 13:30
@yongkangc yongkangc requested a review from Rjected January 23, 2026 13:30
@gakonst
refactor: simplify defensive check comments
407bbd4 
Address review feedback - make comments more clear and concise
@gakonst
fix: replace last() with early-exit scan in defensive check
ca663c0 
The previous defensive check used last() to detect missed entries, but with
key ordering (address, storage_key, block), the lexicographically last key
may not have the highest block number. This could cause entries to be missed.

Now we scan all entries and early-exit on the first missed entry, then fall
back to full scan only if needed. This is correct regardless of key ordering.
@gakonst
perf: remove defensive scan that negated optimization
8e18e6b 
The defensive full-table scan was running on every prune, making the
changeset-based optimization pointless (O(changeset) + O(table) = O(table)).

Changesets are authoritative for which entries changed - if they're incomplete,
that's a data integrity issue that should be caught elsewhere. The fallback
to full scan when changesets are empty handles the edge cases correctly.
@gakonst gakonst force-pushed the yk/rocksdb-sf-prune branch from 5fa2be2 to 2ef8cf5 Compare January 23, 2026 16:29
@gakonst
fix: add defensive check after changeset-based pruning
b84b368 
The tests expected a defensive verification pass after the optimized
changeset-based pruning path to catch entries that were missed due to
incomplete changesets (e.g., entries in RocksDB that don't have
corresponding MDBX/static file changesets).

This adds a full table scan after the optimized path to ensure no
entries are missed, using HashSet deduplication to avoid double-deletes.
@gakonst gakonst force-pushed the yk/rocksdb-sf-prune branch from 2ef8cf5 to b84b368 Compare January 23, 2026 16:41
@gakonst
Copy link
Member Author

gakonst commented Jan 24, 2026

Closing this PR - the main branch already has the correct changeset-based implementation.

Analysis:

The current main branch heal_storages_history and heal_accounts_history functions already use:

  • provider.storage_changesets_range() / provider.account_changesets_range()
  • Routes through EitherReader → static files when storage_changesets_in_static_files / account_changesets_in_static_files is enabled
  • Batched processing (10,000 blocks per batch)
  • Targeted unwind_*_history_indices() instead of full table scans

This PR regresses the implementation by:

  • Replacing heal_* with check_* / prune_* functions
  • Using full table iteration via self.iter::<tables::StoragesHistory>()
  • Removing changeset queries entirely
  • Removing static file integration

The issue #20417 has already been addressed on main via the static file changeset integration (PR #18882).

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

Reviewers

@yongkangc yongkangc yongkangc left review comments

@joshieDo joshieDo Awaiting requested review from joshieDo joshieDo is a code owner

@shekhirin shekhirin Awaiting requested review from shekhirin shekhirin is a code owner

@Rjected Rjected Awaiting requested review from Rjected

Assignees

@yongkangc yongkangc

Labels

A-db Related to the database A-rocksdb Related to rocksdb integration C-enhancement New feature or request

Projects

Reth Tracker
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use changeset-based pruning for StoragesHistory and AccountHistory in RocksDB

3 participants

@gakonst @yongkangc @meetrick