nearcore: use StoreOpener in recompress_storage

This allows removal of get_store_version (since the code is now using
StoreOpener::get_version_if_exists is used) and reduces uses of the
StoreOpener::path method.

Issue: near#6857

core/store/src/config.rs

@@ -1,8 +1,6 @@
 use near_primitives::version::DbVersion;
 
-// TODO(mina86): This is pub only because recompress-storage needs this value.
-// Refactor code so that this can be private.
-pub const STORE_PATH: &str = "data";
+const STORE_PATH: &str = "data";
 
 #[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
 pub struct StoreConfig {
@@ -102,15 +100,8 @@ impl Default for StoreConfig {
     }
 }
 
-// TODO(#6857): Make this method in StoreOpener.  This way caller won’t need to
-// resolve path to the storage.
-pub fn store_path_exists<P: AsRef<std::path::Path>>(path: P) -> bool {
-    std::fs::canonicalize(path).is_ok()
-}
-
-// TODO(#6857): Get rid of this function.  Clients of this method should not
-// care about path of the storage instead letting StoreOpener figure that one
-// out.
+// TODO(#6857): Get rid of this function.  Clients of this method should use
+// StoreOpener::get_path instead..
 pub fn get_store_path(base_path: &std::path::Path) -> std::path::PathBuf {
     base_path.join(STORE_PATH)
 }
@@ -194,6 +185,17 @@ impl<'a> StoreOpener<'a> {
         self
     }
 
+    /// Returns whether database exists.
+    ///
+    /// It performs only basic file-system-level checks and may result in false
+    /// positives if some but not all database files exist.  In particular, this
+    /// is not a guarantee that the database can be opened without an error.
+    pub fn check_if_exists(&self) -> bool {
+        // TODO(mina86): Add some more checks.  At least check if CURRENT file
+        // exists.
+        std::fs::canonicalize(&self.get_path()).is_ok()
+    }
+
     /// Returns path to the underlying RocksDB database.
     ///
     /// Does not check whether the database actually exists.  It merely

core/store/src/lib.rs

@@ -46,7 +46,7 @@ pub mod migrations;
 pub mod test_utils;
 mod trie;
 
-pub use crate::config::{get_store_path, store_path_exists, StoreConfig, StoreOpener, STORE_PATH};
+pub use crate::config::{get_store_path, StoreConfig, StoreOpener};
 
 #[derive(Clone)]
 pub struct Store {

core/store/src/migrations.rs

@@ -6,17 +6,8 @@ use near_primitives::epoch_manager::epoch_info::{EpochInfo, EpochInfoV1};
 use near_primitives::hash::CryptoHash;
 use near_primitives::types::validator_stake::ValidatorStake;
 use near_primitives::types::AccountId;
-use near_primitives::version::DbVersion;
 
-use crate::db::{DBError, RocksDB};
 use crate::{DBCol, Store, StoreOpener, StoreUpdate};
-use std::path::Path;
-
-// This is used by recompress_storage only.  TODO(#6857): Get rid of this
-// function in favour of StoreOpener::get_version_if_exists.
-pub fn get_store_version(path: &Path) -> Result<DbVersion, DBError> {
-    RocksDB::get_version(path)
-}
 
 fn set_store_version_inner(store_update: &mut StoreUpdate, db_version: u32) {
     store_update.set(
@@ -149,7 +140,6 @@ pub fn migrate_29_to_30(store_opener: &StoreOpener) {
     };
     use std::collections::BTreeMap;
 
-    // TODO(#6857): Don’t use .path().
     let store = store_opener.open();
 
     #[derive(BorshDeserialize)]

nearcore/src/lib.rs

@@ -18,10 +18,8 @@ use near_rosetta_rpc::start_rosetta_rpc;
 #[cfg(feature = "performance_stats")]
 use near_rust_allocator_proxy::reset_memory_usage_max;
 use near_store::db::RocksDB;
-use near_store::migrations::{
-    get_store_version, migrate_28_to_29, migrate_29_to_30, set_store_version,
-};
-use near_store::{DBCol, Store, StoreOpener};
+use near_store::migrations::{migrate_28_to_29, migrate_29_to_30, set_store_version};
+use near_store::{DBCol, Store};
 use near_telemetry::TelemetryActor;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
@@ -205,7 +203,7 @@ fn apply_store_migrations_if_exists(
 }
 
 fn init_and_migrate_store(home_dir: &Path, near_config: &NearConfig) -> anyhow::Result<Store> {
-    let opener = StoreOpener::new(&near_config.config.store).home(home_dir);
+    let opener = near_store::StoreOpener::new(&near_config.config.store).home(home_dir);
     let exists = apply_store_migrations_if_exists(&opener, near_config)?;
     let store = opener.open();
     if !exists {
@@ -389,47 +387,49 @@ pub fn recompress_storage(home_dir: &Path, opts: RecompressOpts) -> anyhow::Resu
             .map_err(|err| anyhow::anyhow!("setrlimit: NOFILE: {}", err))?;
     }
 
-    let src_dir = home_dir.join(near_store::STORE_PATH);
-    anyhow::ensure!(
-        near_store::store_path_exists(&src_dir),
-        "{}: source storage doesn’t exist",
-        src_dir.display()
-    );
-    let db_version = get_store_version(&src_dir)?;
-    anyhow::ensure!(
-        db_version == near_primitives::version::DB_VERSION,
-        "{}: expected DB version {} but got {}",
-        src_dir.display(),
-        near_primitives::version::DB_VERSION,
-        db_version
-    );
+    let src_opener = near_store::StoreOpener::new(&config.store).home(home_dir);
+    if let Some(db_version) = src_opener.get_version_if_exists()? {
+        anyhow::ensure!(
+            db_version == near_primitives::version::DB_VERSION,
+            "{}: expected DB version {} but got {}",
+            src_opener.get_path().display(),
+            near_primitives::version::DB_VERSION,
+            db_version
+        );
+    } else {
+        anyhow::bail!("{}: source storage doesn’t exist", src_opener.get_path().display());
+    }
 
+    // Note: opts.dest_dir is resolved relative to current working directory
+    // (since it’s a command line option) which is why we set home to cwd and
+    // path to dest_dir.
+    let cwd = std::env::current_dir()?;
+    let dst_opener = near_store::StoreOpener::new(&config.store).home(&cwd).path(&opts.dest_dir);
     anyhow::ensure!(
-        !near_store::store_path_exists(&opts.dest_dir),
+        !dst_opener.check_if_exists(),
         "{}: directory already exists",
-        opts.dest_dir.display()
+        dst_opener.get_path().display()
     );
 
-    info!(target: "recompress", src = %src_dir.display(), dest = %opts.dest_dir.display(), "Recompressing database");
-    // TODO(#6857): Don’t use .path().
-    let src_store = StoreOpener::new(&config.store).read_only(true).path(&src_dir).open();
+    info!(target: "recompress", src = %src_opener.get_path().display(), dest = %dst_opener.get_path().display(), "Recompressing database");
+
+    let src_store = src_opener.open();
 
     let final_head_height = if skip_columns.contains(&DBCol::PartialChunks) {
         let tip: Option<near_primitives::block::Tip> =
             src_store.get_ser(DBCol::BlockMisc, near_store::FINAL_HEAD_KEY)?;
         anyhow::ensure!(
             tip.is_some(),
             "{}: missing {}; is this a freshly set up node? note that recompress_storage makes no sense on those",
-            src_dir.display(),
+            src_opener.get_path().display(),
             std::str::from_utf8(near_store::FINAL_HEAD_KEY).unwrap(),
         );
         tip.map(|tip| tip.height)
     } else {
         None
     };
 
-    // TODO(#6857): Don’t use .path().
-    let dst_store = StoreOpener::new(&config.store).path(&opts.dest_dir).open();
+    let dst_store = dst_opener.open();
 
     const BATCH_SIZE_BYTES: u64 = 150_000_000;