Skip to content

Commit

Permalink
Make it possible to disable RocksDB completely (paritytech#11537)
Browse files Browse the repository at this point in the history
* Make it possible to disable RocksDB completely

* Make ParityDB non-optional

* Address review comments
  • Loading branch information
nazar-pc authored and ark0f committed Feb 27, 2023
1 parent e89fafb commit d175d99
Show file tree
Hide file tree
Showing 11 changed files with 59 additions and 60 deletions.
2 changes: 1 addition & 1 deletion bin/node/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ cli = [
"sc-cli",
"frame-benchmarking-cli",
"substrate-frame-cli",
"sc-service/db",
"sc-service/rocksdb",
"clap",
"clap_complete",
"substrate-build-script-utils",
Expand Down
11 changes: 4 additions & 7 deletions bin/node/testing/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ["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",
"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" }
Expand Down
4 changes: 3 additions & 1 deletion client/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand All @@ -50,4 +50,6 @@ sp-version = { version = "5.0.0", path = "../../primitives/version" }
tempfile = "3.1.0"

[features]
default = ["rocksdb"]
rocksdb = ["sc-client-db/rocksdb"]
wasmtime = ["sc-service/wasmtime"]
21 changes: 15 additions & 6 deletions client/cli/src/arg_enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ impl Into<sc_service::config::RpcMethods> for RpcMethods {
#[derive(Debug, Clone, PartialEq, Copy)]
pub enum Database {
/// Facebooks RocksDB
#[cfg(feature = "rocksdb")]
RocksDb,
/// ParityDb. <https://github.com/paritytech/parity-db/>
ParityDb,
Expand All @@ -252,12 +253,14 @@ impl std::str::FromStr for Database {
type Err = String;

fn from_str(s: &str) -> Result<Self, String> {
#[cfg(feature = "rocksdb")]
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)
}
if s.eq_ignore_ascii_case("paritydb-experimental") {
return Ok(Self::ParityDbDeprecated)
} else if s.eq_ignore_ascii_case("paritydb") {
Ok(Self::ParityDb)
return Ok(Self::ParityDb)
} else if s.eq_ignore_ascii_case("auto") {
Ok(Self::Auto)
} else {
Expand All @@ -268,8 +271,14 @@ 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 = "rocksdb")]
"rocksdb",
"paritydb",
"paritydb-experimental",
"auto",
]
}
}

Expand Down
12 changes: 11 additions & 1 deletion client/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: 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 = "rocksdb")]
Database::RocksDb => DatabaseSource::RocksDb { path: rocksdb_path, cache_size },
Database::ParityDb => DatabaseSource::ParityDb { path: paritydb_path },
Database::ParityDbDeprecated => {
Expand Down Expand Up @@ -500,7 +501,16 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: 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 = "rocksdb")]
{
Database::RocksDb
},
#[cfg(not(feature = "rocksdb"))]
{
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);
Expand Down
5 changes: 2 additions & 3 deletions client/db/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand All @@ -45,5 +45,4 @@ substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/ru
default = []
test-helpers = []
runtime-benchmarks = []
with-kvdb-rocksdb = ["kvdb-rocksdb"]
with-parity-db = ["parity-db"]
rocksdb = ["kvdb-rocksdb"]
18 changes: 11 additions & 7 deletions client/db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,13 @@

pub mod offchain;

#[cfg(any(feature = "with-kvdb-rocksdb", test))]
pub mod bench;

mod children;
#[cfg(feature = "with-parity-db")]
mod parity_db;
mod stats;
mod storage_cache;
#[cfg(any(feature = "with-kvdb-rocksdb", test))]
#[cfg(any(feature = "rocksdb", test))]
mod upgrade;
mod utils;

Expand Down Expand Up @@ -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-kvdb-rocksdb", test))]
pub use bench::BenchmarkingState;

const CACHE_HEADERS: usize = 8;
Expand All @@ -106,7 +103,6 @@ const DEFAULT_CHILD_RATIO: (usize, usize) = (1, 10);
pub type DbState<B> =
sp_state_machine::TrieBackend<Arc<dyn sp_state_machine::Storage<HashFor<B>>>, HashFor<B>>;

#[cfg(feature = "with-parity-db")]
/// Length of a [`DbHash`].
const DB_HASH_LEN: usize = 32;

Expand Down Expand Up @@ -330,6 +326,7 @@ pub enum DatabaseSource {
cache_size: usize,
},
/// Load a RocksDB database from a given path. Recommended for most uses.
#[cfg(feature = "rocksdb")]
RocksDb {
/// Path to the database.
path: PathBuf,
Expand Down Expand Up @@ -362,7 +359,9 @@ 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 = "rocksdb")]
DatabaseSource::RocksDb { path, .. } => Some(path),
DatabaseSource::ParityDb { path } => Some(path),
DatabaseSource::Custom { .. } => None,
}
}
Expand All @@ -374,7 +373,11 @@ impl DatabaseSource {
*paritydb_path = p.into();
true
},
DatabaseSource::RocksDb { ref mut path, .. } |
#[cfg(feature = "rocksdb")]
DatabaseSource::RocksDb { ref mut path, .. } => {
*path = p.into();
true
},
DatabaseSource::ParityDb { ref mut path } => {
*path = p.into();
true
Expand All @@ -388,6 +391,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 = "rocksdb")]
DatabaseSource::RocksDb { .. } => "RocksDb",
DatabaseSource::ParityDb { .. } => "ParityDb",
DatabaseSource::Custom { .. } => "Custom",
Expand Down
31 changes: 4 additions & 27 deletions client/db/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -198,6 +192,7 @@ fn open_database_at<Block: BlockT>(
) -> OpenDbResult {
let db: Arc<dyn Database<DbHash>> = match &db_source {
DatabaseSource::ParityDb { path } => open_parity_db::<Block>(path, db_type, create)?,
#[cfg(feature = "rocksdb")]
DatabaseSource::RocksDb { path, cache_size } =>
open_kvdb_rocksdb::<Block>(path, db_type, create, *cache_size)?,
DatabaseSource::Custom { db, require_create_flag } => {
Expand Down Expand Up @@ -266,7 +261,6 @@ impl From<OpenDbError> for sp_blockchain::Error {
}
}

#[cfg(feature = "with-parity-db")]
impl From<parity_db::Error> for OpenDbError {
fn from(err: parity_db::Error) -> Self {
if matches!(err, parity_db::Error::DatabaseNotFound) {
Expand All @@ -287,7 +281,6 @@ impl From<io::Error> for OpenDbError {
}
}

#[cfg(feature = "with-parity-db")]
fn open_parity_db<Block: BlockT>(path: &Path, db_type: DatabaseType, create: bool) -> OpenDbResult {
match crate::parity_db::open(path, db_type, create, false) {
Ok(db) => Ok(db),
Expand All @@ -300,16 +293,7 @@ fn open_parity_db<Block: BlockT>(path: &Path, db_type: DatabaseType, create: boo
}
}

#[cfg(not(feature = "with-parity-db"))]
fn open_parity_db<Block: BlockT>(
_path: &Path,
_db_type: DatabaseType,
_create: bool,
) -> OpenDbResult {
Err(OpenDbError::NotEnabled("with-parity-db"))
}

#[cfg(any(feature = "with-kvdb-rocksdb", test))]
#[cfg(any(feature = "rocksdb", test))]
fn open_kvdb_rocksdb<Block: BlockT>(
path: &Path,
db_type: DatabaseType,
Expand Down Expand Up @@ -359,7 +343,7 @@ fn open_kvdb_rocksdb<Block: BlockT>(
Ok(sp_database::as_database(db))
}

#[cfg(not(any(feature = "with-kvdb-rocksdb", test)))]
#[cfg(not(any(feature = "rocksdb", test)))]
fn open_kvdb_rocksdb<Block: BlockT>(
_path: &Path,
_db_type: DatabaseType,
Expand Down Expand Up @@ -602,7 +586,7 @@ mod tests {
use std::path::PathBuf;
type Block = RawBlock<ExtrinsicWrapper<u32>>;

#[cfg(any(feature = "with-kvdb-rocksdb", test))]
#[cfg(any(feature = "rocksdb", test))]
#[test]
fn database_type_subdir_migration() {
type Block = RawBlock<ExtrinsicWrapper<u64>>;
Expand Down Expand Up @@ -639,7 +623,6 @@ mod tests {
"db_version",
);

#[cfg(feature = "with-parity-db")]
check_dir_for_db_type(
DatabaseType::Full,
DatabaseSource::ParityDb { path: PathBuf::new() },
Expand Down Expand Up @@ -702,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();
Expand Down Expand Up @@ -749,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();
Expand Down Expand Up @@ -801,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();
Expand Down
4 changes: 2 additions & 2 deletions client/service/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ readme = "README.md"
targets = ["x86_64-unknown-linux-gnu"]

[features]
default = ["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.
db = ["sc-client-db/with-kvdb-rocksdb", "sc-client-db/with-parity-db"]
rocksdb = ["sc-client-db/rocksdb"]
wasmtime = ["sc-executor/wasmtime"]
# exposes the client type
test-helpers = []
Expand Down
2 changes: 1 addition & 1 deletion test-utils/client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
9 changes: 5 additions & 4 deletions utils/frame/benchmarking-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand All @@ -59,5 +59,6 @@ sp-trie = { version = "6.0.0", path = "../../../primitives/trie" }
gethostname = "0.2.3"

[features]
default = ["db", "sc-client-db/runtime-benchmarks"]
db = ["sc-client-db/with-kvdb-rocksdb", "sc-client-db/with-parity-db"]
default = ["rocksdb", "runtime-benchmarks"]
runtime-benchmarks = ["sc-client-db/runtime-benchmarks"]
rocksdb = ["sc-cli/rocksdb", "sc-client-db/rocksdb"]

0 comments on commit d175d99

Please sign in to comment.