Removes holding storages in AccountsHashVerifier for fastboot (anza-x…

…yz#120)
codebender828 · Mar 16, 2024 · 096a1f4 · 096a1f4
1 parent 27e51b3
commit 096a1f4
Show file tree

Hide file tree

Showing 8 changed files with 52 additions and 172 deletions.
diff --git a/accounts-db/src/lib.rs b/accounts-db/src/lib.rs
@@ -37,7 +37,6 @@ pub mod secondary_index;
 pub mod shared_buffer_reader;
 pub mod sorted_storages;
 pub mod stake_rewards;
-pub mod starting_snapshot_storages;
 pub mod storable_accounts;
 pub mod tiered_storage;
 pub mod utils;

diff --git a/accounts-db/src/starting_snapshot_storages.rs b/accounts-db/src/starting_snapshot_storages.rs
diff --git a/core/src/accounts_hash_verifier.rs b/core/src/accounts_hash_verifier.rs
@@ -9,7 +9,6 @@ use {
             IncrementalAccountsHash,
         },
         sorted_storages::SortedStorages,
-        starting_snapshot_storages::StartingSnapshotStorages,
     },
     solana_measure::measure_us,
     solana_runtime::{
@@ -43,7 +42,6 @@ impl AccountsHashVerifier {
         accounts_package_sender: Sender<AccountsPackage>,
         accounts_package_receiver: Receiver<AccountsPackage>,
         snapshot_package_sender: Option<Sender<SnapshotPackage>>,
-        starting_snapshot_storages: StartingSnapshotStorages,
         exit: Arc<AtomicBool>,
         snapshot_config: SnapshotConfig,
     ) -> Self {
@@ -53,14 +51,6 @@ impl AccountsHashVerifier {
             .name("solAcctHashVer".to_string())
             .spawn(move || {
                 info!("AccountsHashVerifier has started");
-                // To support fastboot, we must ensure the storages used in the latest POST snapshot are
-                // not recycled nor removed early.  Hold an Arc of their AppendVecs to prevent them from
-                // expiring.
-                let mut fastboot_storages = match starting_snapshot_storages {
-                    StartingSnapshotStorages::Genesis => None,
-                    StartingSnapshotStorages::Archive => None,
-                    StartingSnapshotStorages::Fastboot(storages) => Some(storages),
-                };
                 loop {
                     if exit.load(Ordering::Relaxed) {
                         break;
@@ -81,40 +71,13 @@ impl AccountsHashVerifier {
                     info!("handling accounts package: {accounts_package:?}");
                     let enqueued_time = accounts_package.enqueued.elapsed();
 
-                    // If this accounts package is for a snapshot, then clone the storages to
-                    // save for fastboot.
-                    let snapshot_storages_for_fastboot = accounts_package
-                        .snapshot_info
-                        .is_some()
-                        .then(|| accounts_package.snapshot_storages.clone());
-
-                    let slot = accounts_package.slot;
                     let (_, handling_time_us) = measure_us!(Self::process_accounts_package(
                         accounts_package,
                         snapshot_package_sender.as_ref(),
                         &snapshot_config,
                         &exit,
                     ));
 
-                    if let Some(snapshot_storages_for_fastboot) = snapshot_storages_for_fastboot {
-                        // Get the number of storages that are being kept alive for fastboot.
-                        // Looking at the storage Arc's strong reference count, we know that one
-                        // ref is for fastboot, and one ref is for snapshot packaging.  If there
-                        // are no others, then the storage will be kept alive because of fastboot.
-                        let num_storages_kept_alive = snapshot_storages_for_fastboot
-                            .iter()
-                            .filter(|storage| Arc::strong_count(storage) == 2)
-                            .count();
-                        let num_storages_total = snapshot_storages_for_fastboot.len();
-                        fastboot_storages = Some(snapshot_storages_for_fastboot);
-                        datapoint_info!(
-                            "fastboot",
-                            ("slot", slot, i64),
-                            ("num_storages_total", num_storages_total, i64),
-                            ("num_storages_kept_alive", num_storages_kept_alive, i64),
-                        );
-                    }
-
                     datapoint_info!(
                         "accounts_hash_verifier",
                         (
@@ -132,13 +95,6 @@ impl AccountsHashVerifier {
                     );
                 }
                 info!("AccountsHashVerifier has stopped");
-                debug!(
-                    "Number of storages kept alive for fastboot: {}",
-                    fastboot_storages
-                        .as_ref()
-                        .map(|storages| storages.len())
-                        .unwrap_or(0)
-                );
             })
             .unwrap();
         Self {

diff --git a/core/src/validator.rs b/core/src/validator.rs
@@ -35,7 +35,6 @@ use {
         accounts_index::AccountSecondaryIndexes,
         accounts_update_notifier_interface::AccountsUpdateNotifier,
         hardened_unpack::{open_genesis_config, MAX_GENESIS_ARCHIVE_UNPACKED_SIZE},
-        starting_snapshot_storages::StartingSnapshotStorages,
         utils::{move_and_async_delete_path, move_and_async_delete_path_contents},
     },
     solana_client::connection_cache::{ConnectionCache, Protocol},
@@ -691,7 +690,6 @@ impl Validator {
             completed_slots_receiver,
             leader_schedule_cache,
             starting_snapshot_hashes,
-            starting_snapshot_storages,
             TransactionHistoryServices {
                 transaction_status_sender,
                 transaction_status_service,
@@ -781,7 +779,6 @@ impl Validator {
             accounts_package_sender.clone(),
             accounts_package_receiver,
             snapshot_package_sender,
-            starting_snapshot_storages,
             exit.clone(),
             config.snapshot_config.clone(),
         );
@@ -1770,7 +1767,6 @@ fn load_blockstore(
         CompletedSlotsReceiver,
         LeaderScheduleCache,
         Option<StartingSnapshotHashes>,
-        StartingSnapshotStorages,
         TransactionHistoryServices,
         blockstore_processor::ProcessOptions,
         BlockstoreRootScan,
@@ -1860,27 +1856,23 @@ fn load_blockstore(
     let entry_notifier_service = entry_notifier
         .map(|entry_notifier| EntryNotifierService::new(entry_notifier, exit.clone()));
 
-    let (
-        bank_forks,
-        mut leader_schedule_cache,
-        starting_snapshot_hashes,
-        starting_snapshot_storages,
-    ) = bank_forks_utils::load_bank_forks(
-        &genesis_config,
-        &blockstore,
-        config.account_paths.clone(),
-        Some(&config.snapshot_config),
-        &process_options,
-        transaction_history_services
-            .cache_block_meta_sender
-            .as_ref(),
-        entry_notifier_service
-            .as_ref()
-            .map(|service| service.sender()),
-        accounts_update_notifier,
-        exit,
-    )
-    .map_err(|err| err.to_string())?;
+    let (bank_forks, mut leader_schedule_cache, starting_snapshot_hashes) =
+        bank_forks_utils::load_bank_forks(
+            &genesis_config,
+            &blockstore,
+            config.account_paths.clone(),
+            Some(&config.snapshot_config),
+            &process_options,
+            transaction_history_services
+                .cache_block_meta_sender
+                .as_ref(),
+            entry_notifier_service
+                .as_ref()
+                .map(|service| service.sender()),
+            accounts_update_notifier,
+            exit,
+        )
+        .map_err(|err| err.to_string())?;
 
     // Before replay starts, set the callbacks in each of the banks in BankForks so that
     // all dropped banks come through the `pruned_banks_receiver` channel. This way all bank
@@ -1906,7 +1898,6 @@ fn load_blockstore(
         completed_slots_receiver,
         leader_schedule_cache,
         starting_snapshot_hashes,
-        starting_snapshot_storages,
         transaction_history_services,
         process_options,
         blockstore_root_scan,

diff --git a/core/tests/epoch_accounts_hash.rs b/core/tests/epoch_accounts_hash.rs
@@ -9,7 +9,6 @@ use {
         accounts_hash::CalcAccountsHashConfig,
         accounts_index::AccountSecondaryIndexes,
         epoch_accounts_hash::EpochAccountsHash,
-        starting_snapshot_storages::StartingSnapshotStorages,
     },
     solana_core::{
         accounts_hash_verifier::AccountsHashVerifier,
@@ -197,7 +196,6 @@ impl BackgroundServices {
             accounts_package_sender.clone(),
             accounts_package_receiver,
             Some(snapshot_package_sender),
-            StartingSnapshotStorages::Genesis,
             exit.clone(),
             snapshot_config.clone(),
         );

diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs
@@ -11,7 +11,6 @@ use {
         accounts_hash::AccountsHash,
         accounts_index::AccountSecondaryIndexes,
         epoch_accounts_hash::EpochAccountsHash,
-        starting_snapshot_storages::StartingSnapshotStorages,
     },
     solana_core::{
         accounts_hash_verifier::AccountsHashVerifier,
@@ -1044,7 +1043,6 @@ fn test_snapshots_with_background_services(
         accounts_package_sender,
         accounts_package_receiver,
         Some(snapshot_package_sender),
-        StartingSnapshotStorages::Genesis,
         exit.clone(),
         snapshot_test_config.snapshot_config.clone(),
     );

diff --git a/ledger-tool/src/ledger_utils.rs b/ledger-tool/src/ledger_utils.rs
@@ -268,24 +268,19 @@ pub fn load_and_process_ledger(
     };
 
     let exit = Arc::new(AtomicBool::new(false));
-    let (
-        bank_forks,
-        leader_schedule_cache,
-        starting_snapshot_hashes,
-        starting_snapshot_storages,
-        ..,
-    ) = bank_forks_utils::load_bank_forks(
-        genesis_config,
-        blockstore.as_ref(),
-        account_paths,
-        snapshot_config.as_ref(),
-        &process_options,
-        None,
-        None, // Maybe support this later, though
-        accounts_update_notifier,
-        exit.clone(),
-    )
-    .map_err(LoadAndProcessLedgerError::LoadBankForks)?;
+    let (bank_forks, leader_schedule_cache, starting_snapshot_hashes, ..) =
+        bank_forks_utils::load_bank_forks(
+            genesis_config,
+            blockstore.as_ref(),
+            account_paths,
+            snapshot_config.as_ref(),
+            &process_options,
+            None,
+            None, // Maybe support this later, though
+            accounts_update_notifier,
+            exit.clone(),
+        )
+        .map_err(LoadAndProcessLedgerError::LoadBankForks)?;
     let block_verification_method = value_t!(
         arg_matches,
         "block_verification_method",
@@ -330,7 +325,6 @@ pub fn load_and_process_ledger(
         accounts_package_sender.clone(),
         accounts_package_receiver,
         None,
-        starting_snapshot_storages,
         exit.clone(),
         SnapshotConfig::new_load_only(),
     );