From 96dd6214269c57fe2c1135f5f32ea09c8c2dcef1 Mon Sep 17 00:00:00 2001 From: Brennan Date: Wed, 22 Feb 2023 12:15:17 -0800 Subject: [PATCH] Remove ignored slots from repair (#30438) --- core/src/repair_service.rs | 25 ++----- core/src/repair_weight.rs | 10 +-- core/src/repair_weighted_traversal.rs | 98 +++------------------------ 3 files changed, 18 insertions(+), 115 deletions(-) diff --git a/core/src/repair_service.rs b/core/src/repair_service.rs index 6eeedeb83b..117b5eab69 100644 --- a/core/src/repair_service.rs +++ b/core/src/repair_service.rs @@ -1,11 +1,15 @@ //! The `repair_service` module implements the tools necessary to generate a thread which //! regularly finds missing shreds in the ledger and sends repair requests for those shreds +#[cfg(test)] +use { + crate::duplicate_repair_status::DuplicateSlotRepairStatus, solana_ledger::shred::Nonce, + solana_sdk::clock::DEFAULT_MS_PER_SLOT, +}; use { crate::{ ancestor_hashes_service::{AncestorHashesReplayUpdateReceiver, AncestorHashesService}, cluster_info_vote_listener::VerifiedVoteReceiver, cluster_slots::ClusterSlots, - duplicate_repair_status::DuplicateSlotRepairStatus, outstanding_requests::OutstandingRequests, repair_weight::RepairWeight, serve_repair::{ServeRepair, ShredRepairType, REPAIR_PEERS_CACHE_CAPACITY}, @@ -18,7 +22,7 @@ use { shred, }, solana_measure::measure::Measure, - solana_runtime::{bank_forks::BankForks, contains::Contains}, + solana_runtime::bank_forks::BankForks, solana_sdk::{ clock::{Slot, DEFAULT_TICKS_PER_SECOND, MS_PER_TICK}, epoch_schedule::EpochSchedule, @@ -40,8 +44,6 @@ use { time::{Duration, Instant}, }, }; -#[cfg(test)] -use {solana_ledger::shred::Nonce, solana_sdk::clock::DEFAULT_MS_PER_SLOT}; // Time to defer repair requests to allow for turbine propagation const DEFER_REPAIR_THRESHOLD: Duration = Duration::from_millis(200); @@ -280,8 +282,6 @@ impl RepairService { let mut repair_timing = RepairTiming::default(); let mut best_repairs_stats = BestRepairsStats::default(); let mut last_stats = Instant::now(); - let duplicate_slot_repair_statuses: HashMap = - HashMap::new(); let mut peers_cache = LruCache::new(REPAIR_PEERS_CACHE_CAPACITY); loop { @@ -352,7 +352,6 @@ impl RepairService { MAX_REPAIR_LENGTH, MAX_UNKNOWN_LAST_INDEX_REPAIRS, MAX_CLOSEST_COMPLETION_REPAIRS, - &duplicate_slot_repair_statuses, &mut repair_timing, &mut best_repairs_stats, ); @@ -569,20 +568,15 @@ impl RepairService { } /// Repairs any fork starting at the input slot (uses blockstore for fork info) - pub fn generate_repairs_for_fork<'a>( + pub fn generate_repairs_for_fork( blockstore: &Blockstore, repairs: &mut Vec, max_repairs: usize, slot: Slot, - duplicate_slot_repair_statuses: &impl Contains<'a, Slot>, ) { let mut pending_slots = vec![slot]; while repairs.len() < max_repairs && !pending_slots.is_empty() { let slot = pending_slots.pop().unwrap(); - if duplicate_slot_repair_statuses.contains(&slot) { - // These are repaired through a different path - continue; - } if let Some(slot_meta) = blockstore.meta(slot).unwrap() { let new_repairs = Self::generate_repairs_for_slot( blockstore, @@ -844,7 +838,6 @@ mod test { MAX_REPAIR_LENGTH, MAX_UNKNOWN_LAST_INDEX_REPAIRS, MAX_CLOSEST_COMPLETION_REPAIRS, - &HashSet::default(), &mut RepairTiming::default(), &mut BestRepairsStats::default(), ), @@ -881,7 +874,6 @@ mod test { MAX_REPAIR_LENGTH, MAX_UNKNOWN_LAST_INDEX_REPAIRS, MAX_CLOSEST_COMPLETION_REPAIRS, - &HashSet::default(), &mut RepairTiming::default(), &mut BestRepairsStats::default(), ), @@ -942,7 +934,6 @@ mod test { MAX_REPAIR_LENGTH, MAX_UNKNOWN_LAST_INDEX_REPAIRS, MAX_CLOSEST_COMPLETION_REPAIRS, - &HashSet::default(), &mut RepairTiming::default(), &mut BestRepairsStats::default(), ), @@ -958,7 +949,6 @@ mod test { expected.len() - 2, MAX_UNKNOWN_LAST_INDEX_REPAIRS, MAX_CLOSEST_COMPLETION_REPAIRS, - &HashSet::default(), &mut RepairTiming::default(), &mut BestRepairsStats::default(), )[..], @@ -1005,7 +995,6 @@ mod test { MAX_REPAIR_LENGTH, MAX_UNKNOWN_LAST_INDEX_REPAIRS, MAX_CLOSEST_COMPLETION_REPAIRS, - &HashSet::default(), &mut RepairTiming::default(), &mut BestRepairsStats::default(), ), diff --git a/core/src/repair_weight.rs b/core/src/repair_weight.rs index 294e96e5fe..2fe7f912ab 100644 --- a/core/src/repair_weight.rs +++ b/core/src/repair_weight.rs @@ -11,7 +11,7 @@ use { ancestor_iterator::AncestorIterator, blockstore::Blockstore, blockstore_meta::SlotMeta, }, solana_measure::measure::Measure, - solana_runtime::{contains::Contains, epoch_stakes::EpochStakes}, + solana_runtime::epoch_stakes::EpochStakes, solana_sdk::{ clock::Slot, epoch_schedule::{Epoch, EpochSchedule}, @@ -147,7 +147,7 @@ impl RepairWeight { } #[allow(clippy::too_many_arguments)] - pub fn get_best_weighted_repairs<'a>( + pub fn get_best_weighted_repairs( &mut self, blockstore: &Blockstore, epoch_stakes: &HashMap, @@ -156,7 +156,6 @@ impl RepairWeight { max_new_shreds: usize, max_unknown_last_index_repairs: usize, max_closest_completion_repairs: usize, - ignore_slots: &impl Contains<'a, Slot>, repair_timing: &mut RepairTiming, stats: &mut BestRepairsStats, ) -> Vec { @@ -186,7 +185,6 @@ impl RepairWeight { &mut slot_meta_cache, &mut best_shreds_repairs, max_new_shreds, - ignore_slots, ); let num_best_shreds_repairs = best_shreds_repairs.len(); let repair_slots_set: HashSet = @@ -343,13 +341,12 @@ impl RepairWeight { } // Generate shred repairs for main subtree rooted at `self.root` - fn get_best_shreds<'a>( + fn get_best_shreds( &mut self, blockstore: &Blockstore, slot_meta_cache: &mut HashMap>, repairs: &mut Vec, max_new_shreds: usize, - ignore_slots: &impl Contains<'a, Slot>, ) { let root_tree = self.trees.get(&self.root).expect("Root tree must exist"); repair_weighted_traversal::get_best_repair_shreds( @@ -358,7 +355,6 @@ impl RepairWeight { slot_meta_cache, repairs, max_new_shreds, - ignore_slots, ); } diff --git a/core/src/repair_weighted_traversal.rs b/core/src/repair_weighted_traversal.rs index 7cc681bbcc..dc3dbea4dc 100644 --- a/core/src/repair_weighted_traversal.rs +++ b/core/src/repair_weighted_traversal.rs @@ -4,7 +4,6 @@ use { serve_repair::ShredRepairType, tree_diff::TreeDiff, }, solana_ledger::{blockstore::Blockstore, blockstore_meta::SlotMeta}, - solana_runtime::contains::Contains, solana_sdk::{clock::Slot, hash::Hash}, std::collections::{HashMap, HashSet}, }; @@ -73,13 +72,12 @@ impl<'a> Iterator for RepairWeightTraversal<'a> { /// Generate shred repairs for `tree` starting at `tree.root`. /// Prioritized by stake weight, additionally considers children not present in `tree` but in /// blockstore. -pub fn get_best_repair_shreds<'a>( +pub fn get_best_repair_shreds( tree: &HeaviestSubtreeForkChoice, blockstore: &Blockstore, slot_meta_cache: &mut HashMap>, repairs: &mut Vec, max_new_shreds: usize, - ignore_slots: &impl Contains<'a, Slot>, ) { let initial_len = repairs.len(); let max_repairs = initial_len + max_new_shreds; @@ -100,15 +98,13 @@ pub fn get_best_repair_shreds<'a>( if let Some(slot_meta) = slot_meta { match next { Visit::Unvisited(slot) => { - if !ignore_slots.contains(&slot) { - let new_repairs = RepairService::generate_repairs_for_slot( - blockstore, - slot, - slot_meta, - max_repairs - repairs.len(), - ); - repairs.extend(new_repairs); - } + let new_repairs = RepairService::generate_repairs_for_slot( + blockstore, + slot, + slot_meta, + max_repairs - repairs.len(), + ); + repairs.extend(new_repairs); visited_set.insert(slot); } Visit::Visited(_) => { @@ -126,7 +122,6 @@ pub fn get_best_repair_shreds<'a>( repairs, max_repairs, *new_child_slot, - ignore_slots, ); } visited_set.insert(*new_child_slot); @@ -234,7 +229,6 @@ pub mod test { &mut slot_meta_cache, &mut repairs, 6, - &HashSet::default(), ); assert_eq!( repairs, @@ -264,7 +258,6 @@ pub mod test { &mut slot_meta_cache, &mut repairs, 6, - &HashSet::default(), ); assert_eq!( repairs, @@ -305,7 +298,6 @@ pub mod test { &mut slot_meta_cache, &mut repairs, 4, - &HashSet::default(), ); assert_eq!( repairs, @@ -327,7 +319,6 @@ pub mod test { &mut slot_meta_cache, &mut repairs, 4, - &HashSet::default(), ); assert_eq!( repairs, @@ -354,7 +345,6 @@ pub mod test { &mut slot_meta_cache, &mut repairs, std::usize::MAX, - &HashSet::default(), ); let last_shred = blockstore.meta(0).unwrap().unwrap().received; assert_eq!( @@ -366,78 +356,6 @@ pub mod test { ); } - #[test] - fn test_get_best_repair_shreds_ignore() { - let (blockstore, heaviest_subtree_fork_choice) = setup_forks(); - sleep_shred_deferment_period(); - - // Adding slots to ignore should remove them from the repair set, but - // should not remove their children - let mut repairs = vec![]; - let mut slot_meta_cache = HashMap::default(); - let mut ignore_set: HashSet = vec![1, 3].into_iter().collect(); - let last_shred = blockstore.meta(0).unwrap().unwrap().received; - get_best_repair_shreds( - &heaviest_subtree_fork_choice, - &blockstore, - &mut slot_meta_cache, - &mut repairs, - std::usize::MAX, - &ignore_set, - ); - assert_eq!( - repairs, - [0, 2, 4, 5] - .iter() - .map(|slot| ShredRepairType::HighestShred(*slot, last_shred)) - .collect::>() - ); - - // Adding slot 2 to ignore should not remove its unexplored children from - // the repair set - repairs = vec![]; - slot_meta_cache = HashMap::default(); - blockstore.add_tree(tr(2) / (tr(6) / tr(7)), true, false, 2, Hash::default()); - ignore_set.insert(2); - sleep_shred_deferment_period(); - get_best_repair_shreds( - &heaviest_subtree_fork_choice, - &blockstore, - &mut slot_meta_cache, - &mut repairs, - std::usize::MAX, - &ignore_set, - ); - assert_eq!( - repairs, - [0, 4, 6, 7, 5] - .iter() - .map(|slot| ShredRepairType::HighestShred(*slot, last_shred)) - .collect::>() - ); - - // Adding unexplored child 6 to ignore set should remove it and it's - // child 7 from the repair set - repairs = vec![]; - ignore_set.insert(6); - slot_meta_cache = HashMap::default(); - get_best_repair_shreds( - &heaviest_subtree_fork_choice, - &blockstore, - &mut slot_meta_cache, - &mut repairs, - std::usize::MAX, - &ignore_set, - ); - assert_eq!( - repairs, - [0, 4, 5] - .iter() - .map(|slot| ShredRepairType::HighestShred(*slot, last_shred)) - .collect::>() - ); - } - fn setup_forks() -> (Blockstore, HeaviestSubtreeForkChoice) { /* Build fork structure: