Removes `snapshot_bank()` wrapper fn (#28753)

core/tests/snapshots.rs

@@ -408,19 +408,29 @@ fn test_concurrent_snapshot_packaging(
             }
         };
 
-        snapshot_utils::snapshot_bank(
-            &bank,
-            vec![],
-            accounts_package_sender,
+        let snapshot_storages = bank.get_snapshot_storages(None);
+        let bank_snapshot_info = snapshot_utils::add_bank_snapshot(
             bank_snapshots_dir,
-            full_snapshot_archives_dir,
-            incremental_snapshot_archives_dir,
+            &bank,
+            &snapshot_storages,
             snapshot_config.snapshot_version,
-            snapshot_config.archive_format,
-            None,
-            AccountsPackageType::Snapshot(SnapshotType::FullSnapshot),
         )
         .unwrap();
+        let accounts_package = AccountsPackage::new(
+            AccountsPackageType::Snapshot(SnapshotType::FullSnapshot),
+            &bank,
+            &bank_snapshot_info,
+            bank_snapshots_dir,
+            vec![],
+            full_snapshot_archives_dir,
+            incremental_snapshot_archives_dir,
+            snapshot_storages,
+            snapshot_config.archive_format,
+            snapshot_config.snapshot_version,
+            None,
+        )
+        .unwrap();
+        accounts_package_sender.send(accounts_package).unwrap();
 
         bank_forks.insert(bank);
         if slot == saved_slot as u64 {

runtime/src/accounts_background_service.rs

@@ -374,19 +374,31 @@ impl SnapshotRequestHandler {
 
         // Snapshot the bank and send over an accounts package
         let mut snapshot_time = Measure::start("snapshot_time");
-        snapshot_utils::snapshot_bank(
-            &snapshot_root_bank,
-            status_cache_slot_deltas,
-            &self.accounts_package_sender,
+        let snapshot_storages = snapshot_utils::get_snapshot_storages(&snapshot_root_bank);
+        let bank_snapshot_info = snapshot_utils::add_bank_snapshot(
             &self.snapshot_config.bank_snapshots_dir,
-            &self.snapshot_config.full_snapshot_archives_dir,
-            &self.snapshot_config.incremental_snapshot_archives_dir,
+            &snapshot_root_bank,
+            &snapshot_storages,
             self.snapshot_config.snapshot_version,
-            self.snapshot_config.archive_format,
-            hash_for_testing,
-            accounts_package_type,
         )
         .expect("snapshot bank");
+        let accounts_package = AccountsPackage::new(
+            accounts_package_type,
+            &snapshot_root_bank,
+            &bank_snapshot_info,
+            &self.snapshot_config.bank_snapshots_dir,
+            status_cache_slot_deltas,
+            &self.snapshot_config.full_snapshot_archives_dir,
+            &self.snapshot_config.incremental_snapshot_archives_dir,
+            snapshot_storages,
+            self.snapshot_config.archive_format,
+            self.snapshot_config.snapshot_version,
+            hash_for_testing,
+        )
+        .expect("failed to hard link bank snapshot into a tmpdir");
+        self.accounts_package_sender
+            .send(accounts_package)
+            .expect("send accounts package");
         snapshot_time.stop();
         info!(
             "Took bank snapshot. accounts package type: {:?}, slot: {}, accounts hash: {}, bank hash: {}",

runtime/src/snapshot_utils.rs

@@ -747,12 +747,18 @@ where
 }
 
 /// Serialize a bank to a snapshot
-pub fn add_bank_snapshot<P: AsRef<Path>>(
-    bank_snapshots_dir: P,
+///
+/// **DEVELOPER NOTE** Any error that is returned from this function may bring down the node!  This
+/// function is called from AccountsBackgroundService to handle snapshot requests.  Since taking a
+/// snapshot is not permitted to fail, any errors returned here will trigger the node to shutdown.
+/// So, be careful whenever adding new code that may return errors.
+pub fn add_bank_snapshot(
+    bank_snapshots_dir: impl AsRef<Path>,
     bank: &Bank,
     snapshot_storages: &[SnapshotStorage],
     snapshot_version: SnapshotVersion,
 ) -> Result<BankSnapshotInfo> {
+    let mut add_snapshot_time = Measure::start("add-snapshot-ms");
     let slot = bank.slot();
     // bank_snapshots_dir/slot
     let bank_snapshots_dir = get_bank_snapshots_dir(bank_snapshots_dir, slot);
@@ -779,6 +785,7 @@ pub fn add_bank_snapshot<P: AsRef<Path>>(
     let consumed_size =
         serialize_snapshot_data_file(&bank_snapshot_path, bank_snapshot_serializer)?;
     bank_serialize.stop();
+    add_snapshot_time.stop();
 
     // Monitor sizes because they're capped to MAX_SNAPSHOT_DATA_FILE_SIZE
     datapoint_info!(
@@ -788,6 +795,7 @@ pub fn add_bank_snapshot<P: AsRef<Path>>(
     );
 
     inc_new_counter_info!("bank-serialize-ms", bank_serialize.as_ms() as usize);
+    inc_new_counter_info!("add-snapshot-ms", add_snapshot_time.as_ms() as usize);
 
     info!(
         "{} for slot {} at {}",
@@ -2076,62 +2084,8 @@ pub fn purge_old_bank_snapshots(bank_snapshots_dir: impl AsRef<Path>) {
     do_purge(get_bank_snapshots_post(&bank_snapshots_dir));
 }
 
-/// Gather the necessary elements for a snapshot of the given `root_bank`.
-///
-/// **DEVELOPER NOTE** Any error that is returned from this function may bring down the node!  This
-/// function is called from AccountsBackgroundService to handle snapshot requests.  Since taking a
-/// snapshot is not permitted to fail, any errors returned here will trigger the node to shutdown.
-/// So, be careful whenever adding new code that may return errors.
-#[allow(clippy::too_many_arguments)]
-pub fn snapshot_bank(
-    root_bank: &Bank,
-    status_cache_slot_deltas: Vec<BankSlotDelta>,
-    accounts_package_sender: &Sender<AccountsPackage>,
-    bank_snapshots_dir: impl AsRef<Path>,
-    full_snapshot_archives_dir: impl AsRef<Path>,
-    incremental_snapshot_archives_dir: impl AsRef<Path>,
-    snapshot_version: SnapshotVersion,
-    archive_format: ArchiveFormat,
-    hash_for_testing: Option<Hash>,
-    accounts_package_type: AccountsPackageType,
-) -> Result<()> {
-    let snapshot_storages = get_snapshot_storages(root_bank);
-
-    let mut add_snapshot_time = Measure::start("add-snapshot-ms");
-    let bank_snapshot_info = add_bank_snapshot(
-        &bank_snapshots_dir,
-        root_bank,
-        &snapshot_storages,
-        snapshot_version,
-    )?;
-    add_snapshot_time.stop();
-    inc_new_counter_info!("add-snapshot-ms", add_snapshot_time.as_ms() as usize);
-
-    let accounts_package = AccountsPackage::new(
-        accounts_package_type,
-        root_bank,
-        &bank_snapshot_info,
-        bank_snapshots_dir,
-        status_cache_slot_deltas,
-        full_snapshot_archives_dir,
-        incremental_snapshot_archives_dir,
-        snapshot_storages,
-        archive_format,
-        snapshot_version,
-        hash_for_testing,
-    )
-    .expect("failed to hard link bank snapshot into a tmpdir");
-
-    // Submit the accounts package
-    accounts_package_sender
-        .send(accounts_package)
-        .expect("send accounts package");
-
-    Ok(())
-}
-
 /// Get the snapshot storages for this bank
-fn get_snapshot_storages(bank: &Bank) -> SnapshotStorages {
+pub fn get_snapshot_storages(bank: &Bank) -> SnapshotStorages {
     let mut measure_snapshot_storages = Measure::start("snapshot-storages");
     let snapshot_storages = bank.get_snapshot_storages(None);
     measure_snapshot_storages.stop();