From 6b9aff928db1e30676d69b904035aff159acb5ac Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Mon, 29 Aug 2022 23:29:07 +0200 Subject: [PATCH] store: add StoreConfig::migration_snapshot option; deprecated options in Config (#7486) Deprecated use_db_migration_snapshot and db_migration_snapshot_path fields in Config by migration_snapshot field in StoreConfig. The main motivation for doing it is being able to configure the path separately for hot and cold storage (once cold storage is implemented). --- core/store/src/config.rs | 79 +++++++++++++++++++++++++-- core/store/src/db/rocksdb/snapshot.rs | 26 ++++----- core/store/src/lib.rs | 2 +- nearcore/src/config.rs | 27 +++++---- nearcore/src/lib.rs | 60 ++++++++++---------- 5 files changed, 131 insertions(+), 63 deletions(-) diff --git a/core/store/src/config.rs b/core/store/src/config.rs index 707bf3cd241..98844301682 100644 --- a/core/store/src/config.rs +++ b/core/store/src/config.rs @@ -47,6 +47,37 @@ pub struct StoreConfig { /// We're still experimenting with this parameter and it seems decreasing its value can improve /// the performance of the storage pub trie_cache_capacities: Vec<(ShardUId, usize)>, + + /// Path where to create RocksDB checkpoints during database migrations or + /// `false` to disable that feature. + /// + /// If this feature is enabled, when database migration happens a RocksDB + /// checkpoint will be created just before the migration starts. This way, + /// if there are any failures during migration, the database can be + /// recovered from the checkpoint. + /// + /// The field can be one of: + /// * an absolute path name → the snapshot will be created in specified + /// directory. No sub-directories will be created so for example you + /// probably don’t want `/tmp` but rather `/tmp/neard-db-snapshot`; + /// * an relative path name → the snapshot will be created in a directory + /// inside of the RocksDB database directory (see `path` field); + /// * `true` (the default) → this is equivalent to setting the field to + /// `migration-snapshot`; and + /// * `false` → the snapshot will not be created. + /// + /// Note that if the snapshot is on a different file system than the + /// database, creating the snapshot may itself take time as data may need to + /// be copied between the databases. + #[serde(skip_serializing_if = "MigrationSnapshot::is_default")] + pub migration_snapshot: MigrationSnapshot, +} + +#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] +#[serde(untagged)] +pub enum MigrationSnapshot { + Enabled(bool), + Path(std::path::PathBuf), } /// Mode in which to open the storage. @@ -109,10 +140,48 @@ impl Default for StoreConfig { block_size: bytesize::ByteSize::kib(16), trie_cache_capacities: vec![(ShardUId { version: 1, shard_id: 3 }, 45_000_000)], + + migration_snapshot: Default::default(), } } } +impl MigrationSnapshot { + /// Returns path to the snapshot given path to the database. + /// + /// Returns `None` if migration snapshot is disabled. Relative paths are + /// resolved relative to `db_path`. + pub fn get_path<'a>(&'a self, db_path: &std::path::Path) -> Option { + let path = match &self { + Self::Enabled(false) => return None, + Self::Enabled(true) => std::path::Path::new("migration-snapshot"), + Self::Path(path) => path.as_path(), + }; + Some(db_path.join(path)) + } + + /// Checks whether the object equals its default value. + fn is_default(&self) -> bool { + matches!(self, Self::Enabled(true)) + } + + /// Formats an example of how to edit `config.json` to set migration path to + /// given value. + pub fn format_example(&self) -> String { + let value = serde_json::to_string(self).unwrap(); + format!( + " {{\n \"store\": {{\n \"migration_snapshot\": \ + {value}\n }}\n }}" + ) + } +} + +impl Default for MigrationSnapshot { + fn default() -> Self { + Self::Enabled(true) + } +} + /// Builder for opening a RocksDB database. /// /// Typical usage: @@ -205,10 +274,10 @@ impl<'a> StoreOpener<'a> { /// Note that due to RocksDB being weird, this will create an empty database /// if it does not already exist. This might not be what you want so make /// sure the database already exists. - pub fn new_migration_snapshot( - &self, - snapshot_path: std::path::PathBuf, - ) -> Result { - crate::Snapshot::new(&self.path, self.config, snapshot_path) + pub fn new_migration_snapshot(&self) -> Result { + match self.config.migration_snapshot.get_path(&self.path) { + Some(path) => crate::Snapshot::new(&self.path, self.config, path), + None => Ok(crate::Snapshot::no_snapshot()), + } } } diff --git a/core/store/src/db/rocksdb/snapshot.rs b/core/store/src/db/rocksdb/snapshot.rs index 890372e412f..373550f5488 100644 --- a/core/store/src/db/rocksdb/snapshot.rs +++ b/core/store/src/db/rocksdb/snapshot.rs @@ -89,6 +89,11 @@ impl Snapshot { } } } + + /// Returns path to the snapshot if it exists. + pub fn path(&self) -> Option<&std::path::Path> { + self.0.as_deref() + } } impl std::ops::Drop for Snapshot { @@ -114,32 +119,25 @@ impl std::ops::Drop for Snapshot { fn test_snapshot_creation() { use assert_matches::assert_matches; - let (tmpdir, opener) = crate::Store::test_opener(); - let path = tmpdir.path().join("cp"); + let (_tmpdir, opener) = crate::Store::test_opener(); // Create the database core::mem::drop(opener.open()); // Creating snapshot should work now. - let snapshot = opener.new_migration_snapshot(path.clone()).unwrap(); + let snapshot = opener.new_migration_snapshot().unwrap(); // Snapshot already exists so cannot create a new one. - assert_matches!( - opener.new_migration_snapshot(path.clone()), - Err(SnapshotError::AlreadyExists(_)) - ); + assert_matches!(opener.new_migration_snapshot(), Err(SnapshotError::AlreadyExists(_))); snapshot.remove(); // This should work correctly again since the snapshot has been removed. - opener.new_migration_snapshot(path.clone()).unwrap(); + opener.new_migration_snapshot().unwrap(); // And this again should fail. We don’t remove the snapshot in // Snapshot::drop. - assert_matches!( - opener.new_migration_snapshot(path.clone()), - Err(SnapshotError::AlreadyExists(_)) - ); + assert_matches!(opener.new_migration_snapshot(), Err(SnapshotError::AlreadyExists(_))); } /// Tests that reading data from a snapshot is possible. @@ -149,7 +147,7 @@ fn test_snapshot_recovery() { const COL: crate::DBCol = crate::DBCol::BlockMisc; let (tmpdir, opener) = crate::Store::test_opener(); - let path = tmpdir.path().join("cp"); + let path = opener.config().migration_snapshot.get_path(opener.path()).unwrap(); // Populate some data { @@ -160,7 +158,7 @@ fn test_snapshot_recovery() { } // Create snapshot - let snapshot = opener.new_migration_snapshot(path.clone()).unwrap(); + let snapshot = opener.new_migration_snapshot().unwrap(); // Delete the data from the database. { diff --git a/core/store/src/lib.rs b/core/store/src/lib.rs index 62de18b0ea7..da067177aac 100644 --- a/core/store/src/lib.rs +++ b/core/store/src/lib.rs @@ -37,7 +37,7 @@ pub use crate::trie::{ }; mod columns; -mod config; +pub mod config; pub mod db; pub mod flat_state; mod metrics; diff --git a/nearcore/src/config.rs b/nearcore/src/config.rs index e6679cb9868..0a38eddea59 100644 --- a/nearcore/src/config.rs +++ b/nearcore/src/config.rs @@ -199,10 +199,6 @@ fn default_trie_viewer_state_size_limit() -> Option { Some(50_000) } -fn default_use_checkpoints_for_db_migration() -> bool { - true -} - #[derive(Serialize, Deserialize, Clone, Debug)] pub struct Consensus { /// Minimum number of peers to start syncing. @@ -317,17 +313,20 @@ pub struct Config { /// If set, overrides value in genesis configuration. #[serde(skip_serializing_if = "Option::is_none")] pub max_gas_burnt_view: Option, - /// Checkpoints let the user recover from interrupted DB migrations. - #[serde(default = "default_use_checkpoints_for_db_migration")] - pub use_db_migration_snapshot: bool, - /// Location of the DB checkpoint for the DB migrations. This can be one of the following: - /// * Empty, the checkpoint will be created in the database location, i.e. '$home/data'. - /// * Absolute path that points to an existing directory. The checkpoint will be a sub-directory in that directory. - /// For example, setting "use_db_migration_snapshot" to "/tmp/" will create a directory "/tmp/db_migration_snapshot" and populate it with the database files. - #[serde(skip_serializing_if = "Option::is_none")] - pub db_migration_snapshot_path: Option, /// Different parameters to configure/optimize underlying storage. pub store: near_store::StoreConfig, + + // TODO(mina86): Remove those two altogether at some point. We need to be + // somewhat careful though and make sure that we don’t start silently + // ignoring this option without users setting corresponding store option. + // For the time being, we’re failing inside of create_db_checkpoint if this + // option is set. + /// Deprecated; use `store.migration_snapshot` instead. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub use_db_migration_snapshot: Option, + /// Deprecated; use `store.migration_snapshot` instead. + #[serde(skip_serializing_if = "Option::is_none")] + pub db_migration_snapshot_path: Option, } impl Default for Config { @@ -355,7 +354,7 @@ impl Default for Config { trie_viewer_state_size_limit: default_trie_viewer_state_size_limit(), max_gas_burnt_view: None, db_migration_snapshot_path: None, - use_db_migration_snapshot: true, + use_db_migration_snapshot: None, store: near_store::StoreConfig::default(), } } diff --git a/nearcore/src/lib.rs b/nearcore/src/lib.rs index 11dd1aa40e7..afab63e95de 100644 --- a/nearcore/src/lib.rs +++ b/nearcore/src/lib.rs @@ -44,31 +44,33 @@ pub fn get_default_home() -> PathBuf { PathBuf::default() } -/// Returns the path of the DB checkpoint. -/// Default location is the same as the database location: `path`. -fn db_checkpoint_path(path: &Path, near_config: &NearConfig) -> PathBuf { - let root_path = - if let Some(db_migration_snapshot_path) = &near_config.config.db_migration_snapshot_path { - assert!( - db_migration_snapshot_path.is_absolute(), - "'db_migration_snapshot_path' must be an absolute path to an existing directory." - ); - db_migration_snapshot_path.clone() - } else { - path.to_path_buf() - }; - root_path.join(DB_CHECKPOINT_NAME) -} - -const DB_CHECKPOINT_NAME: &str = "db_migration_snapshot"; - /// Creates a consistent DB checkpoint and returns its path. /// By default it creates checkpoints in the DB directory, but can be overridden by the config. fn create_db_checkpoint( opener: &StoreOpener, near_config: &NearConfig, ) -> anyhow::Result { - match opener.new_migration_snapshot(db_checkpoint_path(opener.path(), near_config)) { + use near_store::config::MigrationSnapshot; + + let example = match ( + near_config.config.use_db_migration_snapshot, + near_config.config.db_migration_snapshot_path.as_ref(), + ) { + (None, None) => None, + (Some(false), _) => Some(MigrationSnapshot::Enabled(false)), + (_, None) => Some(MigrationSnapshot::Enabled(true)), + (_, Some(path)) => Some(MigrationSnapshot::Path(path.join("migration-snapshot"))), + }; + if let Some(example) = example { + anyhow::bail!( + "‘use_db_migration_snapshot’ and ‘db_migration_snapshot_path’ \ + options are deprecated.\n\ + Set ‘store.migration_snapshot’ to instead, e.g.:\n{}", + example.format_example() + ) + } + + match opener.new_migration_snapshot() { Ok(snapshot) => Ok(snapshot), Err(near_store::SnapshotError::AlreadyExists(snap_path)) => { Err(anyhow::anyhow!( @@ -80,13 +82,17 @@ fn create_db_checkpoint( )) } Err(near_store::SnapshotError::IOError(err)) => { - Err(anyhow::anyhow!( + let path = std::path::PathBuf::from("/path/to/snapshot/dir"); + let on = MigrationSnapshot::Path(path).format_example(); + let off = MigrationSnapshot::Enabled(false).format_example(); + anyhow::bail!( "Failed to create a database migration snapshot: {err}.\n\ - You can change the location of the snapshot by adjusting `config.json`:\n\ - \t\"db_migration_snapshot_path\": \"/absolute/path/to/existing/dir\",\n\ + To change the location of snapshot adjust \ + ‘store.migration_snapshot’ property in ‘config.json’:\n\ + {on}\n\ Alternatively, you can disable database migration snapshots in `config.json`:\n\ - \t\"use_db_migration_snapshot\": false," - )) + {off}" + ) } } } @@ -132,11 +138,7 @@ fn apply_store_migrations_if_exists( // Before starting a DB migration, create a snapshot of the database. If // the migration fails, the snapshot can be used to restore the database to // its original state. - let snapshot = if near_config.config.use_db_migration_snapshot { - create_db_checkpoint(store_opener, near_config)? - } else { - near_store::Snapshot::no_snapshot() - }; + let snapshot = create_db_checkpoint(store_opener, near_config)?; // Add migrations here based on `db_version`. if db_version <= 26 {