From c304c45b60908a0f650916060ea36762477cb310 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Sat, 28 May 2022 06:36:21 +0300 Subject: [PATCH 1/3] Make it possible to disable RocksDB completely --- client/cli/Cargo.toml | 5 ++++- client/cli/src/arg_enums.rs | 29 +++++++++++++++++++------ client/cli/src/config.rs | 14 +++++++++++- client/db/src/lib.rs | 20 +++++++++++++---- client/db/src/utils.rs | 2 ++ client/service/Cargo.toml | 4 +++- test-utils/client/Cargo.toml | 2 +- utils/frame/benchmarking-cli/Cargo.toml | 11 ++++++---- 8 files changed, 68 insertions(+), 19 deletions(-) diff --git a/client/cli/Cargo.toml b/client/cli/Cargo.toml index 456489e5f6639..a5b575bcee321 100644 --- a/client/cli/Cargo.toml +++ b/client/cli/Cargo.toml @@ -31,7 +31,7 @@ thiserror = "1.0.30" tiny-bip39 = "0.8.2" tokio = { version = "1.17.0", features = ["signal", "rt-multi-thread", "parking_lot"] } sc-client-api = { version = "4.0.0-dev", path = "../api" } -sc-client-db = { version = "0.10.0-dev", path = "../db" } +sc-client-db = { version = "0.10.0-dev", default-features = false, path = "../db" } sc-keystore = { version = "4.0.0-dev", path = "../keystore" } sc-network = { version = "0.10.0-dev", path = "../network" } sc-service = { version = "0.10.0-dev", default-features = false, path = "../service" } @@ -50,4 +50,7 @@ sp-version = { version = "5.0.0", path = "../../primitives/version" } tempfile = "3.1.0" [features] +default = ["with-parity-db", "with-rocks-db"] +with-parity-db = ["sc-client-db/with-parity-db"] +with-rocks-db = ["sc-client-db/with-kvdb-rocksdb"] wasmtime = ["sc-service/wasmtime"] diff --git a/client/cli/src/arg_enums.rs b/client/cli/src/arg_enums.rs index bc0989cf34659..cdf7a779fdbce 100644 --- a/client/cli/src/arg_enums.rs +++ b/client/cli/src/arg_enums.rs @@ -237,13 +237,16 @@ impl Into for RpcMethods { /// Database backend #[derive(Debug, Clone, PartialEq, Copy)] pub enum Database { + #[cfg(feature = "with-rocks-db")] /// Facebooks RocksDB RocksDb, + #[cfg(feature = "with-parity-db")] /// ParityDb. ParityDb, /// Detect whether there is an existing database. Use it, if there is, if not, create new /// instance of ParityDb Auto, + #[cfg(feature = "with-parity-db")] /// ParityDb. ParityDbDeprecated, } @@ -252,13 +255,17 @@ impl std::str::FromStr for Database { type Err = String; fn from_str(s: &str) -> Result { + #[cfg(feature = "with-rocks-db")] if s.eq_ignore_ascii_case("rocksdb") { - Ok(Self::RocksDb) - } else if s.eq_ignore_ascii_case("paritydb-experimental") { - Ok(Self::ParityDbDeprecated) + return Ok(Self::RocksDb) + } + #[cfg(feature = "with-parity-db")] + if s.eq_ignore_ascii_case("paritydb-experimental") { + return Ok(Self::ParityDbDeprecated) } else if s.eq_ignore_ascii_case("paritydb") { - Ok(Self::ParityDb) - } else if s.eq_ignore_ascii_case("auto") { + return Ok(Self::ParityDb) + } + if s.eq_ignore_ascii_case("auto") { Ok(Self::Auto) } else { Err(format!("Unknown variant `{}`, known variants: {:?}", s, Self::variants())) @@ -268,8 +275,16 @@ impl std::str::FromStr for Database { impl Database { /// Returns all the variants of this enum to be shown in the cli. - pub fn variants() -> &'static [&'static str] { - &["rocksdb", "paritydb", "paritydb-experimental", "auto"] + pub const fn variants() -> &'static [&'static str] { + &[ + #[cfg(feature = "with-rocks-db")] + "rocksdb", + #[cfg(feature = "with-parity-db")] + "paritydb", + #[cfg(feature = "with-parity-db")] + "paritydb-experimental", + "auto", + ] } } diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index 5e91cf6c74dae..7ae369b167d41 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -220,8 +220,11 @@ pub trait CliConfiguration: Sized { let rocksdb_path = base_path.join("db").join(role_dir); let paritydb_path = base_path.join("paritydb").join(role_dir); Ok(match database { + #[cfg(feature = "with-rocks-db")] Database::RocksDb => DatabaseSource::RocksDb { path: rocksdb_path, cache_size }, + #[cfg(feature = "with-parity-db")] Database::ParityDb => DatabaseSource::ParityDb { path: paritydb_path }, + #[cfg(feature = "with-parity-db")] Database::ParityDbDeprecated => { eprintln!( "WARNING: \"paritydb-experimental\" database setting is deprecated and will be removed in future releases. \ @@ -500,7 +503,16 @@ pub trait CliConfiguration: Sized { let net_config_dir = config_dir.join(DEFAULT_NETWORK_CONFIG_PATH); let client_id = C::client_id(); let database_cache_size = self.database_cache_size()?.unwrap_or(1024); - let database = self.database()?.unwrap_or(Database::RocksDb); + let database = self.database()?.unwrap_or( + #[cfg(feature = "with-rocks-db")] + { + Database::RocksDb + }, + #[cfg(all(feature = "with-parity-db", not(feature = "with-rocks-db")))] + { + Database::ParityDb + }, + ); let node_key = self.node_key(&net_config_dir)?; let role = self.role(is_dev)?; let max_runtime_instances = self.max_runtime_instances()?.unwrap_or(8); diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index a32a666c3c980..19e6ca2d5d859 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -30,7 +30,7 @@ pub mod offchain; -#[cfg(any(feature = "with-kvdb-rocksdb", test))] +#[cfg(any(feature = "with-parity-db", feature = "with-kvdb-rocksdb", test))] pub mod bench; mod children; @@ -94,7 +94,7 @@ use sp_trie::{prefixed_key, MemoryDB, PrefixedMemoryDB}; pub use sc_state_db::PruningMode; pub use sp_database::Database; -#[cfg(any(feature = "with-kvdb-rocksdb", test))] +#[cfg(any(feature = "with-parity-db", feature = "with-kvdb-rocksdb", test))] pub use bench::BenchmarkingState; const CACHE_HEADERS: usize = 8; @@ -327,6 +327,7 @@ pub enum DatabaseSource { cache_size: usize, }, /// Load a RocksDB database from a given path. Recommended for most uses. + #[cfg(feature = "with-kvdb-rocksdb")] RocksDb { /// Path to the database. path: PathBuf, @@ -335,6 +336,7 @@ pub enum DatabaseSource { }, /// Load a ParityDb database from a given path. + #[cfg(feature = "with-parity-db")] ParityDb { /// Path to the database. path: PathBuf, @@ -359,7 +361,10 @@ impl DatabaseSource { // IIUC this is needed for polkadot to create its own dbs, so until it can use parity db // I would think rocksdb, but later parity-db. DatabaseSource::Auto { paritydb_path, .. } => Some(paritydb_path), - DatabaseSource::RocksDb { path, .. } | DatabaseSource::ParityDb { path } => Some(path), + #[cfg(feature = "with-kvdb-rocksdb")] + DatabaseSource::RocksDb { path, .. } => Some(path), + #[cfg(feature = "with-parity-db")] + DatabaseSource::ParityDb { path } => Some(path), DatabaseSource::Custom { .. } => None, } } @@ -371,7 +376,12 @@ impl DatabaseSource { *paritydb_path = p.into(); true }, - DatabaseSource::RocksDb { ref mut path, .. } | + #[cfg(feature = "with-kvdb-rocksdb")] + DatabaseSource::RocksDb { ref mut path, .. } => { + *path = p.into(); + true + }, + #[cfg(feature = "with-parity-db")] DatabaseSource::ParityDb { ref mut path } => { *path = p.into(); true @@ -385,7 +395,9 @@ impl std::fmt::Display for DatabaseSource { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let name = match self { DatabaseSource::Auto { .. } => "Auto", + #[cfg(feature = "with-kvdb-rocksdb")] DatabaseSource::RocksDb { .. } => "RocksDb", + #[cfg(feature = "with-parity-db")] DatabaseSource::ParityDb { .. } => "ParityDb", DatabaseSource::Custom { .. } => "Custom", }; diff --git a/client/db/src/utils.rs b/client/db/src/utils.rs index 0227e4db8bcd0..4b0bb4e023dcd 100644 --- a/client/db/src/utils.rs +++ b/client/db/src/utils.rs @@ -197,7 +197,9 @@ fn open_database_at( create: bool, ) -> OpenDbResult { let db: Arc> = match &db_source { + #[cfg(feature = "with-parity-db")] DatabaseSource::ParityDb { path } => open_parity_db::(path, db_type, create)?, + #[cfg(feature = "with-kvdb-rocksdb")] DatabaseSource::RocksDb { path, cache_size } => open_kvdb_rocksdb::(path, db_type, create, *cache_size)?, DatabaseSource::Custom { db, require_create_flag } => { diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index 22bac652c30c4..38771e5760564 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -16,7 +16,9 @@ targets = ["x86_64-unknown-linux-gnu"] default = ["db"] # The RocksDB feature activates the RocksDB database backend. If it is not activated, and you pass # a path to a database, an error will be produced at runtime. -db = ["sc-client-db/with-kvdb-rocksdb", "sc-client-db/with-parity-db"] +with-parity-db = ["sc-client-db/with-parity-db"] +with-rocks-db = ["sc-client-db/with-kvdb-rocksdb"] +db = ["with-parity-db", "with-rocks-db"] wasmtime = ["sc-executor/wasmtime"] # exposes the client type test-helpers = [] diff --git a/test-utils/client/Cargo.toml b/test-utils/client/Cargo.toml index 1ff7d0de1d676..ce5ef2ffcc01a 100644 --- a/test-utils/client/Cargo.toml +++ b/test-utils/client/Cargo.toml @@ -19,7 +19,7 @@ hex = "0.4" serde = "1.0.136" serde_json = "1.0.79" sc-client-api = { version = "4.0.0-dev", path = "../../client/api" } -sc-client-db = { version = "0.10.0-dev", features = [ +sc-client-db = { version = "0.10.0-dev", default-features = false, features = [ "test-helpers", ], path = "../../client/db" } sc-consensus = { version = "0.10.0-dev", path = "../../client/consensus/common" } diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml index 5f30e21b2986a..d4836cc7ef271 100644 --- a/utils/frame/benchmarking-cli/Cargo.toml +++ b/utils/frame/benchmarking-cli/Cargo.toml @@ -39,9 +39,9 @@ frame-benchmarking = { version = "4.0.0-dev", path = "../../../frame/benchmarkin frame-support = { version = "4.0.0-dev", path = "../../../frame/support" } frame-system = { version = "4.0.0-dev", path = "../../../frame/system" } sc-block-builder = { version = "0.10.0-dev", path = "../../../client/block-builder" } -sc-cli = { version = "0.10.0-dev", path = "../../../client/cli" } +sc-cli = { version = "0.10.0-dev", default-features = false, path = "../../../client/cli" } sc-client-api = { version = "4.0.0-dev", path = "../../../client/api" } -sc-client-db = { version = "0.10.0-dev", path = "../../../client/db" } +sc-client-db = { version = "0.10.0-dev", default-features = false, path = "../../../client/db" } sc-executor = { version = "0.10.0-dev", path = "../../../client/executor" } sc-service = { version = "0.10.0-dev", default-features = false, path = "../../../client/service" } sc-sysinfo = { version = "6.0.0-dev", path = "../../../client/sysinfo" } @@ -58,5 +58,8 @@ sp-storage = { version = "6.0.0", path = "../../../primitives/storage" } sp-trie = { version = "6.0.0", path = "../../../primitives/trie" } [features] -default = ["db", "sc-client-db/runtime-benchmarks"] -db = ["sc-client-db/with-kvdb-rocksdb", "sc-client-db/with-parity-db"] +default = ["db", "runtime-benchmarks"] +runtime-benchmarks = ["sc-client-db/runtime-benchmarks"] +with-parity-db = ["sc-cli/with-parity-db", "sc-client-db/with-parity-db"] +with-rocks-db = ["sc-cli/with-rocks-db", "sc-client-db/with-kvdb-rocksdb"] +db = ["with-parity-db", "with-rocks-db"] From 61ad9b8da3bca8ea4a55b3da4e0a4a233f565219 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Mon, 30 May 2022 16:38:41 +0300 Subject: [PATCH 2/3] Make ParityDB non-optional --- bin/node/cli/Cargo.toml | 2 +- bin/node/testing/Cargo.toml | 11 ++++------- client/cli/Cargo.toml | 3 +-- client/cli/src/arg_enums.rs | 7 +------ client/cli/src/config.rs | 4 +--- client/db/Cargo.toml | 3 +-- client/db/src/lib.rs | 8 -------- client/db/src/utils.rs | 25 ------------------------- client/service/Cargo.toml | 4 +--- utils/frame/benchmarking-cli/Cargo.toml | 4 +--- 10 files changed, 11 insertions(+), 60 deletions(-) diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index ab6644a379bb5..9365efe229e1c 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -153,7 +153,7 @@ cli = [ "sc-cli", "frame-benchmarking-cli", "substrate-frame-cli", - "sc-service/db", + "sc-service/with-rocks-db", "clap", "clap_complete", "substrate-build-script-utils", diff --git a/bin/node/testing/Cargo.toml b/bin/node/testing/Cargo.toml index f7a78d10910b3..6f776434e7100 100644 --- a/bin/node/testing/Cargo.toml +++ b/bin/node/testing/Cargo.toml @@ -22,21 +22,18 @@ frame-system = { version = "4.0.0-dev", path = "../../../frame/system" } node-executor = { version = "3.0.0-dev", path = "../executor" } node-primitives = { version = "2.0.0", path = "../primitives" } node-runtime = { version = "3.0.0-dev", path = "../runtime" } -pallet-asset-tx-payment = { version = "4.0.0-dev", path = "../../../frame/transaction-payment/asset-tx-payment/" } +pallet-asset-tx-payment = { version = "4.0.0-dev", path = "../../../frame/transaction-payment/asset-tx-payment" } pallet-transaction-payment = { version = "4.0.0-dev", path = "../../../frame/transaction-payment" } sc-block-builder = { version = "0.10.0-dev", path = "../../../client/block-builder" } -sc-client-api = { version = "4.0.0-dev", path = "../../../client/api/" } -sc-client-db = { version = "0.10.0-dev", features = [ - "kvdb-rocksdb", - "parity-db", -], path = "../../../client/db/" } +sc-client-api = { version = "4.0.0-dev", path = "../../../client/api" } +sc-client-db = { version = "0.10.0-dev", features = ["with-kvdb-rocksdb"], path = "../../../client/db" } sc-consensus = { version = "0.10.0-dev", path = "../../../client/consensus/common" } sc-executor = { version = "0.10.0-dev", features = [ "wasmtime", ], path = "../../../client/executor" } sc-service = { version = "0.10.0-dev", features = [ "test-helpers", - "db", + "with-rocks-db", ], path = "../../../client/service" } sp-api = { version = "4.0.0-dev", path = "../../../primitives/api" } sp-block-builder = { version = "4.0.0-dev", path = "../../../primitives/block-builder" } diff --git a/client/cli/Cargo.toml b/client/cli/Cargo.toml index a5b575bcee321..3b1a555a0a8de 100644 --- a/client/cli/Cargo.toml +++ b/client/cli/Cargo.toml @@ -50,7 +50,6 @@ sp-version = { version = "5.0.0", path = "../../primitives/version" } tempfile = "3.1.0" [features] -default = ["with-parity-db", "with-rocks-db"] -with-parity-db = ["sc-client-db/with-parity-db"] +default = ["with-rocks-db"] with-rocks-db = ["sc-client-db/with-kvdb-rocksdb"] wasmtime = ["sc-service/wasmtime"] diff --git a/client/cli/src/arg_enums.rs b/client/cli/src/arg_enums.rs index cdf7a779fdbce..f9d1fd8de0364 100644 --- a/client/cli/src/arg_enums.rs +++ b/client/cli/src/arg_enums.rs @@ -237,16 +237,14 @@ impl Into for RpcMethods { /// Database backend #[derive(Debug, Clone, PartialEq, Copy)] pub enum Database { - #[cfg(feature = "with-rocks-db")] /// Facebooks RocksDB + #[cfg(feature = "with-rocks-db")] RocksDb, - #[cfg(feature = "with-parity-db")] /// ParityDb. ParityDb, /// Detect whether there is an existing database. Use it, if there is, if not, create new /// instance of ParityDb Auto, - #[cfg(feature = "with-parity-db")] /// ParityDb. ParityDbDeprecated, } @@ -259,7 +257,6 @@ impl std::str::FromStr for Database { if s.eq_ignore_ascii_case("rocksdb") { return Ok(Self::RocksDb) } - #[cfg(feature = "with-parity-db")] if s.eq_ignore_ascii_case("paritydb-experimental") { return Ok(Self::ParityDbDeprecated) } else if s.eq_ignore_ascii_case("paritydb") { @@ -279,9 +276,7 @@ impl Database { &[ #[cfg(feature = "with-rocks-db")] "rocksdb", - #[cfg(feature = "with-parity-db")] "paritydb", - #[cfg(feature = "with-parity-db")] "paritydb-experimental", "auto", ] diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index 7ae369b167d41..86185e3715e3c 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -222,9 +222,7 @@ pub trait CliConfiguration: Sized { Ok(match database { #[cfg(feature = "with-rocks-db")] Database::RocksDb => DatabaseSource::RocksDb { path: rocksdb_path, cache_size }, - #[cfg(feature = "with-parity-db")] Database::ParityDb => DatabaseSource::ParityDb { path: paritydb_path }, - #[cfg(feature = "with-parity-db")] Database::ParityDbDeprecated => { eprintln!( "WARNING: \"paritydb-experimental\" database setting is deprecated and will be removed in future releases. \ @@ -508,7 +506,7 @@ pub trait CliConfiguration: Sized { { Database::RocksDb }, - #[cfg(all(feature = "with-parity-db", not(feature = "with-rocks-db")))] + #[cfg(not(feature = "with-rocks-db"))] { Database::ParityDb }, diff --git a/client/db/Cargo.toml b/client/db/Cargo.toml index e1472bcbda01a..51d73b5630a91 100644 --- a/client/db/Cargo.toml +++ b/client/db/Cargo.toml @@ -22,7 +22,7 @@ kvdb-memorydb = "0.11.0" kvdb-rocksdb = { version = "0.15.2", optional = true } linked-hash-map = "0.5.4" log = "0.4.17" -parity-db = { version = "0.3.13", optional = true } +parity-db = "0.3.13" parking_lot = "0.12.0" sc-client-api = { version = "4.0.0-dev", path = "../api" } sc-state-db = { version = "0.10.0-dev", path = "../state-db" } @@ -46,4 +46,3 @@ default = [] test-helpers = [] runtime-benchmarks = [] with-kvdb-rocksdb = ["kvdb-rocksdb"] -with-parity-db = ["parity-db"] diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index 19e6ca2d5d859..bfec2d4d26322 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -30,11 +30,9 @@ pub mod offchain; -#[cfg(any(feature = "with-parity-db", feature = "with-kvdb-rocksdb", test))] pub mod bench; mod children; -#[cfg(feature = "with-parity-db")] mod parity_db; mod stats; mod storage_cache; @@ -94,7 +92,6 @@ use sp_trie::{prefixed_key, MemoryDB, PrefixedMemoryDB}; pub use sc_state_db::PruningMode; pub use sp_database::Database; -#[cfg(any(feature = "with-parity-db", feature = "with-kvdb-rocksdb", test))] pub use bench::BenchmarkingState; const CACHE_HEADERS: usize = 8; @@ -106,7 +103,6 @@ const DEFAULT_CHILD_RATIO: (usize, usize) = (1, 10); pub type DbState = sp_state_machine::TrieBackend>>, HashFor>; -#[cfg(feature = "with-parity-db")] /// Length of a [`DbHash`]. const DB_HASH_LEN: usize = 32; @@ -336,7 +332,6 @@ pub enum DatabaseSource { }, /// Load a ParityDb database from a given path. - #[cfg(feature = "with-parity-db")] ParityDb { /// Path to the database. path: PathBuf, @@ -363,7 +358,6 @@ impl DatabaseSource { DatabaseSource::Auto { paritydb_path, .. } => Some(paritydb_path), #[cfg(feature = "with-kvdb-rocksdb")] DatabaseSource::RocksDb { path, .. } => Some(path), - #[cfg(feature = "with-parity-db")] DatabaseSource::ParityDb { path } => Some(path), DatabaseSource::Custom { .. } => None, } @@ -381,7 +375,6 @@ impl DatabaseSource { *path = p.into(); true }, - #[cfg(feature = "with-parity-db")] DatabaseSource::ParityDb { ref mut path } => { *path = p.into(); true @@ -397,7 +390,6 @@ impl std::fmt::Display for DatabaseSource { DatabaseSource::Auto { .. } => "Auto", #[cfg(feature = "with-kvdb-rocksdb")] DatabaseSource::RocksDb { .. } => "RocksDb", - #[cfg(feature = "with-parity-db")] DatabaseSource::ParityDb { .. } => "ParityDb", DatabaseSource::Custom { .. } => "Custom", }; diff --git a/client/db/src/utils.rs b/client/db/src/utils.rs index 4b0bb4e023dcd..85f2f5bb85d43 100644 --- a/client/db/src/utils.rs +++ b/client/db/src/utils.rs @@ -34,12 +34,6 @@ use sp_trie::DBValue; /// Number of columns in the db. Must be the same for both full && light dbs. /// Otherwise RocksDb will fail to open database && check its type. -#[cfg(any( - feature = "with-kvdb-rocksdb", - feature = "with-parity-db", - feature = "test-helpers", - test -))] pub const NUM_COLUMNS: u32 = 13; /// Meta column. The set of keys in the column is shared by full && light storages. pub const COLUMN_META: u32 = 0; @@ -197,7 +191,6 @@ fn open_database_at( create: bool, ) -> OpenDbResult { let db: Arc> = match &db_source { - #[cfg(feature = "with-parity-db")] DatabaseSource::ParityDb { path } => open_parity_db::(path, db_type, create)?, #[cfg(feature = "with-kvdb-rocksdb")] DatabaseSource::RocksDb { path, cache_size } => @@ -268,7 +261,6 @@ impl From for sp_blockchain::Error { } } -#[cfg(feature = "with-parity-db")] impl From for OpenDbError { fn from(err: parity_db::Error) -> Self { if matches!(err, parity_db::Error::DatabaseNotFound) { @@ -289,7 +281,6 @@ impl From for OpenDbError { } } -#[cfg(feature = "with-parity-db")] fn open_parity_db(path: &Path, db_type: DatabaseType, create: bool) -> OpenDbResult { match crate::parity_db::open(path, db_type, create, false) { Ok(db) => Ok(db), @@ -302,15 +293,6 @@ fn open_parity_db(path: &Path, db_type: DatabaseType, create: boo } } -#[cfg(not(feature = "with-parity-db"))] -fn open_parity_db( - _path: &Path, - _db_type: DatabaseType, - _create: bool, -) -> OpenDbResult { - Err(OpenDbError::NotEnabled("with-parity-db")) -} - #[cfg(any(feature = "with-kvdb-rocksdb", test))] fn open_kvdb_rocksdb( path: &Path, @@ -641,7 +623,6 @@ mod tests { "db_version", ); - #[cfg(feature = "with-parity-db")] check_dir_for_db_type( DatabaseType::Full, DatabaseSource::ParityDb { path: PathBuf::new() }, @@ -704,8 +685,6 @@ mod tests { assert_eq!(joined.remaining_len().unwrap(), Some(0)); } - #[cfg(feature = "with-parity-db")] - #[cfg(any(feature = "with-kvdb-rocksdb", test))] #[test] fn test_open_database_auto_new() { let db_dir = tempfile::TempDir::new().unwrap(); @@ -751,8 +730,6 @@ mod tests { } } - #[cfg(feature = "with-parity-db")] - #[cfg(any(feature = "with-kvdb-rocksdb", test))] #[test] fn test_open_database_rocksdb_new() { let db_dir = tempfile::TempDir::new().unwrap(); @@ -803,8 +780,6 @@ mod tests { } } - #[cfg(feature = "with-parity-db")] - #[cfg(any(feature = "with-kvdb-rocksdb", test))] #[test] fn test_open_database_paritydb_new() { let db_dir = tempfile::TempDir::new().unwrap(); diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index 38771e5760564..099b9d462c834 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -13,12 +13,10 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [features] -default = ["db"] +default = ["with-rocks-db"] # The RocksDB feature activates the RocksDB database backend. If it is not activated, and you pass # a path to a database, an error will be produced at runtime. -with-parity-db = ["sc-client-db/with-parity-db"] with-rocks-db = ["sc-client-db/with-kvdb-rocksdb"] -db = ["with-parity-db", "with-rocks-db"] wasmtime = ["sc-executor/wasmtime"] # exposes the client type test-helpers = [] diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml index d4836cc7ef271..7b076f910379c 100644 --- a/utils/frame/benchmarking-cli/Cargo.toml +++ b/utils/frame/benchmarking-cli/Cargo.toml @@ -58,8 +58,6 @@ sp-storage = { version = "6.0.0", path = "../../../primitives/storage" } sp-trie = { version = "6.0.0", path = "../../../primitives/trie" } [features] -default = ["db", "runtime-benchmarks"] +default = ["with-rocks-db", "runtime-benchmarks"] runtime-benchmarks = ["sc-client-db/runtime-benchmarks"] -with-parity-db = ["sc-cli/with-parity-db", "sc-client-db/with-parity-db"] with-rocks-db = ["sc-cli/with-rocks-db", "sc-client-db/with-kvdb-rocksdb"] -db = ["with-parity-db", "with-rocks-db"] From e2e63714d9eae0770892d19b40de25bf15152405 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Wed, 1 Jun 2022 05:12:24 +0300 Subject: [PATCH 3/3] Address review comments --- bin/node/cli/Cargo.toml | 2 +- bin/node/testing/Cargo.toml | 4 ++-- client/cli/Cargo.toml | 4 ++-- client/cli/src/arg_enums.rs | 9 ++++----- client/cli/src/config.rs | 6 +++--- client/db/Cargo.toml | 2 +- client/db/src/lib.rs | 10 +++++----- client/db/src/utils.rs | 8 ++++---- client/service/Cargo.toml | 4 ++-- utils/frame/benchmarking-cli/Cargo.toml | 4 ++-- 10 files changed, 26 insertions(+), 27 deletions(-) diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index 9365efe229e1c..8f18aec891e9b 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -153,7 +153,7 @@ cli = [ "sc-cli", "frame-benchmarking-cli", "substrate-frame-cli", - "sc-service/with-rocks-db", + "sc-service/rocksdb", "clap", "clap_complete", "substrate-build-script-utils", diff --git a/bin/node/testing/Cargo.toml b/bin/node/testing/Cargo.toml index 6f776434e7100..7caf10366b48c 100644 --- a/bin/node/testing/Cargo.toml +++ b/bin/node/testing/Cargo.toml @@ -26,14 +26,14 @@ pallet-asset-tx-payment = { version = "4.0.0-dev", path = "../../../frame/transa pallet-transaction-payment = { version = "4.0.0-dev", path = "../../../frame/transaction-payment" } sc-block-builder = { version = "0.10.0-dev", path = "../../../client/block-builder" } sc-client-api = { version = "4.0.0-dev", path = "../../../client/api" } -sc-client-db = { version = "0.10.0-dev", features = ["with-kvdb-rocksdb"], path = "../../../client/db" } +sc-client-db = { version = "0.10.0-dev", features = ["rocksdb"], path = "../../../client/db" } sc-consensus = { version = "0.10.0-dev", path = "../../../client/consensus/common" } sc-executor = { version = "0.10.0-dev", features = [ "wasmtime", ], path = "../../../client/executor" } sc-service = { version = "0.10.0-dev", features = [ "test-helpers", - "with-rocks-db", + "rocksdb", ], path = "../../../client/service" } sp-api = { version = "4.0.0-dev", path = "../../../primitives/api" } sp-block-builder = { version = "4.0.0-dev", path = "../../../primitives/block-builder" } diff --git a/client/cli/Cargo.toml b/client/cli/Cargo.toml index 3b1a555a0a8de..4f0d777d137b9 100644 --- a/client/cli/Cargo.toml +++ b/client/cli/Cargo.toml @@ -50,6 +50,6 @@ sp-version = { version = "5.0.0", path = "../../primitives/version" } tempfile = "3.1.0" [features] -default = ["with-rocks-db"] -with-rocks-db = ["sc-client-db/with-kvdb-rocksdb"] +default = ["rocksdb"] +rocksdb = ["sc-client-db/rocksdb"] wasmtime = ["sc-service/wasmtime"] diff --git a/client/cli/src/arg_enums.rs b/client/cli/src/arg_enums.rs index f9d1fd8de0364..283fef985dfb9 100644 --- a/client/cli/src/arg_enums.rs +++ b/client/cli/src/arg_enums.rs @@ -238,7 +238,7 @@ impl Into for RpcMethods { #[derive(Debug, Clone, PartialEq, Copy)] pub enum Database { /// Facebooks RocksDB - #[cfg(feature = "with-rocks-db")] + #[cfg(feature = "rocksdb")] RocksDb, /// ParityDb. ParityDb, @@ -253,7 +253,7 @@ impl std::str::FromStr for Database { type Err = String; fn from_str(s: &str) -> Result { - #[cfg(feature = "with-rocks-db")] + #[cfg(feature = "rocksdb")] if s.eq_ignore_ascii_case("rocksdb") { return Ok(Self::RocksDb) } @@ -261,8 +261,7 @@ impl std::str::FromStr for Database { return Ok(Self::ParityDbDeprecated) } else if s.eq_ignore_ascii_case("paritydb") { return Ok(Self::ParityDb) - } - if s.eq_ignore_ascii_case("auto") { + } else if s.eq_ignore_ascii_case("auto") { Ok(Self::Auto) } else { Err(format!("Unknown variant `{}`, known variants: {:?}", s, Self::variants())) @@ -274,7 +273,7 @@ impl Database { /// Returns all the variants of this enum to be shown in the cli. pub const fn variants() -> &'static [&'static str] { &[ - #[cfg(feature = "with-rocks-db")] + #[cfg(feature = "rocksdb")] "rocksdb", "paritydb", "paritydb-experimental", diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index 86185e3715e3c..6e1317c11fbc4 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -220,7 +220,7 @@ pub trait CliConfiguration: Sized { let rocksdb_path = base_path.join("db").join(role_dir); let paritydb_path = base_path.join("paritydb").join(role_dir); Ok(match database { - #[cfg(feature = "with-rocks-db")] + #[cfg(feature = "rocksdb")] Database::RocksDb => DatabaseSource::RocksDb { path: rocksdb_path, cache_size }, Database::ParityDb => DatabaseSource::ParityDb { path: paritydb_path }, Database::ParityDbDeprecated => { @@ -502,11 +502,11 @@ pub trait CliConfiguration: Sized { let client_id = C::client_id(); let database_cache_size = self.database_cache_size()?.unwrap_or(1024); let database = self.database()?.unwrap_or( - #[cfg(feature = "with-rocks-db")] + #[cfg(feature = "rocksdb")] { Database::RocksDb }, - #[cfg(not(feature = "with-rocks-db"))] + #[cfg(not(feature = "rocksdb"))] { Database::ParityDb }, diff --git a/client/db/Cargo.toml b/client/db/Cargo.toml index 51d73b5630a91..3b6402b3f6023 100644 --- a/client/db/Cargo.toml +++ b/client/db/Cargo.toml @@ -45,4 +45,4 @@ substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/ru default = [] test-helpers = [] runtime-benchmarks = [] -with-kvdb-rocksdb = ["kvdb-rocksdb"] +rocksdb = ["kvdb-rocksdb"] diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index bfec2d4d26322..644b1ebdb777a 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -36,7 +36,7 @@ mod children; mod parity_db; mod stats; mod storage_cache; -#[cfg(any(feature = "with-kvdb-rocksdb", test))] +#[cfg(any(feature = "rocksdb", test))] mod upgrade; mod utils; @@ -323,7 +323,7 @@ pub enum DatabaseSource { cache_size: usize, }, /// Load a RocksDB database from a given path. Recommended for most uses. - #[cfg(feature = "with-kvdb-rocksdb")] + #[cfg(feature = "rocksdb")] RocksDb { /// Path to the database. path: PathBuf, @@ -356,7 +356,7 @@ impl DatabaseSource { // IIUC this is needed for polkadot to create its own dbs, so until it can use parity db // I would think rocksdb, but later parity-db. DatabaseSource::Auto { paritydb_path, .. } => Some(paritydb_path), - #[cfg(feature = "with-kvdb-rocksdb")] + #[cfg(feature = "rocksdb")] DatabaseSource::RocksDb { path, .. } => Some(path), DatabaseSource::ParityDb { path } => Some(path), DatabaseSource::Custom { .. } => None, @@ -370,7 +370,7 @@ impl DatabaseSource { *paritydb_path = p.into(); true }, - #[cfg(feature = "with-kvdb-rocksdb")] + #[cfg(feature = "rocksdb")] DatabaseSource::RocksDb { ref mut path, .. } => { *path = p.into(); true @@ -388,7 +388,7 @@ impl std::fmt::Display for DatabaseSource { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let name = match self { DatabaseSource::Auto { .. } => "Auto", - #[cfg(feature = "with-kvdb-rocksdb")] + #[cfg(feature = "rocksdb")] DatabaseSource::RocksDb { .. } => "RocksDb", DatabaseSource::ParityDb { .. } => "ParityDb", DatabaseSource::Custom { .. } => "Custom", diff --git a/client/db/src/utils.rs b/client/db/src/utils.rs index 85f2f5bb85d43..567950d089e1b 100644 --- a/client/db/src/utils.rs +++ b/client/db/src/utils.rs @@ -192,7 +192,7 @@ fn open_database_at( ) -> OpenDbResult { let db: Arc> = match &db_source { DatabaseSource::ParityDb { path } => open_parity_db::(path, db_type, create)?, - #[cfg(feature = "with-kvdb-rocksdb")] + #[cfg(feature = "rocksdb")] DatabaseSource::RocksDb { path, cache_size } => open_kvdb_rocksdb::(path, db_type, create, *cache_size)?, DatabaseSource::Custom { db, require_create_flag } => { @@ -293,7 +293,7 @@ fn open_parity_db(path: &Path, db_type: DatabaseType, create: boo } } -#[cfg(any(feature = "with-kvdb-rocksdb", test))] +#[cfg(any(feature = "rocksdb", test))] fn open_kvdb_rocksdb( path: &Path, db_type: DatabaseType, @@ -343,7 +343,7 @@ fn open_kvdb_rocksdb( Ok(sp_database::as_database(db)) } -#[cfg(not(any(feature = "with-kvdb-rocksdb", test)))] +#[cfg(not(any(feature = "rocksdb", test)))] fn open_kvdb_rocksdb( _path: &Path, _db_type: DatabaseType, @@ -586,7 +586,7 @@ mod tests { use std::path::PathBuf; type Block = RawBlock>; - #[cfg(any(feature = "with-kvdb-rocksdb", test))] + #[cfg(any(feature = "rocksdb", test))] #[test] fn database_type_subdir_migration() { type Block = RawBlock>; diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index 099b9d462c834..c1cb9e459041b 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -13,10 +13,10 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [features] -default = ["with-rocks-db"] +default = ["rocksdb"] # The RocksDB feature activates the RocksDB database backend. If it is not activated, and you pass # a path to a database, an error will be produced at runtime. -with-rocks-db = ["sc-client-db/with-kvdb-rocksdb"] +rocksdb = ["sc-client-db/rocksdb"] wasmtime = ["sc-executor/wasmtime"] # exposes the client type test-helpers = [] diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml index 7b076f910379c..6216033dbbc5d 100644 --- a/utils/frame/benchmarking-cli/Cargo.toml +++ b/utils/frame/benchmarking-cli/Cargo.toml @@ -58,6 +58,6 @@ sp-storage = { version = "6.0.0", path = "../../../primitives/storage" } sp-trie = { version = "6.0.0", path = "../../../primitives/trie" } [features] -default = ["with-rocks-db", "runtime-benchmarks"] +default = ["rocksdb", "runtime-benchmarks"] runtime-benchmarks = ["sc-client-db/runtime-benchmarks"] -with-rocks-db = ["sc-cli/with-rocks-db", "sc-client-db/with-kvdb-rocksdb"] +rocksdb = ["sc-cli/rocksdb", "sc-client-db/rocksdb"]