-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat(cli): add --rocksdb.* flags for RocksDB table routing #21191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
25f98a7
5166a6c
cfa9a4a
bf13a52
1aad483
df237b9
9e2d423
5f8dd4b
aa0f9e8
3905e31
09cc552
662684a
53940b9
0c8d650
3972cd0
8e64a3f
c1a323f
ee64133
dcb9691
0d8219f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| //! clap [Args](clap::Args) for `RocksDB` table routing configuration | ||
|
|
||
| use clap::{ArgAction, Args}; | ||
|
|
||
| /// Parameters for `RocksDB` table routing configuration. | ||
| /// | ||
| /// These flags control which database tables are stored in `RocksDB` instead of MDBX. | ||
| /// All flags are genesis-initialization-only: changing them after genesis requires a re-sync. | ||
| #[derive(Debug, Args, PartialEq, Eq, Default, Clone, Copy)] | ||
| #[command(next_help_heading = "RocksDB")] | ||
| pub struct RocksDbArgs { | ||
| /// Route all supported tables to `RocksDB` instead of MDBX. | ||
| /// | ||
| /// This enables `RocksDB` for `tx-hash`, `storages-history`, and `account-history` tables. | ||
| /// Cannot be combined with individual flags set to false. | ||
| #[arg(long = "rocksdb.all", action = ArgAction::SetTrue)] | ||
| pub all: bool, | ||
|
|
||
| /// Route tx hash -> number table to `RocksDB` instead of MDBX. | ||
| #[arg(long = "rocksdb.tx-hash", action = ArgAction::Set)] | ||
| pub tx_hash: Option<bool>, | ||
|
|
||
| /// Route storages history tables to `RocksDB` instead of MDBX. | ||
| #[arg(long = "rocksdb.storages-history", action = ArgAction::Set)] | ||
| pub storages_history: Option<bool>, | ||
|
|
||
| /// Route account history tables to `RocksDB` instead of MDBX. | ||
| #[arg(long = "rocksdb.account-history", action = ArgAction::Set)] | ||
| pub account_history: Option<bool>, | ||
| } | ||
|
|
||
| impl RocksDbArgs { | ||
| /// Validates the `RocksDB` arguments. | ||
| /// | ||
| /// Returns an error if `--rocksdb.all` is used with any individual flag set to `false`. | ||
| pub fn validate(&self) -> Result<(), RocksDbArgsError> { | ||
| if self.all { | ||
| if self.tx_hash == Some(false) { | ||
| return Err(RocksDbArgsError::ConflictingFlags("tx-hash")); | ||
| } | ||
| if self.storages_history == Some(false) { | ||
| return Err(RocksDbArgsError::ConflictingFlags("storages-history")); | ||
| } | ||
| if self.account_history == Some(false) { | ||
| return Err(RocksDbArgsError::ConflictingFlags("account-history")); | ||
| } | ||
| } | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| /// Error type for `RocksDB` argument validation. | ||
| #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] | ||
| pub enum RocksDbArgsError { | ||
| /// `--rocksdb.all` cannot be combined with an individual flag set to false. | ||
| #[error("--rocksdb.all cannot be combined with --rocksdb.{0}=false")] | ||
| ConflictingFlags(&'static str), | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use clap::Parser; | ||
|
|
||
| #[derive(Parser)] | ||
| struct CommandParser<T: Args> { | ||
| #[command(flatten)] | ||
| args: T, | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_default_rocksdb_args() { | ||
| let args = CommandParser::<RocksDbArgs>::parse_from(["reth"]).args; | ||
| assert_eq!(args, RocksDbArgs::default()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_parse_all_flag() { | ||
| let args = CommandParser::<RocksDbArgs>::parse_from(["reth", "--rocksdb.all"]).args; | ||
| assert!(args.all); | ||
| assert_eq!(args.tx_hash, None); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_parse_individual_flags() { | ||
| let args = CommandParser::<RocksDbArgs>::parse_from([ | ||
| "reth", | ||
| "--rocksdb.tx-hash=true", | ||
| "--rocksdb.storages-history=false", | ||
| "--rocksdb.account-history=true", | ||
| ]) | ||
| .args; | ||
| assert!(!args.all); | ||
| assert_eq!(args.tx_hash, Some(true)); | ||
| assert_eq!(args.storages_history, Some(false)); | ||
| assert_eq!(args.account_history, Some(true)); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_validate_all_alone_ok() { | ||
| let args = RocksDbArgs { all: true, ..Default::default() }; | ||
| assert!(args.validate().is_ok()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_validate_all_with_true_ok() { | ||
| let args = RocksDbArgs { all: true, tx_hash: Some(true), ..Default::default() }; | ||
| assert!(args.validate().is_ok()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_validate_all_with_false_errors() { | ||
| let args = RocksDbArgs { all: true, tx_hash: Some(false), ..Default::default() }; | ||
| assert_eq!(args.validate(), Err(RocksDbArgsError::ConflictingFlags("tx-hash"))); | ||
|
|
||
| let args = RocksDbArgs { all: true, storages_history: Some(false), ..Default::default() }; | ||
| assert_eq!(args.validate(), Err(RocksDbArgsError::ConflictingFlags("storages-history"))); | ||
|
|
||
| let args = RocksDbArgs { all: true, account_history: Some(false), ..Default::default() }; | ||
| assert_eq!(args.validate(), Err(RocksDbArgsError::ConflictingFlags("account-history"))); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
| use crate::{ | ||
| args::{ | ||
| DatabaseArgs, DatadirArgs, DebugArgs, DevArgs, EngineArgs, NetworkArgs, PayloadBuilderArgs, | ||
| PruningArgs, RpcServerArgs, StaticFilesArgs, TxPoolArgs, | ||
| PruningArgs, RocksDbArgs, RpcServerArgs, StaticFilesArgs, TxPoolArgs, | ||
| }, | ||
| dirs::{ChainPath, DataDirPath}, | ||
| utils::get_single_header, | ||
|
|
@@ -21,6 +21,7 @@ use reth_primitives_traits::SealedHeader; | |
| use reth_stages_types::StageId; | ||
| use reth_storage_api::{ | ||
| BlockHashReader, DatabaseProviderFactory, HeaderProvider, StageCheckpointReader, | ||
| StorageSettings, | ||
| }; | ||
| use reth_storage_errors::provider::ProviderResult; | ||
| use reth_transaction_pool::TransactionPool; | ||
|
|
@@ -150,6 +151,9 @@ pub struct NodeConfig<ChainSpec> { | |
|
|
||
| /// All static files related arguments | ||
| pub static_files: StaticFilesArgs, | ||
|
|
||
| /// All `RocksDB` table routing arguments | ||
| pub rocksdb: RocksDbArgs, | ||
| } | ||
|
|
||
| impl NodeConfig<ChainSpec> { | ||
|
|
@@ -181,6 +185,7 @@ impl<ChainSpec> NodeConfig<ChainSpec> { | |
| engine: EngineArgs::default(), | ||
| era: EraArgs::default(), | ||
| static_files: StaticFilesArgs::default(), | ||
| rocksdb: RocksDbArgs::default(), | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -255,6 +260,7 @@ impl<ChainSpec> NodeConfig<ChainSpec> { | |
| engine, | ||
| era, | ||
| static_files, | ||
| rocksdb, | ||
| .. | ||
| } = self; | ||
| NodeConfig { | ||
|
|
@@ -274,6 +280,7 @@ impl<ChainSpec> NodeConfig<ChainSpec> { | |
| engine, | ||
| era, | ||
| static_files, | ||
| rocksdb, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -350,6 +357,22 @@ impl<ChainSpec> NodeConfig<ChainSpec> { | |
| self.pruning.prune_config(&self.chain) | ||
| } | ||
|
|
||
| /// Returns the effective storage settings derived from static-file and `RocksDB` CLI args. | ||
| pub fn storage_settings(&self) -> StorageSettings { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| let tx_hash = self.rocksdb.all || self.rocksdb.tx_hash.unwrap_or(false); | ||
| let storages_history = self.rocksdb.all || self.rocksdb.storages_history.unwrap_or(false); | ||
| let account_history = self.rocksdb.all || self.rocksdb.account_history.unwrap_or(false); | ||
|
|
||
| StorageSettings { | ||
| receipts_in_static_files: self.static_files.receipts, | ||
| transaction_senders_in_static_files: self.static_files.transaction_senders, | ||
| account_changesets_in_static_files: self.static_files.account_changesets, | ||
| transaction_hash_numbers_in_rocksdb: tx_hash, | ||
| storages_history_in_rocksdb: storages_history, | ||
| account_history_in_rocksdb: account_history, | ||
| } | ||
| } | ||
|
|
||
| /// Returns the max block that the node should run to, looking it up from the network if | ||
| /// necessary | ||
| pub async fn max_block<Provider, Client>( | ||
|
|
@@ -544,6 +567,7 @@ impl<ChainSpec> NodeConfig<ChainSpec> { | |
| engine: self.engine, | ||
| era: self.era, | ||
| static_files: self.static_files, | ||
| rocksdb: self.rocksdb, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -585,6 +609,7 @@ impl<ChainSpec> Clone for NodeConfig<ChainSpec> { | |
| engine: self.engine.clone(), | ||
| era: self.era.clone(), | ||
| static_files: self.static_files, | ||
| rocksdb: self.rocksdb, | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default is None