Fixes BankFrom.*Timings (#34249)

2023-11-28 13:05:54 -05:00 · 2023-11-28 13:05:54 -05:00 · b9ef204faf
parent 4d2018fc6e
commit b9ef204faf
2 changed files with 68 additions and 91 deletions
--- a/runtime/src/snapshot_bank_utils.rs
+++ b/runtime/src/snapshot_bank_utils.rs
@ -13,16 +13,16 @@ use {
        snapshot_hash::SnapshotHash,
        snapshot_package::{AccountsPackage, AccountsPackageKind, SnapshotKind, SnapshotPackage},
        snapshot_utils::{
-            self, archive_snapshot_package, build_storage_from_snapshot_dir,
-            delete_contents_of_path, deserialize_snapshot_data_file,
-            deserialize_snapshot_data_files, get_bank_snapshot_dir, get_highest_bank_snapshot_post,
-            get_highest_full_snapshot_archive_info, get_highest_incremental_snapshot_archive_info,
-            get_snapshot_file_name, get_storages_to_serialize, hard_link_storages_to_snapshot,
-            serialize_snapshot_data_file, verify_and_unarchive_snapshots,
-            verify_unpacked_snapshots_dir_and_version, write_snapshot_version_file,
-            AddBankSnapshotError, ArchiveFormat, BankSnapshotInfo, BankSnapshotType, SnapshotError,
-            SnapshotRootPaths, SnapshotVersion, StorageAndNextAppendVecId,
-            UnpackedSnapshotsDirAndVersion, VerifySlotDeltasError,
+            self, archive_snapshot_package, delete_contents_of_path,
+            deserialize_snapshot_data_file, deserialize_snapshot_data_files, get_bank_snapshot_dir,
+            get_highest_bank_snapshot_post, get_highest_full_snapshot_archive_info,
+            get_highest_incremental_snapshot_archive_info, get_snapshot_file_name,
+            get_storages_to_serialize, hard_link_storages_to_snapshot,
+            rebuild_storages_from_snapshot_dir, serialize_snapshot_data_file,
+            verify_and_unarchive_snapshots, verify_unpacked_snapshots_dir_and_version,
+            write_snapshot_version_file, AddBankSnapshotError, ArchiveFormat, BankSnapshotInfo,
+            BankSnapshotType, SnapshotError, SnapshotRootPaths, SnapshotVersion,
+            StorageAndNextAppendVecId, UnpackedSnapshotsDirAndVersion, VerifySlotDeltasError,
        },
        status_cache,
    },
@ -202,18 +202,18 @@ fn serialize_status_cache(
    })
 }

-#[derive(Debug, Default)]
-pub struct BankFromArchiveTimings {
-    pub rebuild_bank_from_snapshots_us: u64,
-    pub full_snapshot_untar_us: u64,
-    pub incremental_snapshot_untar_us: u64,
-    pub verify_snapshot_bank_us: u64,
+#[derive(Debug)]
+pub struct BankFromArchivesTimings {
+    pub untar_full_snapshot_archive_us: u64,
+    pub untar_incremental_snapshot_archive_us: u64,
+    pub rebuild_bank_us: u64,
+    pub verify_bank_us: u64,
 }

-#[derive(Debug, Default)]
+#[derive(Debug)]
 pub struct BankFromDirTimings {
-    pub rebuild_bank_from_snapshot_us: u64,
-    pub build_storage_us: u64,
+    pub rebuild_storages_us: u64,
+    pub rebuild_bank_us: u64,
 }

 /// Utility for parsing out bank specific information from a snapshot archive. This utility can be used
@ -276,7 +276,7 @@ pub fn bank_from_snapshot_archives(
    accounts_db_config: Option<AccountsDbConfig>,
    accounts_update_notifier: Option<AccountsUpdateNotifier>,
    exit: Arc<AtomicBool>,
-) -> snapshot_utils::Result<(Bank, BankFromArchiveTimings)> {
+) -> snapshot_utils::Result<(Bank, BankFromArchivesTimings)> {
    info!(
        "Loading bank from full snapshot archive: {}, and incremental snapshot archive: {:?}",
        full_snapshot_archive_info.path().display(),
@ -375,37 +375,29 @@ pub fn bank_from_snapshot_archives(
    }
    measure_verify.stop();

-    let timings = BankFromArchiveTimings {
-        rebuild_bank_from_snapshots_us: measure_rebuild.as_us(),
-        full_snapshot_untar_us: unarchived_full_snapshot.measure_untar.as_us(),
-        incremental_snapshot_untar_us: unarchived_incremental_snapshot
+    let timings = BankFromArchivesTimings {
+        untar_full_snapshot_archive_us: unarchived_full_snapshot.measure_untar.as_us(),
+        untar_incremental_snapshot_archive_us: unarchived_incremental_snapshot
            .map_or(0, |unarchive_preparation_result| {
                unarchive_preparation_result.measure_untar.as_us()
            }),
-        verify_snapshot_bank_us: measure_verify.as_us(),
+        rebuild_bank_us: measure_rebuild.as_us(),
+        verify_bank_us: measure_verify.as_us(),
    };
    datapoint_info!(
        "bank_from_snapshot_archives",
        (
-            "full_snapshot_untar_us",
-            timings.full_snapshot_untar_us,
+            "untar_full_snapshot_archive_us",
+            timings.untar_full_snapshot_archive_us,
            i64
        ),
        (
-            "incremental_snapshot_untar_us",
-            timings.incremental_snapshot_untar_us,
-            i64
-        ),
-        (
-            "rebuild_bank_from_snapshots_us",
-            timings.rebuild_bank_from_snapshots_us,
-            i64
-        ),
-        (
-            "verify_snapshot_bank_us",
-            timings.verify_snapshot_bank_us,
+            "untar_incremental_snapshot_archive_us",
+            timings.untar_incremental_snapshot_archive_us,
            i64
        ),
+        ("rebuild_bank_us", timings.rebuild_bank_us, i64),
+        ("verify_bank_us", timings.verify_bank_us, i64),
    );
    Ok((bank, timings))
 }
@ -506,11 +498,15 @@ pub fn bank_from_snapshot_dir(

    let next_append_vec_id = Arc::new(AtomicAppendVecId::new(0));

-    let (storage, measure_build_storage) = measure!(
-        build_storage_from_snapshot_dir(bank_snapshot, account_paths, next_append_vec_id.clone())?,
-        "build storage from snapshot dir"
+    let (storage, measure_rebuild_storages) = measure!(
+        rebuild_storages_from_snapshot_dir(
+            bank_snapshot,
+            account_paths,
+            next_append_vec_id.clone()
+        )?,
+        "rebuild storages from snapshot dir"
    );
-    info!("{}", measure_build_storage);
+    info!("{}", measure_rebuild_storages);

    let next_append_vec_id =
        Arc::try_unwrap(next_append_vec_id).expect("this is the only strong reference");
@ -518,46 +514,39 @@ pub fn bank_from_snapshot_dir(
        storage,
        next_append_vec_id,
    };
-    let mut measure_rebuild = Measure::start("rebuild bank from snapshot");
-    let bank = rebuild_bank_from_snapshot(
-        bank_snapshot,
-        account_paths,
-        storage_and_next_append_vec_id,
-        genesis_config,
-        runtime_config,
-        debug_keys,
-        additional_builtins,
-        account_secondary_indexes,
-        limit_load_slot_count_from_snapshot,
-        shrink_ratio,
-        verify_index,
-        accounts_db_config,
-        accounts_update_notifier,
-        exit,
-    )?;
-    measure_rebuild.stop();
-    info!("{}", measure_rebuild);
+    let (bank, measure_rebuild_bank) = measure!(
+        rebuild_bank_from_snapshot(
+            bank_snapshot,
+            account_paths,
+            storage_and_next_append_vec_id,
+            genesis_config,
+            runtime_config,
+            debug_keys,
+            additional_builtins,
+            account_secondary_indexes,
+            limit_load_slot_count_from_snapshot,
+            shrink_ratio,
+            verify_index,
+            accounts_db_config,
+            accounts_update_notifier,
+            exit,
+        )?,
+        "rebuild bank from snapshot"
+    );
+    info!("{}", measure_rebuild_bank);

    // Skip bank.verify_snapshot_bank.  Subsequent snapshot requests/accounts hash verification requests
    // will calculate and check the accounts hash, so we will still have safety/correctness there.
    bank.set_initial_accounts_hash_verification_completed();

    let timings = BankFromDirTimings {
-        rebuild_bank_from_snapshot_us: measure_rebuild.as_us(),
-        build_storage_us: measure_build_storage.as_us(),
+        rebuild_storages_us: measure_rebuild_storages.as_us(),
+        rebuild_bank_us: measure_rebuild_bank.as_us(),
    };
    datapoint_info!(
        "bank_from_snapshot_dir",
-        (
-            "build_storage_from_snapshot_dir_us",
-            timings.build_storage_us,
-            i64
-        ),
-        (
-            "rebuild_bank_from_snapshot_us",
-            timings.rebuild_bank_from_snapshot_us,
-            i64
-        ),
+        ("rebuild_storages_us", timings.rebuild_storages_us, i64),
+        ("rebuild_bank_us", timings.rebuild_bank_us, i64),
    );
    Ok((bank, timings))
 }
--- a/runtime/src/snapshot_utils.rs
+++ b/runtime/src/snapshot_utils.rs
@ -1211,20 +1211,6 @@ pub(crate) fn get_storages_to_serialize(
        .collect::<Vec<_>>()
 }

-#[derive(Debug, Default)]
-pub struct BankFromArchiveTimings {
-    pub rebuild_bank_from_snapshots_us: u64,
-    pub full_snapshot_untar_us: u64,
-    pub incremental_snapshot_untar_us: u64,
-    pub verify_snapshot_bank_us: u64,
-}
-
-#[derive(Debug, Default)]
-pub struct BankFromDirTimings {
-    pub rebuild_bank_from_snapshot_us: u64,
-    pub build_storage_us: u64,
-}
-
 // From testing, 4 seems to be a sweet spot for ranges of 60M-360M accounts and 16-64 cores. This may need to be tuned later.
 const PARALLEL_UNTAR_READERS_DEFAULT: usize = 4;

@ -1461,9 +1447,11 @@ fn streaming_snapshot_dir_files(
    Ok(())
 }

-/// Perform the common tasks when deserialize a snapshot.  Handles reading snapshot file, reading the version file,
-/// and then returning those fields plus the rebuilt storage
-pub fn build_storage_from_snapshot_dir(
+/// Performs the common tasks when deserializing a snapshot
+///
+/// Handles reading the snapshot file and version file,
+/// then returning those fields plus the rebuilt storages.
+pub fn rebuild_storages_from_snapshot_dir(
    snapshot_info: &BankSnapshotInfo,
    account_paths: &[PathBuf],
    next_append_vec_id: Arc<AtomicAppendVecId>,