Skip to content

Commit

Permalink
Remove send snapshot hard unwrap (#326)
Browse files Browse the repository at this point in the history
  • Loading branch information
carllin committed Apr 11, 2024
1 parent f6cac1e commit d5c291a
Show file tree
Hide file tree
Showing 14 changed files with 219 additions and 124 deletions.
31 changes: 20 additions & 11 deletions core/src/commitment_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,8 @@ mod tests {
bank_forks
.write()
.unwrap()
.set_root(x, &AbsRequestSender::default(), None);
.set_root(x, &AbsRequestSender::default(), None)
.unwrap();
}

// Add an additional bank/vote that will root slot 2
Expand Down Expand Up @@ -596,11 +597,15 @@ mod tests {
.read()
.unwrap()
.highest_super_majority_root();
bank_forks.write().unwrap().set_root(
root,
&AbsRequestSender::default(),
Some(highest_super_majority_root),
);
bank_forks
.write()
.unwrap()
.set_root(
root,
&AbsRequestSender::default(),
Some(highest_super_majority_root),
)
.unwrap();
let highest_super_majority_root_bank =
bank_forks.read().unwrap().get(highest_super_majority_root);
assert!(highest_super_majority_root_bank.is_some());
Expand Down Expand Up @@ -675,11 +680,15 @@ mod tests {
.read()
.unwrap()
.highest_super_majority_root();
bank_forks.write().unwrap().set_root(
root,
&AbsRequestSender::default(),
Some(highest_super_majority_root),
);
bank_forks
.write()
.unwrap()
.set_root(
root,
&AbsRequestSender::default(),
Some(highest_super_majority_root),
)
.unwrap();
let highest_super_majority_root_bank =
bank_forks.read().unwrap().get(highest_super_majority_root);
assert!(highest_super_majority_root_bank.is_some());
Expand Down
4 changes: 3 additions & 1 deletion core/src/repair/ancestor_hashes_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1908,7 +1908,9 @@ mod test {
{
let mut w_bank_forks = bank_forks.write().unwrap();
w_bank_forks.insert(new_root_bank);
w_bank_forks.set_root(new_root_slot, &AbsRequestSender::default(), None);
w_bank_forks
.set_root(new_root_slot, &AbsRequestSender::default(), None)
.unwrap();
}
popular_pruned_slot_pool.insert(dead_duplicate_confirmed_slot);
assert!(!dead_slot_pool.is_empty());
Expand Down
40 changes: 27 additions & 13 deletions core/src/replay_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ use {
solana_runtime::{
accounts_background_service::AbsRequestSender,
bank::{bank_hash_details, Bank, NewBankOptions},
bank_forks::{BankForks, MAX_ROOT_DISTANCE_FOR_VOTE_ONLY},
bank_forks::{BankForks, SetRootError, MAX_ROOT_DISTANCE_FOR_VOTE_ONLY},
commitment::BlockCommitmentCache,
installed_scheduler_pool::BankWithScheduler,
prioritization_fee_cache::PrioritizationFeeCache,
Expand Down Expand Up @@ -980,7 +980,7 @@ impl ReplayStage {
);
}

Self::handle_votable_bank(
if let Err(e) = Self::handle_votable_bank(
vote_bank,
switch_fork_decision,
&bank_forks,
Expand All @@ -1007,7 +1007,10 @@ impl ReplayStage {
&mut epoch_slots_frozen_slots,
&drop_bank_sender,
wait_to_vote_slot,
);
) {
error!("Unable to set root: {e}");
return;
}
}
voting_time.stop();

Expand Down Expand Up @@ -2306,7 +2309,7 @@ impl ReplayStage {
epoch_slots_frozen_slots: &mut EpochSlotsFrozenSlots,
drop_bank_sender: &Sender<Vec<Arc<Bank>>>,
wait_to_vote_slot: Option<Slot>,
) {
) -> Result<(), SetRootError> {
if bank.is_empty() {
datapoint_info!("replay_stage-voted_empty_bank", ("slot", bank.slot(), i64));
}
Expand Down Expand Up @@ -2362,7 +2365,7 @@ impl ReplayStage {
vote_signatures,
epoch_slots_frozen_slots,
drop_bank_sender,
);
)?;

blockstore.slots_stats.mark_rooted(new_root);

Expand Down Expand Up @@ -2406,6 +2409,7 @@ impl ReplayStage {
voting_sender,
wait_to_vote_slot,
);
Ok(())
}

fn generate_vote_tx(
Expand Down Expand Up @@ -4136,13 +4140,13 @@ impl ReplayStage {
voted_signatures: &mut Vec<Signature>,
epoch_slots_frozen_slots: &mut EpochSlotsFrozenSlots,
drop_bank_sender: &Sender<Vec<Arc<Bank>>>,
) {
) -> Result<(), SetRootError> {
bank_forks.read().unwrap().prune_program_cache(new_root);
let removed_banks = bank_forks.write().unwrap().set_root(
new_root,
accounts_background_request_sender,
highest_super_majority_root,
);
)?;

drop_bank_sender
.send(removed_banks)
Expand Down Expand Up @@ -4173,6 +4177,7 @@ impl ReplayStage {

unfrozen_gossip_verified_vote_hashes.set_root(new_root);
*epoch_slots_frozen_slots = epoch_slots_frozen_slots.split_off(&new_root);
Ok(())
// epoch_slots_frozen_slots now only contains entries >= `new_root`
}

Expand Down Expand Up @@ -4694,7 +4699,8 @@ pub(crate) mod tests {
&mut Vec::new(),
&mut epoch_slots_frozen_slots,
&drop_bank_sender,
);
)
.unwrap();
assert_eq!(bank_forks.read().unwrap().root(), root);
assert_eq!(progress.len(), 1);
assert!(progress.get(&root).is_some());
Expand Down Expand Up @@ -4772,7 +4778,8 @@ pub(crate) mod tests {
&mut Vec::new(),
&mut EpochSlotsFrozenSlots::default(),
&drop_bank_sender,
);
)
.unwrap();
assert_eq!(bank_forks.read().unwrap().root(), root);
assert!(bank_forks.read().unwrap().get(confirmed_root).is_some());
assert!(bank_forks.read().unwrap().get(fork).is_none());
Expand Down Expand Up @@ -5797,7 +5804,9 @@ pub(crate) mod tests {
bank_forks.insert(Bank::new_from_parent(bank0.clone(), &Pubkey::default(), 9));
let bank9 = bank_forks.get(9).unwrap();
bank_forks.insert(Bank::new_from_parent(bank9, &Pubkey::default(), 10));
bank_forks.set_root(9, &AbsRequestSender::default(), None);
bank_forks
.set_root(9, &AbsRequestSender::default(), None)
.unwrap();
let total_epoch_stake = bank0.total_epoch_stake();

// Insert new ForkProgress for slot 10 and its
Expand Down Expand Up @@ -5891,7 +5900,9 @@ pub(crate) mod tests {
.get_propagated_stats_mut(0)
.unwrap()
.is_leader_slot = true;
bank_forks.set_root(0, &AbsRequestSender::default(), None);
bank_forks
.set_root(0, &AbsRequestSender::default(), None)
.unwrap();
let total_epoch_stake = bank_forks.root_bank().total_epoch_stake();

// Insert new ForkProgress representing a slot for all slots 1..=num_banks. Only
Expand Down Expand Up @@ -5974,7 +5985,9 @@ pub(crate) mod tests {
.get_propagated_stats_mut(0)
.unwrap()
.is_leader_slot = true;
bank_forks.set_root(0, &AbsRequestSender::default(), None);
bank_forks
.set_root(0, &AbsRequestSender::default(), None)
.unwrap();

let total_epoch_stake = num_validators as u64 * stake_per_validator;

Expand Down Expand Up @@ -6596,7 +6609,8 @@ pub(crate) mod tests {
bank_forks
.write()
.unwrap()
.set_root(3, &AbsRequestSender::default(), None);
.set_root(3, &AbsRequestSender::default(), None)
.unwrap();
let mut descendants = bank_forks.read().unwrap().descendants();
let mut ancestors = bank_forks.read().unwrap().ancestors();
let slot_3_descendants = descendants.get(&3).unwrap().clone();
Expand Down
12 changes: 7 additions & 5 deletions core/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2097,11 +2097,13 @@ fn maybe_warp_slot(
warp_slot,
solana_accounts_db::accounts_db::CalcAccountsHashDataSource::Storages,
));
bank_forks.set_root(
warp_slot,
accounts_background_request_sender,
Some(warp_slot),
);
bank_forks
.set_root(
warp_slot,
accounts_background_request_sender,
Some(warp_slot),
)
.map_err(|err| err.to_string())?;
leader_schedule_cache.set_root(&bank_forks.root_bank());

let full_snapshot_archive_info = match snapshot_bank_utils::bank_to_full_snapshot_archive(
Expand Down
1 change: 1 addition & 0 deletions core/src/vote_simulator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ impl VoteSimulator {
&mut EpochSlotsFrozenSlots::default(),
&drop_bank_sender,
)
.unwrap()
}

pub fn create_and_vote_new_branch(
Expand Down
108 changes: 66 additions & 42 deletions core/tests/epoch_accounts_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,13 +295,17 @@ fn test_epoch_accounts_hash_basic(test_environment: TestEnvironment) {
if bank.slot().checked_rem(SET_ROOT_INTERVAL).unwrap() == 0 {
trace!("rooting bank {}", bank.slot());
bank_forks.read().unwrap().prune_program_cache(bank.slot());
bank_forks.write().unwrap().set_root(
bank.slot(),
&test_environment
.background_services
.accounts_background_request_sender,
None,
);
bank_forks
.write()
.unwrap()
.set_root(
bank.slot(),
&test_environment
.background_services
.accounts_background_request_sender,
None,
)
.unwrap();
}

// To ensure EAH calculations are correct, calculate the accounts hash here, in-band.
Expand Down Expand Up @@ -407,13 +411,17 @@ fn test_snapshots_have_expected_epoch_accounts_hash() {
// Root every bank. This is what a normal validator does as well.
// `set_root()` is also what requests snapshots and EAH calculations.
bank_forks.read().unwrap().prune_program_cache(bank.slot());
bank_forks.write().unwrap().set_root(
bank.slot(),
&test_environment
.background_services
.accounts_background_request_sender,
None,
);
bank_forks
.write()
.unwrap()
.set_root(
bank.slot(),
&test_environment
.background_services
.accounts_background_request_sender,
None,
)
.unwrap();

// After submitting an EAH calculation request, wait until it gets handled by ABS so that
// subsequent snapshot requests are not swallowed.
Expand Down Expand Up @@ -531,13 +539,17 @@ fn test_background_services_request_handling_for_epoch_accounts_hash() {
if bank.block_height() == set_root_slot {
info!("Calling set_root() on bank {}...", bank.slot());
bank_forks.read().unwrap().prune_program_cache(bank.slot());
bank_forks.write().unwrap().set_root(
bank.slot(),
&test_environment
.background_services
.accounts_background_request_sender,
None,
);
bank_forks
.write()
.unwrap()
.set_root(
bank.slot(),
&test_environment
.background_services
.accounts_background_request_sender,
None,
)
.unwrap();
info!("Calling set_root() on bank {}... DONE", bank.slot());

// wait until eah is valid
Expand Down Expand Up @@ -588,13 +600,17 @@ fn test_epoch_accounts_hash_and_warping() {
epoch_schedule.get_first_slot_in_epoch(bank.epoch() + 1) + eah_stop_offset;
// have to set root here so that we can flush the write cache
bank_forks.read().unwrap().prune_program_cache(bank.slot());
bank_forks.write().unwrap().set_root(
bank.slot(),
&test_environment
.background_services
.accounts_background_request_sender,
None,
);
bank_forks
.write()
.unwrap()
.set_root(
bank.slot(),
&test_environment
.background_services
.accounts_background_request_sender,
None,
)
.unwrap();
// flush the write cache so warping can calculate the accounts hash from storages
bank.force_flush_accounts_cache();
let bank = bank_forks
Expand All @@ -614,13 +630,17 @@ fn test_epoch_accounts_hash_and_warping() {
.insert(Bank::new_from_parent(bank, &Pubkey::default(), slot))
.clone_without_scheduler();
bank_forks.read().unwrap().prune_program_cache(bank.slot());
bank_forks.write().unwrap().set_root(
bank.slot(),
&test_environment
.background_services
.accounts_background_request_sender,
None,
);
bank_forks
.write()
.unwrap()
.set_root(
bank.slot(),
&test_environment
.background_services
.accounts_background_request_sender,
None,
)
.unwrap();
info!("Waiting for epoch accounts hash...");
_ = bank
.rc
Expand Down Expand Up @@ -654,13 +674,17 @@ fn test_epoch_accounts_hash_and_warping() {
.insert(Bank::new_from_parent(bank, &Pubkey::default(), slot))
.clone_without_scheduler();
bank_forks.read().unwrap().prune_program_cache(bank.slot());
bank_forks.write().unwrap().set_root(
bank.slot(),
&test_environment
.background_services
.accounts_background_request_sender,
None,
);
bank_forks
.write()
.unwrap()
.set_root(
bank.slot(),
&test_environment
.background_services
.accounts_background_request_sender,
None,
)
.unwrap();
info!("Waiting for epoch accounts hash...");
_ = bank
.rc
Expand Down
Loading

0 comments on commit d5c291a

Please sign in to comment.