feat: add RocksDB variant to EitherReader and EitherWriter by yongkangc · Pull Request #20288 · paradigmxyz/reth

yongkangc · 2025-12-11T06:30:25Z

Closes #20278, #20279, #20280

Add RocksDB variants to EitherReader, EitherWriter, and EitherWriterDestination enums.

This is done by switching from DB to TransactionDB and add RocksTx wrapper providing MDBX-like transaction semantics.

Changes

Replace DB with TransactionDB in RocksDBProvider
Add RocksTx<'db> wrapper with get, put, delete, iter, commit, rollback
Update EitherWriter/EitherReader to use RocksTx instead of RocksDBProvider
Add tests for read-your-writes, rollback, and iterator behavior

yongkangc · 2025-12-11T08:20:16Z

crates/storage/provider/src/either_writer.rs

+            Ok(EitherReader::RocksDB(provider.rocksdb_provider()))
+        } else {
+            Ok(EitherReader::Database(provider.tx_ref().cursor_read::<tables::StoragesHistory>()?))
+        }


will have to add in constructor for account history after
#20282

is merged in

yongkangc · 2025-12-11T08:50:05Z

@Rjected might be obvious but just clarifying: we would still need implementation to add the actual operations (e.g., append, read, prune) for StoragesHistory and TransactionHashNumbers when using the RocksDB variant.

That hasn't been added for this pr.

yongkangc · 2025-12-11T08:58:46Z

crates/storage/provider/src/either_writer.rs

+
+    /// Creates a new [`EitherWriter`] for transaction hash numbers based on storage settings.
+    #[cfg(all(unix, feature = "rocksdb"))]
+    pub fn new_transaction_hash_numbers<P>(


is this what you mean by constructor? @Rjected

yep. I think we need to make sure this fn is not gated by cfg though

Done! Introduced type aliases (RocksTxArg, RocksTxRefArg) that resolve to RocksTx<'a> or &'a RocksTx<'a> when rocksdb is enabled, and () when disabled. The constructors are now always available - internal cfg blocks handle the routing.

joshieDo · 2025-12-11T13:02:11Z

crates/storage/provider/src/either_writer.rs

+    /// Write to `RocksDB`
+    #[cfg(all(unix, feature = "rocksdb"))]
+    RocksDB(RocksDBProvider),


we probably will want to use a rocksdb tx instead of just the provider

I see, could u expand on why we would wanna go with tx instead? @joshieDo

ideally we should explicitly control with DatabaseProvider::commit when we actually commit to disk and in what order. with an txn we can roll back and commit but not with a provider. this breaks API compatibility in terms of rollback which might be necessary for some parts

This PR is just for EitherReader / EitherWriter, so IMO we def want a rocksdb TX type in here. And we want to manage the tx lifecycle separately (at the provider level). I think it's fine if we use a rocksdb TX in the enums, even if this means the constructors like new_transaction_hash_numbers have to take a rocksdb TX argument that would be removed later

@joshieDo @Rjected made changes from DB to TransactionDB and added RocksTx wrapper.

This gives us the similar tx semantics as MDBX (read-your-writes, explicit commit/rollback, iterators seeing uncommitted writes). Let me know if that aligns with the design we should aim for.

## Problem Either* types needed RocksDB support for routing data to RocksDB-backed tables based on StorageSettings flags. ## Solution Add RocksDB variants and constructors to EitherReader, EitherWriter, and EitherWriterDestination enums. ## Changes - Add `EitherWriterDestination::RocksDB` variant - Add `EitherWriter::RocksDB` variant with constructors (cfg-gated): - `new_storages_history()`, `new_transaction_hash_numbers()` - Add `EitherReader::RocksDB` variant with constructors (cfg-gated): - `new_storages_history()`, `new_transaction_hash_numbers()` - Add `RocksDBProviderFactory` trait for provider access ## Expected Impact Enables routing write-heavy index tables (StoragesHistory, TransactionHashNumbers) to RocksDB when configured via StorageSettings. Closes #20278, #20279, #20280

Add RocksDB variants and constructors to EitherReader, EitherWriter, and EitherWriterDestination enums. **Changes** - Add `EitherWriterDestination::RocksDB` variant - Add `EitherWriter::RocksDB` variant (cfg-gated) - Add `EitherReader::RocksDB` variant (cfg-gated) - Add `new_storages_history()` constructor for EitherWriter/EitherReader - Add `new_transaction_hash_numbers()` constructor for EitherWriter/EitherReader Closes #20278, #20279, #20280

codspeed-hq · 2025-12-12T03:26:06Z

CodSpeed Performance Report

Merging #20288 will degrade performances by 32.51%

_{Comparing feat/either-reader-rocksdb (e00dd37) with main (3c41b99)}

Summary

❌ 1 regression
✅ 112 untouched
⏩ 4 skipped¹

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
❌	prefix set \| size: 10 \| `Vec` with binary search lookup	3.8 µs	5.7 µs	-32.51%

4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

Replace DB with TransactionDB to enable native transaction support with read-your-writes, commit/rollback, and iterators seeing uncommitted data. Add RocksTx wrapper that provides MDBX-compatible transaction API.

Use type aliases (RocksTxArg, RocksTxRefArg) to make constructors available regardless of feature flags. This allows callers to use these functions without conditional compilation.

fgimenez

lgtm, once this one is merged we will need to update #20253 to:

Remove RocksDBAccess enum: TransactionDB doesn't support read-only mode
Remove RocksDBBuilder::read_only() method: cannot be implemented with TransactionDB
Remove read-only access checks in put(), delete(), write_batch() that return ProviderError::ReadOnlyRocksDBAccess
Remove access field from RocksDBProviderInner struct

Rjected

LGTM, let's explore TransactionDB (or OptimisticTransactionDB) vs WriteBatch for read vs write in a follow up, if it makes sense

yongkangc · 2025-12-13T04:04:42Z

LGTM, let's explore TransactionDB (or OptimisticTransactionDB) vs WriteBatch for read vs write in a follow up, if it makes sense

Thanks will do!

yongkangc · 2025-12-13T04:48:44Z

lgtm, once this one is merged we will need to update #20253 to:

Remove RocksDBAccess enum: TransactionDB doesn't support read-only mode

Remove RocksDBBuilder::read_only() method: cannot be implemented with TransactionDB

Remove read-only access checks in put(), delete(), write_batch() that return ProviderError::ReadOnlyRocksDBAccess

Remove access field from RocksDBProviderInner struct

Roger thanks for note fede @fgimenez

github-project-automation bot added this to Reth Tracker Dec 11, 2025

github-project-automation bot moved this to Backlog in Reth Tracker Dec 11, 2025

github-actions bot added the C-enhancement New feature or request label Dec 11, 2025

yongkangc changed the title ~~Add RocksDB variant to EitherReader~~ feat: add RocksDB variant to EitherReader Dec 11, 2025

yongkangc added the A-db Related to the database label Dec 11, 2025

yongkangc self-assigned this Dec 11, 2025

yongkangc mentioned this pull request Dec 11, 2025

feat: add RocksDB variant to EitherWriter #20287

Closed

yongkangc force-pushed the feat/either-reader-rocksdb branch from fa993f3 to 02c1ea8 Compare December 11, 2025 07:55

yongkangc changed the title ~~feat: add RocksDB variant to EitherReader~~ feat: add RocksDB routing variants and constructors for Either* types Dec 11, 2025

This was referenced Dec 11, 2025

feat: add RocksDB destination to EitherWriter #20286

Closed

feat: add account_history_in_rocksdb field to StorageSettings #20282

Merged

yongkangc force-pushed the feat/either-reader-rocksdb branch from 02c1ea8 to b1571e5 Compare December 11, 2025 08:12

yongkangc changed the title ~~feat: add RocksDB routing variants and constructors for Either* types~~ feat: add RocksDB variant to EitherReader and EitherWriter Dec 11, 2025

yongkangc commented Dec 11, 2025

View reviewed changes

yongkangc force-pushed the feat/either-reader-rocksdb branch 2 times, most recently from ce435c6 to d4daf0e Compare December 11, 2025 08:41

yongkangc marked this pull request as ready for review December 11, 2025 08:43

yongkangc requested review from joshieDo, rakita and shekhirin as code owners December 11, 2025 08:43

yongkangc requested review from Rjected and mattsse December 11, 2025 08:43

yongkangc commented Dec 11, 2025

View reviewed changes

joshieDo reviewed Dec 11, 2025

View reviewed changes

yongkangc added 2 commits December 12, 2025 03:14

yongkangc force-pushed the feat/either-reader-rocksdb branch from d4daf0e to bb2b4dc Compare December 12, 2025 03:14

feat(storage): switch RocksDB to TransactionDB for MDBX-like semantics

d703c66

Replace DB with TransactionDB to enable native transaction support with read-your-writes, commit/rollback, and iterators seeing uncommitted data. Add RocksTx wrapper that provides MDBX-compatible transaction API.

yongkangc requested review from fgimenez and joshieDo December 12, 2025 09:59

yongkangc moved this from Backlog to In Progress in Reth Tracker Dec 12, 2025

yongkangc added 2 commits December 12, 2025 10:09

refactor: remove cfg gates from EitherWriter/EitherReader constructors

3b42a65

Use type aliases (RocksTxArg, RocksTxRefArg) to make constructors available regardless of feature flags. This allows callers to use these functions without conditional compilation.

clippy

e00dd37

fgimenez approved these changes Dec 12, 2025

View reviewed changes

fgimenez mentioned this pull request Dec 12, 2025

feat(storage): add method to check invariants on RocksDB tables #20340

Merged

Rjected approved these changes Dec 13, 2025

View reviewed changes

Rjected mentioned this pull request Dec 13, 2025

feat(storage): add rocksdb provider into database provider #20253

Merged

yongkangc added this pull request to the merge queue Dec 13, 2025

yongkangc mentioned this pull request Dec 13, 2025

Add in account history constructors to either writer and reader #20347

Closed

Merged via the queue into main with commit 5621132 Dec 13, 2025
43 of 47 checks passed

yongkangc deleted the feat/either-reader-rocksdb branch December 13, 2025 04:17

github-project-automation bot moved this from In Progress to Done in Reth Tracker Dec 13, 2025

Conversation

yongkangc commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yongkangc commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshieDo Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yongkangc Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yongkangc Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codspeed-hq bot commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #20288 will degrade performances by 32.51%

Summary

Benchmarks breakdown

Footnotes

Uh oh!

fgimenez left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rjected left a comment

Choose a reason for hiding this comment

Uh oh!

yongkangc commented Dec 13, 2025

Uh oh!

Uh oh!

yongkangc commented Dec 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yongkangc commented Dec 11, 2025 •

edited

Loading

yongkangc commented Dec 11, 2025 •

edited

Loading

joshieDo Dec 11, 2025 •

edited

Loading

yongkangc Dec 11, 2025 •

edited

Loading

yongkangc Dec 12, 2025 •

edited

Loading

codspeed-hq bot commented Dec 12, 2025 •

edited

Loading

fgimenez left a comment •

edited

Loading