store: add StoreConfig::migration_snapshot option; deprecated options in Config

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<std::path::PathBuf> {
+        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, crate::SnapshotError> {
-        crate::Snapshot::new(&self.path, self.config, snapshot_path)
+    pub fn new_migration_snapshot(&self) -> Result<crate::Snapshot, crate::SnapshotError> {
+        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()),
+        }
     }
 }

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

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;

nearcore/src/config.rs

@@ -199,10 +199,6 @@ fn default_trie_viewer_state_size_limit() -> Option<u64> {
     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<Gas>,
-    /// 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<PathBuf>,
     /// 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<bool>,
+    /// Deprecated; use `store.migration_snapshot` instead.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub db_migration_snapshot_path: Option<PathBuf>,
 }
 
 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(),
         }
     }

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<near_store::Snapshot> {
-    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 {