Skip to content

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

Merged
fgimenez merged 4 commits intoparadigmxyz:mainfrom
duyquang6:rocksdb-database-provider
Dec 15, 2025
Merged

feat(storage): add rocksdb provider into database provider#20253
fgimenez merged 4 commits intoparadigmxyz:mainfrom
duyquang6:rocksdb-database-provider

Conversation

@duyquang6
Copy link
Contributor

@duyquang6 duyquang6 commented Dec 10, 2025 •
edited
Loading

Resolve #20152

Changes:

  • rocksdb_stub.rs (stub for non-Unix, no feature), rocksdb_provider.rs (for unix, enabled rocksdb feature)
  • Added RocksDBProvider to ProviderFactory with builder pattern
  • Added /rocksdb directory alongside /db and /static_files
  • Integration: Updated provider factory builder, node launch, CLI commands, and test utilities

@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Dec 10, 2025
@duyquang6 duyquang6 force-pushed the rocksdb-database-provider branch 2 times, most recently from 615f6f6 to cb7f785 Compare December 10, 2025 11:05
duyquang6
duyquang6 commented Dec 10, 2025
View reviewed changes
crates/storage/provider/src/providers/rocksdb_stub.rs
Copy link
Contributor Author

@duyquang6 duyquang6 Dec 10, 2025

Choose a reason for hiding this comment

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

this is no-op version of rocksdb provider which help simplify integrate into DatabaseProvider

@duyquang6 duyquang6 force-pushed the rocksdb-database-provider branch 2 times, most recently from 3039b51 to 08722e9 Compare December 10, 2025 11:29
fgimenez
fgimenez reviewed Dec 10, 2025
View reviewed changes
Copy link
Member

@fgimenez fgimenez left a comment

Choose a reason for hiding this comment

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

looks good, some initial suggestions

@duyquang6 duyquang6 force-pushed the rocksdb-database-provider branch 2 times, most recently from 404ade4 to fbcd569 Compare December 11, 2025 06:51
@duyquang6 duyquang6 marked this pull request as ready for review December 11, 2025 07:14
@duyquang6
Copy link
Contributor Author

duyquang6 commented Dec 11, 2025

thank sir @fgimenez, I’ve pushed the fixes for the comments — it’s ready for review

yongkangc added a commit that referenced this pull request Dec 11, 2025
@yongkangc
feat: add RocksDB variant to EitherReader and EitherWriter
7fea7e3 
Add RocksDB variants to EitherReader, EitherWriter, and EitherWriterDestination enums.

**Changes**
- Add `EitherWriterDestination::RocksDB` variant
- Add `EitherWriter::RocksDB` variant (cfg-gated)
- Add `EitherReader::RocksDB` variant (cfg-gated)

**Note**: Constructors for RocksDB tables will be added in a follow-up PR
after #20253 (RocksDBProviderFactory) merges.

Closes #20278, #20279, #20280
@yongkangc yongkangc mentioned this pull request Dec 11, 2025
Merged
fgimenez
fgimenez approved these changes Dec 11, 2025
View reviewed changes
Copy link
Member

@fgimenez fgimenez left a comment

Choose a reason for hiding this comment

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

lgtm, pending @Rjected @joshieDo @shekhirin @yongkangc

@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Dec 11, 2025
Rjected
Rjected approved these changes Dec 13, 2025
View reviewed changes
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

This makes sense to me, let's merge #20288 first and rebase this one so it can use the tx utils

@duyquang6 duyquang6 marked this pull request as draft December 15, 2025 01:59
@duyquang6 duyquang6 force-pushed the rocksdb-database-provider branch from fbcd569 to 0ce6a2a Compare December 15, 2025 02:05
@duyquang6 duyquang6 force-pushed the rocksdb-database-provider branch from 0ce6a2a to 873bb06 Compare December 15, 2025 02:16
@duyquang6 duyquang6 marked this pull request as ready for review December 15, 2025 02:32
@duyquang6
Copy link
Contributor Author

duyquang6 commented Dec 15, 2025

I rebase this and fix as #20288 (review) suggest, it ready to review

@duyquang6 duyquang6 force-pushed the rocksdb-database-provider branch from 873bb06 to 609e884 Compare December 15, 2025 04:41
duyquang6
duyquang6 commented Dec 15, 2025
View reviewed changes
crates/cli/commands/src/common.rs
),
};
// TransactionDB only support read-write mode
let rocksdb_provider = RocksDBProvider::builder(data_dir.rocksdb())
Copy link
Contributor Author

@duyquang6 duyquang6 Dec 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

context: there is AccessRights::RW and AccessRights::RO, in case of rocksdb, there is only support read write mode, so, in AccessRights::RO, behind rocksdb will use read write mode

I'm not sure if this is what we want sir @fgimenez, as #20288 (review) suggest should remove readonly mode, so this code here adapt to it

Copy link
Member

@fgimenez fgimenez Dec 15, 2025

Choose a reason for hiding this comment

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

yes, this approach is correct, TransactionDB does not support RO and we always create RocksDB in RW no matter the value of AccessRights. AccessRights enum controls MDBX and StaticFiles access, not RocksDB.

One remaining thing, we can now remove the unused ReadOnlyRocsDBAccess error variant from crates/storage/errors/src/provider.rs, not used anywhere

Copy link
Contributor Author

@duyquang6 duyquang6 Dec 15, 2025

Choose a reason for hiding this comment

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

thanks, I fixed that 💯

@duyquang6 duyquang6 force-pushed the rocksdb-database-provider branch from 609e884 to 7b6f716 Compare December 15, 2025 04:48
@fgimenez fgimenez added this pull request to the merge queue Dec 15, 2025
Merged via the queue into paradigmxyz:main with commit 662c048 Dec 15, 2025
45 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Dec 15, 2025
yongkangc pushed a commit that referenced this pull request Dec 15, 2025
@duyquang6 @yongkangc
feat(storage): add rocksdb provider into database provider (#20253)
30738ec
@mattsse mattsse added M-changelog This change should be included in the changelog A-sdk Related to reth's use as a library labels Dec 15, 2025
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Jan 22, 2026
@duyquang6
feat(storage): add rocksdb provider into database provider (paradigmx…
b9630a0 
…yz/reth#20253)
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Feb 11, 2026
@duyquang6
feat(storage): add rocksdb provider into database provider (paradigmx…
7eb55c0 
…yz/reth#20253)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fgimenez fgimenez fgimenez approved these changes

@Rjected Rjected Rjected approved these changes

@joshieDo joshieDo joshieDo approved these changes

@rakita rakita Awaiting requested review from rakita

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

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

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

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

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

Assignees

No one assigned

Labels

A-sdk Related to reth's use as a library M-changelog This change should be included in the changelog

Projects

Reth Tracker
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add RocksDB Provider to DatabaseProvider

5 participants

@duyquang6 @fgimenez @Rjected @joshieDo @mattsse