Add snapshot_utils::package_process_and_archive_xxx_snapshot() (Fixes #…

…18972)

core/tests/snapshots.rs

@@ -731,30 +731,23 @@ mod tests {
     ) -> snapshot_utils::Result<()> {
         let slot = bank.slot();
         info!("Making full snapshot archive from bank at slot: {}", slot);
-        let bank_snapshots = snapshot_utils::get_bank_snapshots(&snapshot_config.snapshot_path)
+        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 snapshot_package = snapshot_utils::package_full_snapshot(
+        snapshot_utils::package_process_and_archive_full_snapshot(
             bank,
-            &bank_snapshots,
+            &bank_snapshot_info,
             &snapshot_config.snapshot_path,
-            bank.src.slot_deltas(&bank.src.roots()),
             &snapshot_config.snapshot_package_output_path,
             bank.get_snapshot_storages(),
             snapshot_config.archive_format,
             snapshot_config.snapshot_version,
             None,
-        )?;
-        let snapshot_package = snapshot_utils::process_accounts_package_pre(
-            snapshot_package,
-            Some(bank.get_thread_pool()),
-            None,
-        );
-        snapshot_utils::archive_snapshot_package(
-            &snapshot_package,
             snapshot_config.maximum_snapshots_to_retain,
-        )
+        )?;
+
+        Ok(())
     }
 
     fn make_incremental_snapshot_archive(
@@ -767,31 +760,24 @@ mod tests {
             "Making incremental snapshot archive from bank at slot: {}, and base slot: {}",
             slot, incremental_snapshot_base_slot,
         );
-        let bank_snapshots = snapshot_utils::get_bank_snapshots(&snapshot_config.snapshot_path)
+        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 incremental_snapshot_package = snapshot_utils::package_incremental_snapshot(
+        snapshot_utils::package_process_and_archive_incremental_snapshot(
             bank,
             incremental_snapshot_base_slot,
-            &bank_snapshots,
+            &bank_snapshot_info,
             &snapshot_config.snapshot_path,
-            bank.src.slot_deltas(&bank.src.roots()),
             &snapshot_config.snapshot_package_output_path,
             bank.get_incremental_snapshot_storages(incremental_snapshot_base_slot),
             snapshot_config.archive_format,
             snapshot_config.snapshot_version,
             None,
-        )?;
-        let incremental_snapshot_package = snapshot_utils::process_accounts_package_pre(
-            incremental_snapshot_package,
-            Some(bank.get_thread_pool()),
-            Some(incremental_snapshot_base_slot),
-        );
-        snapshot_utils::archive_snapshot_package(
-            &incremental_snapshot_package,
             snapshot_config.maximum_snapshots_to_retain,
-        )
+        )?;
+
+        Ok(())
     }
 
     fn restore_from_incremental_snapshot_and_check_banks_are_equal(

runtime/src/snapshot_utils.rs

@@ -1776,25 +1776,20 @@ pub fn bank_to_full_snapshot_archive<P: AsRef<Path>, Q: AsRef<Path>>(
     bank.rehash(); // Bank accounts may have been manually modified by the caller
 
     let temp_dir = tempfile::tempdir_in(snapshots_dir)?;
-
     let storages = bank.get_snapshot_storages();
     let bank_snapshot_info = add_bank_snapshot(&temp_dir, bank, &storages, snapshot_version)?;
-    let package = package_full_snapshot(
+
+    package_process_and_archive_full_snapshot(
         bank,
         &bank_snapshot_info,
         &temp_dir,
-        bank.src.slot_deltas(&bank.src.roots()),
         snapshot_package_output_path,
         storages,
         archive_format,
         snapshot_version,
-        None,
-    )?;
-
-    let package = process_accounts_package_pre(package, thread_pool, None);
-
-    archive_snapshot_package(&package, maximum_snapshots_to_retain)?;
-    Ok(package.tar_output_file)
+        thread_pool,
+        maximum_snapshots_to_retain,
+    )
 }
 
 /// Convenience function to create an incremental snapshot archive out of any Bank, regardless of
@@ -1824,25 +1819,102 @@ pub fn bank_to_incremental_snapshot_archive<P: AsRef<Path>, Q: AsRef<Path>>(
     bank.rehash(); // Bank accounts may have been manually modified by the caller
 
     let temp_dir = tempfile::tempdir_in(snapshots_dir)?;
-
     let storages = bank.get_incremental_snapshot_storages(full_snapshot_slot);
     let bank_snapshot_info = add_bank_snapshot(&temp_dir, bank, &storages, snapshot_version)?;
-    let package = package_incremental_snapshot(
+
+    package_process_and_archive_incremental_snapshot(
         bank,
         full_snapshot_slot,
         &bank_snapshot_info,
         &temp_dir,
-        bank.src.slot_deltas(&bank.src.roots()),
         snapshot_package_output_path,
         storages,
         archive_format,
         snapshot_version,
+        thread_pool,
+        maximum_snapshots_to_retain,
+    )
+}
+
+/// Helper function to hold shared code to package, process, and archive full snapshots
+pub fn package_process_and_archive_full_snapshot(
+    bank: &Bank,
+    bank_snapshot_info: &BankSnapshotInfo,
+    snapshots_dir: impl AsRef<Path>,
+    snapshot_package_output_path: impl AsRef<Path>,
+    snapshot_storages: SnapshotStorages,
+    archive_format: ArchiveFormat,
+    snapshot_version: SnapshotVersion,
+    thread_pool: Option<&ThreadPool>,
+    maximum_snapshots_to_retain: usize,
+) -> Result<PathBuf> {
+    let package = package_full_snapshot(
+        bank,
+        bank_snapshot_info,
+        snapshots_dir,
+        bank.src.slot_deltas(&bank.src.roots()),
+        snapshot_package_output_path,
+        snapshot_storages,
+        archive_format,
+        snapshot_version,
         None,
     )?;
 
-    let package = process_accounts_package_pre(package, thread_pool, Some(full_snapshot_slot));
+    process_and_archive_snapshot_package_pre(
+        package,
+        thread_pool,
+        None,
+        maximum_snapshots_to_retain,
+    )
+}
+
+/// Helper function to hold shared code to package, process, and archive incremental snapshots
+#[allow(clippy::too_many_arguments)]
+pub fn package_process_and_archive_incremental_snapshot(
+    bank: &Bank,
+    incremental_snapshot_base_slot: Slot,
+    bank_snapshot_info: &BankSnapshotInfo,
+    snapshots_dir: impl AsRef<Path>,
+    snapshot_package_output_path: impl AsRef<Path>,
+    snapshot_storages: SnapshotStorages,
+    archive_format: ArchiveFormat,
+    snapshot_version: SnapshotVersion,
+    thread_pool: Option<&ThreadPool>,
+    maximum_snapshots_to_retain: usize,
+) -> Result<PathBuf> {
+    let package = package_incremental_snapshot(
+        bank,
+        incremental_snapshot_base_slot,
+        bank_snapshot_info,
+        snapshots_dir,
+        bank.src.slot_deltas(&bank.src.roots()),
+        snapshot_package_output_path,
+        snapshot_storages,
+        archive_format,
+        snapshot_version,
+        None,
+    )?;
+
+    process_and_archive_snapshot_package_pre(
+        package,
+        thread_pool,
+        Some(incremental_snapshot_base_slot),
+        maximum_snapshots_to_retain,
+    )
+}
+
+/// Helper function to hold shared code to process and archive snapshot packages
+fn process_and_archive_snapshot_package_pre(
+    package_pre: AccountsPackagePre,
+    thread_pool: Option<&ThreadPool>,
+    incremental_snapshot_base_slot: Option<Slot>,
+    maximum_snapshots_to_retain: usize,
+) -> Result<PathBuf> {
+    let package =
+        process_accounts_package_pre(package_pre, thread_pool, incremental_snapshot_base_slot);
 
     archive_snapshot_package(&package, maximum_snapshots_to_retain)?;
+
     Ok(package.tar_output_file)
 }