Name snapshots consistently (#19346)

#### Problem

Snapshot names are overloaded, and there are multiple terms that mean the same thing. This is confusing. Here's a list of ones in the codebase that I've found:

```
- snapshot_dir
- snapshots_dir
- snapshot_path
- snapshot_output_dir
- snapshot_package_output_path
- snapshot_archives_dir
```

#### Summary of Changes

For all the ones that are about the directory where snapshot archives are stored, ensure they are `snapshot_archives_dir`. For the ones about the (bank) snapshots directory, set to `bank_snapshots_dir`.


Co-authored-by: Michael Vines <mvines@gmail.com>

core/src/accounts_hash_verifier.rs

@@ -293,8 +293,8 @@ mod tests {
         let snapshot_config = SnapshotConfig {
             full_snapshot_archive_interval_slots,
             incremental_snapshot_archive_interval_slots: Slot::MAX,
-            snapshot_package_output_path: PathBuf::default(),
-            snapshot_path: PathBuf::default(),
+            snapshot_archives_dir: PathBuf::default(),
+            bank_snapshots_dir: PathBuf::default(),
             archive_format: ArchiveFormat::Tar,
             snapshot_version: SnapshotVersion::default(),
             maximum_snapshots_to_retain: usize::MAX,

core/src/snapshot_packager_service.rs

@@ -121,8 +121,8 @@ mod tests {
     fn create_and_verify_snapshot(temp_dir: &Path) {
         let accounts_dir = temp_dir.join("accounts");
         let snapshots_dir = temp_dir.join("snapshots");
-        let snapshot_package_output_path = temp_dir.join("snapshots_output");
-        fs::create_dir_all(&snapshot_package_output_path).unwrap();
+        let snapshot_archives_dir = temp_dir.join("snapshots_output");
+        fs::create_dir_all(&snapshot_archives_dir).unwrap();
 
         fs::create_dir_all(&accounts_dir).unwrap();
         // Create some storage entries
@@ -161,7 +161,7 @@ mod tests {
 
         // Create a packageable snapshot
         let output_tar_path = snapshot_utils::build_full_snapshot_archive_path(
-            snapshot_package_output_path,
+            snapshot_archives_dir,
             42,
             &Hash::default(),
             ArchiveFormat::TarBzip2,

core/src/test_validator.rs

@@ -523,8 +523,8 @@ impl TestValidator {
             snapshot_config: Some(SnapshotConfig {
                 full_snapshot_archive_interval_slots: 100,
                 incremental_snapshot_archive_interval_slots: Slot::MAX,
-                snapshot_path: ledger_path.join("snapshot"),
-                snapshot_package_output_path: ledger_path.to_path_buf(),
+                bank_snapshots_dir: ledger_path.join("snapshot"),
+                snapshot_archives_dir: ledger_path.to_path_buf(),
                 archive_format: ArchiveFormat::Tar,
                 snapshot_version: SnapshotVersion::default(),
                 maximum_snapshots_to_retain: DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN,

core/src/validator.rs

@@ -1210,7 +1210,7 @@ fn new_banks_from_ledger(
             ledger_path,
             &bank_forks.root_bank(),
             None,
-            &snapshot_config.snapshot_package_output_path,
+            &snapshot_config.snapshot_archives_dir,
             snapshot_config.archive_format,
             Some(bank_forks.root_bank().get_thread_pool()),
             snapshot_config.maximum_snapshots_to_retain,

core/tests/snapshots.rs

@@ -141,8 +141,8 @@ mod tests {
             let snapshot_config = SnapshotConfig {
                 full_snapshot_archive_interval_slots,
                 incremental_snapshot_archive_interval_slots,
-                snapshot_package_output_path: snapshot_archives_dir.path().to_path_buf(),
-                snapshot_path: bank_snapshots_dir.path().to_path_buf(),
+                snapshot_archives_dir: snapshot_archives_dir.path().to_path_buf(),
+                bank_snapshots_dir: bank_snapshots_dir.path().to_path_buf(),
                 archive_format: ArchiveFormat::TarBzip2,
                 snapshot_version,
                 maximum_snapshots_to_retain: DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN,
@@ -166,17 +166,17 @@ mod tests {
         old_genesis_config: &GenesisConfig,
         account_paths: &[PathBuf],
     ) {
-        let (snapshot_path, snapshot_package_output_path) = old_bank_forks
+        let (snapshot_path, snapshot_archives_dir) = old_bank_forks
             .snapshot_config
             .as_ref()
-            .map(|c| (&c.snapshot_path, &c.snapshot_package_output_path))
+            .map(|c| (&c.bank_snapshots_dir, &c.snapshot_archives_dir))
             .unwrap();
 
         let old_last_bank = old_bank_forks.get(old_last_slot).unwrap();
 
         let check_hash_calculation = false;
         let full_snapshot_archive_path = snapshot_utils::build_full_snapshot_archive_path(
-            snapshot_package_output_path.to_path_buf(),
+            snapshot_archives_dir.to_path_buf(),
             old_last_bank.slot(),
             &old_last_bank.get_accounts_hash(),
             ArchiveFormat::TarBzip2,
@@ -191,7 +191,7 @@ mod tests {
                 .snapshot_config
                 .as_ref()
                 .unwrap()
-                .snapshot_path,
+                .bank_snapshots_dir,
             &full_snapshot_archive_info,
             None,
             old_genesis_config,
@@ -273,15 +273,16 @@ mod tests {
         // Generate a snapshot package for last bank
         let last_bank = bank_forks.get(last_slot).unwrap();
         let snapshot_config = &snapshot_test_config.snapshot_config;
-        let snapshot_path = &snapshot_config.snapshot_path;
-        let last_bank_snapshot_info = snapshot_utils::get_highest_bank_snapshot_info(snapshot_path)
-            .expect("no snapshots found in path");
+        let bank_snapshots_dir = &snapshot_config.bank_snapshots_dir;
+        let last_bank_snapshot_info =
+            snapshot_utils::get_highest_bank_snapshot_info(bank_snapshots_dir)
+                .expect("no bank snapshots found in path");
         let accounts_package = AccountsPackage::new_for_full_snapshot(
             last_bank,
             &last_bank_snapshot_info,
-            snapshot_path,
+            bank_snapshots_dir,
             last_bank.src.slot_deltas(&last_bank.src.roots()),
-            &snapshot_config.snapshot_package_output_path,
+            &snapshot_config.snapshot_archives_dir,
             last_bank.get_snapshot_storages(None),
             ArchiveFormat::TarBzip2,
             snapshot_version,
@@ -355,17 +356,16 @@ mod tests {
             SnapshotTestConfig::new(snapshot_version, cluster_type, 1, 1, Slot::MAX);
 
         let bank_forks = &mut snapshot_test_config.bank_forks;
-        let bank_snapshots_dir = &snapshot_test_config.bank_snapshots_dir;
         let snapshot_config = &snapshot_test_config.snapshot_config;
-        let snapshot_path = &snapshot_config.snapshot_path;
-        let snapshot_package_output_path = &snapshot_config.snapshot_package_output_path;
+        let bank_snapshots_dir = &snapshot_config.bank_snapshots_dir;
+        let snapshot_archives_dir = &snapshot_config.snapshot_archives_dir;
         let mint_keypair = &snapshot_test_config.genesis_config_info.mint_keypair;
         let genesis_config = &snapshot_test_config.genesis_config_info.genesis_config;
 
         // Take snapshot of zeroth bank
         let bank0 = bank_forks.get(0).unwrap();
         let storages = bank0.get_snapshot_storages(None);
-        snapshot_utils::add_bank_snapshot(snapshot_path, bank0, &storages, snapshot_version)
+        snapshot_utils::add_bank_snapshot(bank_snapshots_dir, bank0, &storages, snapshot_version)
             .unwrap();
 
         // Set up snapshotting channels
@@ -412,8 +412,8 @@ mod tests {
                 &bank,
                 vec![],
                 package_sender,
-                snapshot_path,
-                snapshot_package_output_path,
+                bank_snapshots_dir,
+                snapshot_archives_dir,
                 snapshot_config.snapshot_version,
                 &snapshot_config.archive_format,
                 None,
@@ -443,7 +443,7 @@ mod tests {
                     )
                     .unwrap();
                 }
-                let last_snapshot_path = fs::read_dir(snapshot_path)
+                let last_snapshot_path = fs::read_dir(bank_snapshots_dir)
                     .unwrap()
                     .filter_map(|entry| {
                         let e = entry.unwrap();
@@ -462,7 +462,7 @@ mod tests {
                 fs_extra::dir::copy(&last_snapshot_path, &saved_snapshots_dir, &options).unwrap();
 
                 saved_archive_path = Some(snapshot_utils::build_full_snapshot_archive_path(
-                    snapshot_package_output_path.to_path_buf(),
+                    snapshot_archives_dir.to_path_buf(),
                     slot,
                     &accounts_hash,
                     ArchiveFormat::TarBzip2,
@@ -472,7 +472,7 @@ mod tests {
 
         // Purge all the outdated snapshots, including the ones needed to generate the package
         // currently sitting in the channel
-        snapshot_utils::purge_old_bank_snapshots(snapshot_path);
+        snapshot_utils::purge_old_bank_snapshots(bank_snapshots_dir);
 
         let mut bank_snapshots = snapshot_utils::get_bank_snapshots(&bank_snapshots_dir);
         bank_snapshots.sort_unstable();
@@ -754,15 +754,21 @@ mod tests {
     ) -> snapshot_utils::Result<()> {
         let slot = bank.slot();
         info!("Making full snapshot archive from bank at slot: {}", slot);
-        let bank_snapshot_info = snapshot_utils::get_bank_snapshots(&snapshot_config.snapshot_path)
-            .into_iter()
-            .find(|elem| elem.slot == slot)
-            .ok_or_else(|| Error::new(ErrorKind::Other, "did not find snapshot with this path"))?;
+        let bank_snapshot_info =
+            snapshot_utils::get_bank_snapshots(&snapshot_config.bank_snapshots_dir)
+                .into_iter()
+                .find(|elem| elem.slot == slot)
+                .ok_or_else(|| {
+                    Error::new(
+                        ErrorKind::Other,
+                        "did not find bank snapshot with this path",
+                    )
+                })?;
         snapshot_utils::package_process_and_archive_full_snapshot(
             bank,
             &bank_snapshot_info,
-            &snapshot_config.snapshot_path,
-            &snapshot_config.snapshot_package_output_path,
+            &snapshot_config.bank_snapshots_dir,
+            &snapshot_config.snapshot_archives_dir,
             bank.get_snapshot_storages(None),
             snapshot_config.archive_format,
             snapshot_config.snapshot_version,
@@ -783,10 +789,16 @@ mod tests {
             "Making incremental snapshot archive from bank at slot: {}, and base slot: {}",
             slot, incremental_snapshot_base_slot,
         );
-        let bank_snapshot_info = snapshot_utils::get_bank_snapshots(&snapshot_config.snapshot_path)
-            .into_iter()
-            .find(|elem| elem.slot == slot)
-            .ok_or_else(|| Error::new(ErrorKind::Other, "did not find snapshot with this path"))?;
+        let bank_snapshot_info =
+            snapshot_utils::get_bank_snapshots(&snapshot_config.bank_snapshots_dir)
+                .into_iter()
+                .find(|elem| elem.slot == slot)
+                .ok_or_else(|| {
+                    Error::new(
+                        ErrorKind::Other,
+                        "did not find bank snapshot with this path",
+                    )
+                })?;
         let storages = {
             let mut storages = bank.get_snapshot_storages(Some(incremental_snapshot_base_slot));
             snapshot_utils::filter_snapshot_storages_for_incremental_snapshot(
@@ -799,8 +811,8 @@ mod tests {
             bank,
             incremental_snapshot_base_slot,
             &bank_snapshot_info,
-            &snapshot_config.snapshot_path,
-            &snapshot_config.snapshot_package_output_path,
+            &snapshot_config.bank_snapshots_dir,
+            &snapshot_config.snapshot_archives_dir,
             storages,
             snapshot_config.archive_format,
             snapshot_config.snapshot_version,
@@ -818,8 +830,8 @@ mod tests {
         genesis_config: &GenesisConfig,
     ) -> snapshot_utils::Result<()> {
         let (deserialized_bank, _) = snapshot_utils::bank_from_latest_snapshot_archives(
-            &snapshot_config.snapshot_path,
-            &snapshot_config.snapshot_package_output_path,
+            &snapshot_config.bank_snapshots_dir,
+            &snapshot_config.snapshot_archives_dir,
             &[accounts_dir],
             &[],
             genesis_config,
@@ -996,10 +1008,8 @@ mod tests {
         info!("Awake! Rebuilding bank from latest snapshot archives...");
 
         let (deserialized_bank, _) = snapshot_utils::bank_from_latest_snapshot_archives(
-            &snapshot_test_config.snapshot_config.snapshot_path,
-            &snapshot_test_config
-                .snapshot_config
-                .snapshot_package_output_path,
+            &snapshot_test_config.snapshot_config.bank_snapshots_dir,
+            &snapshot_test_config.snapshot_config.snapshot_archives_dir,
             &[snapshot_test_config.accounts_dir.as_ref().to_path_buf()],
             &[],
             &snapshot_test_config.genesis_config_info.genesis_config,