From b340b534b9ffa0299dfac7a08dad601edd2d691c Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Wed, 4 Jan 2023 17:06:43 -0800 Subject: [PATCH] Change the latest slot snapshot storage from VecDeque to Option --- core/src/validator.rs | 1 + core/tests/epoch_accounts_hash.rs | 1 + core/tests/snapshots.rs | 21 +++++++---------- ledger-tool/src/main.rs | 1 + runtime/src/accounts_background_service.rs | 27 ++++++++-------------- 5 files changed, 20 insertions(+), 31 deletions(-) diff --git a/core/src/validator.rs b/core/src/validator.rs index 72bda2580956cc..14f6a334e4be41 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -625,6 +625,7 @@ impl Validator { snapshot_request_sender, snapshot_request_receiver, accounts_package_sender, + latest_slot_snapshot_storages: None, }; let pruned_banks_request_handler = PrunedBanksRequestHandler { pruned_banks_receiver, diff --git a/core/tests/epoch_accounts_hash.rs b/core/tests/epoch_accounts_hash.rs index 958c1a02383013..f1771203204688 100755 --- a/core/tests/epoch_accounts_hash.rs +++ b/core/tests/epoch_accounts_hash.rs @@ -210,6 +210,7 @@ impl BackgroundServices { snapshot_request_sender, snapshot_request_receiver, accounts_package_sender, + latest_slot_snapshot_storages: None, }; let pruned_banks_request_handler = PrunedBanksRequestHandler { pruned_banks_receiver, diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index 4c1f81fb9a659d..c069bda426e09a 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -16,7 +16,7 @@ use { AbsRequestHandlers, AbsRequestSender, AccountsBackgroundService, PrunedBanksRequestHandler, SnapshotRequestHandler, }, - accounts_db::{self, SnapshotStorages, ACCOUNTS_DB_CONFIG_FOR_TESTING}, + accounts_db::{self, ACCOUNTS_DB_CONFIG_FOR_TESTING}, accounts_hash::AccountsHash, accounts_index::AccountSecondaryIndexes, bank::{Bank, BankSlotDelta}, @@ -51,7 +51,7 @@ use { }, solana_streamer::socket::SocketAddrSpace, std::{ - collections::{HashSet, VecDeque}, + collections::HashSet, fs, io::{Error, ErrorKind}, path::PathBuf, @@ -234,13 +234,13 @@ fn run_bank_forks_snapshot_n( let (snapshot_request_sender, snapshot_request_receiver) = unbounded(); let request_sender = AbsRequestSender::new(snapshot_request_sender.clone()); - let snapshot_request_handler = SnapshotRequestHandler { + let mut snapshot_request_handler = SnapshotRequestHandler { snapshot_config: snapshot_test_config.snapshot_config.clone(), snapshot_request_sender, snapshot_request_receiver, accounts_package_sender, + latest_slot_snapshot_storages: None, }; - let mut snapshot_slot_storages: VecDeque = VecDeque::new(); for slot in 1..=last_slot { let mut bank = Bank::new_from_parent(&bank_forks[slot - 1], &Pubkey::default(), slot); f(&mut bank, mint_keypair); @@ -252,12 +252,7 @@ fn run_bank_forks_snapshot_n( // set_root should send a snapshot request bank_forks.set_root(bank.slot(), &request_sender, None); bank.update_accounts_hash_for_tests(); - snapshot_request_handler.handle_snapshot_requests( - false, - 0, - &mut None, - &mut snapshot_slot_storages, - ); + snapshot_request_handler.handle_snapshot_requests(false, 0, &mut None); } } @@ -755,15 +750,15 @@ fn test_bank_forks_incremental_snapshot( let (snapshot_request_sender, snapshot_request_receiver) = unbounded(); let request_sender = AbsRequestSender::new(snapshot_request_sender.clone()); - let snapshot_request_handler = SnapshotRequestHandler { + let mut snapshot_request_handler = SnapshotRequestHandler { snapshot_config: snapshot_test_config.snapshot_config.clone(), snapshot_request_sender, snapshot_request_receiver, accounts_package_sender, + latest_slot_snapshot_storages: None, }; let mut last_full_snapshot_slot = None; - let mut snapshot_slot_storages: VecDeque = VecDeque::new(); for slot in 1..=LAST_SLOT { // Make a new bank and perform some transactions let bank = { @@ -795,7 +790,6 @@ fn test_bank_forks_incremental_snapshot( false, 0, &mut last_full_snapshot_slot, - &mut snapshot_slot_storages, ); } @@ -1017,6 +1011,7 @@ fn test_snapshots_with_background_services( snapshot_request_sender, snapshot_request_receiver, accounts_package_sender: accounts_package_sender.clone(), + latest_slot_snapshot_storages: None, }; let pruned_banks_request_handler = PrunedBanksRequestHandler { pruned_banks_receiver, diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 1155b71e962fca..97f035ca0c8533 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -1138,6 +1138,7 @@ fn load_bank_forks( snapshot_request_sender, snapshot_request_receiver, accounts_package_sender, + latest_slot_snapshot_storages: None, }; let pruned_banks_receiver = AccountsBackgroundService::setup_bank_drop_callback(bank_forks.clone()); diff --git a/runtime/src/accounts_background_service.rs b/runtime/src/accounts_background_service.rs index 549800befde829..dcd8d54829e120 100644 --- a/runtime/src/accounts_background_service.rs +++ b/runtime/src/accounts_background_service.rs @@ -2,8 +2,6 @@ //! //! This can be expensive since we have to walk the append vecs being cleaned up. -use std::collections::VecDeque; - mod stats; use { crate::{ @@ -141,16 +139,16 @@ pub struct SnapshotRequestHandler { pub snapshot_request_sender: SnapshotRequestSender, pub snapshot_request_receiver: SnapshotRequestReceiver, pub accounts_package_sender: Sender, + pub latest_slot_snapshot_storages: Option, } impl SnapshotRequestHandler { // Returns the latest requested snapshot slot, if one exists pub fn handle_snapshot_requests( - &self, + &mut self, test_hash_calculation: bool, non_snapshot_time_us: u128, last_full_snapshot_slot: &mut Option, - snapshot_slot_storages: &mut VecDeque, ) -> Option> { let ( snapshot_request, @@ -184,7 +182,6 @@ impl SnapshotRequestHandler { last_full_snapshot_slot, snapshot_request, accounts_package_type, - snapshot_slot_storages, )) } @@ -263,13 +260,12 @@ impl SnapshotRequestHandler { } fn handle_snapshot_request( - &self, + &mut self, test_hash_calculation: bool, non_snapshot_time_us: u128, last_full_snapshot_slot: &mut Option, snapshot_request: SnapshotRequest, accounts_package_type: AccountsPackageType, - snapshot_slot_storages: &mut VecDeque, ) -> Result { debug!( "handling snapshot request: {:?}, {:?}", @@ -379,12 +375,10 @@ impl SnapshotRequestHandler { accounts_hash_for_testing, ) .expect("new accounts package for snapshot"); - snapshot_slot_storages.push_back(snapshot_storages); + // Update the option, so the older one is released, causing the release of + // its reference counts of the appendvecs + let _ret = self.latest_slot_snapshot_storages.insert(snapshot_storages); - // Remove the older ones, causing them to release the reference counts of the appendvecs - while snapshot_slot_storages.len() > 1 { - snapshot_slot_storages.pop_front(); - } accounts_package } SnapshotRequestType::EpochAccountsHash => { @@ -515,17 +509,15 @@ pub struct AbsRequestHandlers { impl AbsRequestHandlers { // Returns the latest requested snapshot block height, if one exists pub fn handle_snapshot_requests( - &self, + &mut self, test_hash_calculation: bool, non_snapshot_time_us: u128, last_full_snapshot_slot: &mut Option, - snapshot_slot_storages: &mut VecDeque, ) -> Option> { self.snapshot_request_handler.handle_snapshot_requests( test_hash_calculation, non_snapshot_time_us, last_full_snapshot_slot, - snapshot_slot_storages, ) } } @@ -538,7 +530,7 @@ impl AccountsBackgroundService { pub fn new( bank_forks: Arc>, exit: &Arc, - request_handlers: AbsRequestHandlers, + mut request_handlers: AbsRequestHandlers, test_hash_calculation: bool, mut last_full_snapshot_slot: Option, ) -> Self { @@ -548,7 +540,6 @@ impl AccountsBackgroundService { let mut removed_slots_count = 0; let mut total_remove_slots_time = 0; let mut last_expiration_check_time = Instant::now(); - let mut snapshot_slot_storages: VecDeque = VecDeque::new(); let t_background = Builder::new() .name("solBgAccounts".to_string()) .spawn(move || { @@ -609,7 +600,6 @@ impl AccountsBackgroundService { test_hash_calculation, non_snapshot_time, &mut last_full_snapshot_slot, - &mut snapshot_slot_storages, ) }) .flatten(); @@ -817,6 +807,7 @@ mod test { snapshot_request_sender: snapshot_request_sender.clone(), snapshot_request_receiver, accounts_package_sender, + latest_slot_snapshot_storages: None, }; let send_snapshot_request = |snapshot_root_bank, request_type| {