Skip to content

feat(cli): add --rocksdb.* flags for RocksDB table routing#21191

Merged
yongkangc merged 20 commits intomainfrom
feat/rocksdb-cli-v2
Jan 20, 2026
Merged

feat(cli): add --rocksdb.* flags for RocksDB table routing#21191
yongkangc merged 20 commits intomainfrom
feat/rocksdb-cli-v2

Conversation

@yongkangc
Copy link
Copy Markdown
Member

@yongkangc yongkangc commented Jan 19, 2026

Summary

Introduces RocksDbArgs struct with dedicated --rocksdb.* prefix for RocksDB table routing.

Flags

  • --rocksdb.tx-hash - Route tx hash -> number table to RocksDB
  • --rocksdb.storages-history - Route storages history to RocksDB
  • --rocksdb.account-history - Route account history to RocksDB

All flags are genesis-initialization-only (changing later requires re-sync).

PR Stack

  1. This PR - CLI flags
  2. Validation logic (depends on this)
  3. Docs (independent)

closes #20393

Introduces RocksDbArgs struct with --rocksdb.tx-hash, --rocksdb.storages-history,
and --rocksdb.account-history flags for controlling which tables route to RocksDB.

These flags are genesis-initialization-only (changing later requires re-sync).
- Add test for bare flag without value (should require a value)
- Add test for space-separated value syntax
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Jan 19, 2026

Merging this PR will not alter performance

✅ 118 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing feat/rocksdb-cli-v2 (0d8219f) with main (3ba3708)

Open in CodSpeed

Footnotes

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

- Add rocksdb field to NodeConfig
- Update init_genesis to apply RocksDB settings on top of static files settings
- All struct initializers and Clone impl updated
@yongkangc yongkangc force-pushed the feat/rocksdb-cli-v2 branch from fc5746f to cfa9a4a Compare January 19, 2026 15:17
@yongkangc yongkangc self-assigned this Jan 19, 2026
@yongkangc yongkangc added A-db Related to the database A-cli Related to the reth CLI A-rocksdb Related to rocksdb integration labels Jan 19, 2026
@yongkangc yongkangc requested review from Rjected and fgimenez January 19, 2026 17:41
Wire RocksDbArgs through NodeCommand to enable CLI flags to be passed
through to NodeConfig. Also fix clippy warnings for doc_markdown
(backticks around RocksDB) and missing_const_for_fn.
@yongkangc yongkangc requested a review from joshieDo January 19, 2026 17:46
@yongkangc yongkangc marked this pull request as ready for review January 19, 2026 18:01
Consolidate 12 tests down to 3 essential ones covering defaults,
parsing, and settings application.
- If any --rocksdb.* flag is set to true, all RocksDB tables are enabled
- Explicit --rocksdb.<table>=false overrides can disable specific tables
- Logs startup message when grouped behavior triggers
- DRY'd doc comments (genesis-only note moved to struct level)
- Added comprehensive tests for grouped enable behavior
- Add test for all-explicit-false edge case
- Add test verifying non-rocksdb fields preserved
- Rename parameter defaults -> settings for clarity
- Extract storage_settings_with_logging() helper to DRY up with_genesis/init_genesis
- Fix clippy doc warnings (backticks around RocksDB)
args: T,
}

#[test]
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

default is None

Encapsulates the storage settings derivation logic that combines
static_files.to_settings() with rocksdb.apply_to_settings() into
a single method, as suggested in PR review.
@yongkangc
Copy link
Copy Markdown
Member Author

@joshieDo refactored into the function storage_settings() based on your comment

@yongkangc yongkangc enabled auto-merge January 20, 2026 12:04
@yongkangc yongkangc requested a review from rakita as a code owner January 20, 2026 14:56
@yongkangc yongkangc force-pushed the feat/rocksdb-cli-v2 branch from b26d54d to 8ebf7fc Compare January 20, 2026 17:43
- Add StorageSettings::merge() to combine settings
- Change RocksDbArgs::apply_to_settings() to to_settings()
- Update NodeConfig::storage_settings() to use the cleaner pattern:
  self.static_files.to_settings().merge(self.rocksdb.to_settings())
@yongkangc yongkangc force-pushed the feat/rocksdb-cli-v2 branch from 8ebf7fc to 3972cd0 Compare January 20, 2026 17:48
}

/// Returns the effective storage settings derived from static-file and `RocksDB` CLI args.
pub fn storage_settings(&self) -> StorageSettings {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we can do it in this PR or on a follow-up, but we probably want to remove static_files.to_settings() i think ?

and we should make sure that the following behaviour introduced here
https://github.com/paradigmxyz/reth/pull/21208/files also applies to rocksdb

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

makes sense, think we might want to do it in a separate pr - will be on this

@yongkangc yongkangc requested review from Rjected and joshieDo January 20, 2026 19:06
gakonst added a commit that referenced this pull request Jan 20, 2026
…ocksDB flags

- Remove StaticFilesArgs::to_settings() method as it's superseded by
  NodeConfig::storage_settings() which handles both static files and RocksDB
- Add edge feature behavior to RocksDbArgs flags, matching the pattern from #21208
  - Individual flags now default to true when 'edge' feature is enabled
  - Changed flags from Option<bool> to bool with proper defaults
- Add RocksDbArgs to EnvironmentArgs in cli/commands for consistent storage settings
- Update all callers to use storage_settings() instead of static_files.to_settings()

Closes comment from #21191 (PR discussion r2709676364)
Copy link
Copy Markdown
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.

lgtm

@yongkangc yongkangc added this pull request to the merge queue Jan 20, 2026
@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Jan 20, 2026
Merged via the queue into main with commit bc79cc4 Jan 20, 2026
45 checks passed
@yongkangc yongkangc deleted the feat/rocksdb-cli-v2 branch January 20, 2026 19:40
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Jan 20, 2026
gakonst added a commit that referenced this pull request Jan 21, 2026
…ocksDB flags

This PR addresses the review comment from #21191:

- Remove `StaticFilesArgs::to_settings()` - superseded by `NodeConfig::storage_settings()`
- Add edge feature behavior to RocksDB flags - flags default to `true` when `edge` feature is enabled
- Add `RocksDbArgs` to `EnvironmentArgs` with `storage_settings()` method
- Update `storage_settings()` to use `StorageSettings::edge()` or `StorageSettings::legacy()` base
- Replace `static_files.to_settings()` calls with `storage_settings()` in launch/common.rs
gakonst added a commit that referenced this pull request Jan 21, 2026
…ocksDB flags

This PR addresses the review comment from #21191:

- Remove `StaticFilesArgs::to_settings()` - superseded by `NodeConfig::storage_settings()`
- Add edge feature behavior to RocksDB flags - flags default to `true` when `edge` feature is enabled
- Add `RocksDbArgs` to `EnvironmentArgs` with `storage_settings()` method
- Update `storage_settings()` to use `StorageSettings::edge()` or `StorageSettings::legacy()` base
- Replace `static_files.to_settings()` calls with `storage_settings()` in launch/common.rs
gakonst added a commit that referenced this pull request Jan 21, 2026
…ocksDB flags

This PR addresses the review comment from #21191:

- Remove `StaticFilesArgs::to_settings()` - superseded by `NodeConfig::storage_settings()`
- Add edge feature behavior to RocksDB flags - flags default to `true` when `edge` feature is enabled
- Add `RocksDbArgs` to `EnvironmentArgs` with `storage_settings()` method
- Update `storage_settings()` to use `StorageSettings::edge()` or `StorageSettings::legacy()` base
- Replace `static_files.to_settings()` calls with `storage_settings()` in launch/common.rs
gakonst added a commit that referenced this pull request Jan 21, 2026
…ocksDB flags

This PR addresses the review comment from #21191:

- Remove `StaticFilesArgs::to_settings()` - superseded by `NodeConfig::storage_settings()`
- Add edge feature behavior to RocksDB flags - flags default to `true` when `edge` feature is enabled
- Add `RocksDbArgs` to `EnvironmentArgs` with `storage_settings()` method
- Update `storage_settings()` to use `StorageSettings::edge()` or `StorageSettings::legacy()` base
- Replace `static_files.to_settings()` calls with `storage_settings()` in launch/common.rs
gakonst added a commit that referenced this pull request Jan 21, 2026
…ocksDB flags

This PR addresses the review comment from #21191:

- Remove `StaticFilesArgs::to_settings()` - superseded by `NodeConfig::storage_settings()`
- Add edge feature behavior to RocksDB flags - flags default to `true` when `edge` feature is enabled
- Add `RocksDbArgs` to `EnvironmentArgs` with `storage_settings()` method
- Update `storage_settings()` to use `StorageSettings::edge()` or `StorageSettings::legacy()` base
- Replace `static_files.to_settings()` calls with `storage_settings()` in launch/common.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Related to the reth CLI A-db Related to the database A-rocksdb Related to rocksdb integration

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Add CLI flags to enable RocksDB storage

3 participants