AHV processes the snapshot dirs in place (#30978)

* AHV processes the snapshot dirs in place

Let account pacakge use the snapshot dir, so AHV computes the accounts hash and turns the pre snapshot dir into a post snapshot dir

* fix status cache path to maintain the archive layout for the in-place snapshot dir archiving

* fix test_package_snapshots

* Fix test_concurrent_snapshot_packaging

* Remove debug change.

* Fix snapshot_links path

* change to borrow for bank_snapshots_dir

* Reverted changes in create_and_verify_snapshot

* Fix param errors

* Fix rebase errors

* Remove NOTE 1

* Remove unwrap

* Remove the variables to make it apparent taht snapshot_links is the bank_snapshots_dir

* Use soft link instead of hard link for snapshot and status cache

* After switching to soft symlinking, the src path should be absolute

core/src/accounts_hash_verifier.rs

@@ -297,7 +297,7 @@ impl AccountsHashVerifier {
 
         if let Some(snapshot_info) = &accounts_package.snapshot_info {
             solana_runtime::serde_snapshot::reserialize_bank_with_new_accounts_hash(
-                snapshot_info.snapshot_links.path(),
+                &snapshot_info.snapshot_links,
                 accounts_package.slot,
                 &accounts_hash_for_reserialize,
                 bank_incremental_snapshot_persistence.as_ref(),

core/src/snapshot_packager_service.rs

@@ -285,7 +285,7 @@ mod tests {
                     archive_format: ArchiveFormat::Tar,
                 },
                 block_height: slot,
-                snapshot_links: TempDir::new().unwrap(),
+                snapshot_links: PathBuf::default(),
                 snapshot_storages: Vec::default(),
                 snapshot_version: SnapshotVersion::default(),
                 snapshot_type,

core/tests/snapshots.rs

@@ -6,7 +6,6 @@ use {
     fs_extra::dir::CopyOptions,
     itertools::Itertools,
     log::{info, trace},
-    snapshot_utils::MAX_BANK_SNAPSHOTS_TO_RETAIN,
     solana_core::{
         accounts_hash_verifier::AccountsHashVerifier,
         snapshot_packager_service::SnapshotPackagerService,
@@ -32,6 +31,7 @@ use {
         snapshot_utils::{
             self, ArchiveFormat,
             SnapshotVersion::{self, V1_2_0},
+            MAX_BANK_SNAPSHOTS_TO_RETAIN,
         },
         status_cache::MAX_CACHE_ENTRIES,
     },

runtime/src/snapshot_package.rs

@@ -10,18 +10,15 @@ use {
         snapshot_hash::SnapshotHash,
         snapshot_utils::{
             self, ArchiveFormat, BankSnapshotInfo, Result, SnapshotError, SnapshotVersion,
-            SNAPSHOT_STATUS_CACHE_FILENAME, TMP_BANK_SNAPSHOT_PREFIX,
         },
     },
     log::*,
     solana_sdk::{clock::Slot, feature_set, sysvar::epoch_schedule::EpochSchedule},
     std::{
-        fs,
         path::{Path, PathBuf},
         sync::Arc,
         time::Instant,
     },
-    tempfile::TempDir,
 };
 
 mod compare;
@@ -78,32 +75,14 @@ impl AccountsPackage {
             }
         }
 
-        // Hard link the snapshot into a tmpdir, to ensure its not removed prior to packaging.
         let bank_snapshot_dir = &bank_snapshot_info.snapshot_dir;
         let bank_snapshots_dir = bank_snapshot_dir
             .parent()
-            .ok_or_else(|| SnapshotError::InvalidSnapshotDirPath(bank_snapshot_dir.clone()))?;
-        let snapshot_links = tempfile::Builder::new()
-            .prefix(&format!("{}{}-", TMP_BANK_SNAPSHOT_PREFIX, bank.slot()))
-            .tempdir_in(bank_snapshots_dir)?;
-        {
-            let snapshot_hardlink_dir = snapshot_links
-                .path()
-                .join(bank_snapshot_info.slot.to_string());
-            fs::create_dir_all(&snapshot_hardlink_dir)?;
-            let snapshot_path = bank_snapshot_info.snapshot_path();
-            let file_name = snapshot_utils::path_to_file_name_str(&snapshot_path)?;
-            fs::hard_link(&snapshot_path, snapshot_hardlink_dir.join(file_name))?;
-            let status_cache_path = bank_snapshot_dir.join(SNAPSHOT_STATUS_CACHE_FILENAME);
-            let status_cache_file_name = snapshot_utils::path_to_file_name_str(&status_cache_path)?;
-            fs::hard_link(
-                &status_cache_path,
-                snapshot_links.path().join(status_cache_file_name),
-            )?;
-        }
+            .ok_or_else(|| SnapshotError::InvalidSnapshotDirPath(bank_snapshot_dir.to_path_buf()))?
+            .to_path_buf();
 
         let snapshot_info = SupplementalSnapshotInfo {
-            snapshot_links,
+            snapshot_links: bank_snapshots_dir,
             archive_format,
             snapshot_version,
             full_snapshot_archives_dir: full_snapshot_archives_dir.as_ref().to_path_buf(),
@@ -180,7 +159,7 @@ impl AccountsPackage {
             rent_collector: RentCollector::default(),
             is_incremental_accounts_hash_feature_enabled: bool::default(),
             snapshot_info: Some(SupplementalSnapshotInfo {
-                snapshot_links: TempDir::new().unwrap(),
+                snapshot_links: PathBuf::default(),
                 archive_format: ArchiveFormat::Tar,
                 snapshot_version: SnapshotVersion::default(),
                 full_snapshot_archives_dir: PathBuf::default(),
@@ -193,14 +172,15 @@ impl AccountsPackage {
 
     /// Returns the path to the snapshot links directory
     ///
-    /// NOTE 1: This path is within the TempDir created for the AccountsPackage, *not* the bank
-    ///         snapshots dir passed into `new_for_snapshot()` when creating the AccountsPackage.
-    /// NOTE 2: This fn will panic if the AccountsPackage is of type EpochAccountsHash.
+    /// NOTE: This fn will panic if the AccountsPackage is of type EpochAccountsHash.
     pub fn snapshot_links_dir(&self) -> &Path {
         match self.package_type {
-            AccountsPackageType::AccountsHashVerifier | AccountsPackageType::Snapshot(..) => {
-                self.snapshot_info.as_ref().unwrap().snapshot_links.path()
-            }
+            AccountsPackageType::AccountsHashVerifier | AccountsPackageType::Snapshot(..) => self
+                .snapshot_info
+                .as_ref()
+                .unwrap()
+                .snapshot_links
+                .as_path(),
             AccountsPackageType::EpochAccountsHash => {
                 panic!("EAH accounts packages do not contain snapshot information")
             }
@@ -220,7 +200,7 @@ impl std::fmt::Debug for AccountsPackage {
 
 /// Supplemental information needed for snapshots
 pub struct SupplementalSnapshotInfo {
-    pub snapshot_links: TempDir,
+    pub snapshot_links: PathBuf,
     pub archive_format: ArchiveFormat,
     pub snapshot_version: SnapshotVersion,
     pub full_snapshot_archives_dir: PathBuf,
@@ -242,7 +222,7 @@ pub enum AccountsPackageType {
 pub struct SnapshotPackage {
     pub snapshot_archive_info: SnapshotArchiveInfo,
     pub block_height: Slot,
-    pub snapshot_links: TempDir,
+    pub snapshot_links: PathBuf,
     pub snapshot_storages: Vec<Arc<AccountStorageEntry>>,
     pub snapshot_version: SnapshotVersion,
     pub snapshot_type: SnapshotType,

runtime/src/snapshot_package/compare.rs

@@ -80,7 +80,6 @@ mod tests {
         },
         solana_sdk::{clock::Slot, hash::Hash},
         std::{path::PathBuf, time::Instant},
-        tempfile::TempDir,
     };
 
     #[test]
@@ -94,7 +93,7 @@ mod tests {
                     archive_format: ArchiveFormat::Tar,
                 },
                 block_height: slot,
-                snapshot_links: TempDir::new().unwrap(),
+                snapshot_links: PathBuf::default(),
                 snapshot_storages: Vec::default(),
                 snapshot_version: SnapshotVersion::default(),
                 snapshot_type,

runtime/src/snapshot_utils.rs

@@ -83,7 +83,6 @@ pub const DEFAULT_INCREMENTAL_SNAPSHOT_ARCHIVE_INTERVAL_SLOTS: Slot = 100;
 const MAX_SNAPSHOT_DATA_FILE_SIZE: u64 = 32 * 1024 * 1024 * 1024; // 32 GiB
 const MAX_SNAPSHOT_VERSION_FILE_SIZE: u64 = 8; // byte
 const VERSION_STRING_V1_2_0: &str = "1.2.0";
-pub(crate) const TMP_BANK_SNAPSHOT_PREFIX: &str = "tmp-bank-snapshot-";
 pub const TMP_SNAPSHOT_ARCHIVE_PREFIX: &str = "tmp-snapshot-archive-";
 pub const BANK_SNAPSHOT_PRE_FILENAME_EXTENSION: &str = "pre";
 // Save some bank snapshots but not too many
@@ -623,16 +622,43 @@ pub fn archive_snapshot_package(
     let staging_accounts_dir = staging_dir.path().join("accounts");
     let staging_snapshots_dir = staging_dir.path().join("snapshots");
     let staging_version_file = staging_dir.path().join(SNAPSHOT_VERSION_FILENAME);
+
+    // Create staging/accounts/
     fs::create_dir_all(&staging_accounts_dir).map_err(|e| {
-        SnapshotError::IoWithSourceAndFile(e, "create staging path", staging_accounts_dir.clone())
+        SnapshotError::IoWithSourceAndFile(
+            e,
+            "create staging accounts path",
+            staging_accounts_dir.clone(),
+        )
     })?;
 
-    // Add the snapshots to the staging directory
-    symlink::symlink_dir(
-        snapshot_package.snapshot_links.path(),
-        staging_snapshots_dir,
-    )
-    .map_err(|e| SnapshotError::IoWithSource(e, "create staging symlinks"))?;
+    let slot_str = snapshot_package.slot().to_string();
+    let staging_snapshot_dir = staging_snapshots_dir.join(&slot_str);
+    // Creates staging snapshots/<slot>/
+    fs::create_dir_all(&staging_snapshot_dir).map_err(|e| {
+        SnapshotError::IoWithSourceAndFile(
+            e,
+            "create staging snapshots path",
+            staging_snapshots_dir.clone(),
+        )
+    })?;
+
+    let src_snapshot_dir = snapshot_package.snapshot_links.join(&slot_str);
+    // To be a source for symlinking and archiving, the path need to be an aboslute path
+    let src_snapshot_dir = src_snapshot_dir
+        .canonicalize()
+        .map_err(|_e| SnapshotError::InvalidSnapshotDirPath(src_snapshot_dir.clone()))?;
+    let staging_snapshot_file = staging_snapshot_dir.join(&slot_str);
+    let src_snapshot_file = src_snapshot_dir.join(slot_str);
+    symlink::symlink_file(src_snapshot_file, staging_snapshot_file)
+        .map_err(|e| SnapshotError::IoWithSource(e, "create snapshot symlink"))?;
+
+    // Following the existing archive format, the status cache is under snapshots/, not under <slot>/
+    // like in the snapshot dir.
+    let staging_status_cache = staging_snapshots_dir.join(SNAPSHOT_STATUS_CACHE_FILENAME);
+    let src_status_cache = src_snapshot_dir.join(SNAPSHOT_STATUS_CACHE_FILENAME);
+    symlink::symlink_file(src_status_cache, staging_status_cache)
+        .map_err(|e| SnapshotError::IoWithSource(e, "create status cache symlink"))?;
 
     // Add the AppendVecs into the compressible list
     for storage in snapshot_package.snapshot_storages.iter() {