Skip to content

feat(stages): add RocksDB helper functions for stage operations#21065

Closed
yongkangc wants to merge 4 commits intoyk/rocksdb-shard-logicfrom
yk/rocksdb-stage-utils
Closed

feat(stages): add RocksDB helper functions for stage operations#21065
yongkangc wants to merge 4 commits intoyk/rocksdb-shard-logicfrom
yk/rocksdb-stage-utils

Conversation

@yongkangc
Copy link
Member

@yongkangc yongkangc commented Jan 14, 2026

Summary
Helper functions for RocksDB integration in stage operations.

Changes

  • Add stage-level utilities for RocksDB operations
  • Support unwinding and rewinding of history indices

Testing
Stage operations verified.


PR Stack

main
  ↑
#21063 (rocksdb-either-reader)
  ↑
#21065 (rocksdb-stage-utils) ◀ you are here
  ↑
#21066 (rocksdb-index-history-stages)
  ↑
#21068 (rocksdb-init-txlookup)
  ↑
#21069 (rocksdb-cli-testutils)
  ↑
#21070 (rocksdb-cli)
  ↑
#21071 (rocksdb-docs)

…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.
@yongkangc yongkangc force-pushed the yk/rocksdb-stage-utils branch from d028b49 to fc776db Compare January 15, 2026 10:12
@yongkangc yongkangc changed the base branch from joshie/par-save-blocks-rocksdb to yk/rocksdb-shard-logic January 15, 2026 11:18
Copy link
Member

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

Code Review: PR #21065

CI Status

  • fmt, docs, crate-checks, unit tests failing

Changes Summary

Adds stage-level utilities for RocksDB operations including shard-and-write helpers and specialized loaders for account/storage history.

Review Notes

✅ Good

  • shard_and_write is a nice generic helper that reduces duplication
  • Clean integration with EitherWriter pattern
  • Proper handling of LoadMode::Flush vs LoadMode::KeepLast

🟡 Suggestions

  1. #[allow(dead_code)]: Multiple functions have this attribute. These should be removed once the functions are actually used by downstream PRs - verify they're used in #21066.

  2. Observability pattern: The progress logging pattern is good:

    let interval = (total_entries / 10).max(1);

    Consider extracting this into a reusable progress reporter helper.

  3. Error context: When decode_key fails, consider adding context about which entry failed for debugging.

Dependencies

Depends on #21063 and #21064 - review those first.


Review by @tempo_ai

@yongkangc yongkangc force-pushed the yk/rocksdb-shard-logic branch from 7a9e46f to 4b3b865 Compare January 16, 2026 08:35
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
@yongkangc yongkangc force-pushed the yk/rocksdb-stage-utils branch from fc776db to 057d87f Compare January 16, 2026 08:36
…metrics

- Add AccountsHistory and StoragesHistory to ROCKSDB_TABLES constant
  to fix panic when RocksDB consistency check accesses these tables
- Make record_operation gracefully handle unknown tables instead of
  panicking, preventing future table additions from breaking tests
- Improve shard_and_write performance by avoiding intermediate
  Vec<Vec<_>> allocation, instead iterating chunks directly
@yongkangc
Copy link
Member Author

Superseded by #21120 (combined with #21066)

@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.

2 participants