updates cluster-slots with root-bank instead of root-slot + bank-fork…

…s (backport #19058) (#19324) * removes unused code from cluster-slots (cherry picked from commit 2fc112e) # Conflicts: # core/src/cluster_slots.rs * updates cluster-slots with root-bank instead of root-slot + bank-forks ClusterSlots::update is taking both root-slot and bank-forks only to later lookup root-bank from bank-forks, which is redundant. Also potentially by the time bank-forks is locked to obtain root-bank, root-slot may have already changed and so be inconsistent with the root-slot passed in as the argument. https://github.com/solana-labs/solana/blob/6d95d679c/core/src/cluster_slots.rs#L32-L39 https://github.com/solana-labs/solana/blob/6d95d679c/core/src/cluster_slots.rs#L122 (cherry picked from commit 40914de) * removes backport merge conflicts Co-authored-by: behzad nouri <[email protected]>
solana-labs · Aug 19, 2021 · f55bb78 · f55bb78
1 parent fa5a71d
commit f55bb78
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 82 deletions.
diff --git a/core/src/cluster_slots.rs b/core/src/cluster_slots.rs
@@ -1,20 +1,21 @@
-use crate::serve_repair::RepairType;
-use itertools::Itertools;
-use solana_gossip::{
-    cluster_info::ClusterInfo, contact_info::ContactInfo, crds::Cursor, epoch_slots::EpochSlots,
-};
-use solana_runtime::{bank_forks::BankForks, epoch_stakes::NodeIdToVoteAccounts};
-use solana_sdk::{clock::Slot, pubkey::Pubkey};
-use std::{
-    collections::{BTreeMap, HashMap, HashSet},
-    sync::{Arc, Mutex, RwLock},
+use {
+    itertools::Itertools,
+    solana_gossip::{
+        cluster_info::ClusterInfo, contact_info::ContactInfo, crds::Cursor, epoch_slots::EpochSlots,
+    },
+    solana_runtime::{bank::Bank, epoch_stakes::NodeIdToVoteAccounts},
+    solana_sdk::{clock::Slot, pubkey::Pubkey},
+    std::{
+        collections::{BTreeMap, HashMap},
+        sync::{Arc, Mutex, RwLock},
+    },
 };
 
 // Limit the size of cluster-slots map in case
 // of receiving bogus epoch slots values.
 const CLUSTER_SLOTS_TRIM_SIZE: usize = 524_288; // 512K
 
-pub type SlotPubkeys = HashMap<Pubkey, u64>;
+pub(crate) type SlotPubkeys = HashMap</*node:*/ Pubkey, /*stake:*/ u64>;
 
 #[derive(Default)]
 pub struct ClusterSlots {
@@ -25,17 +26,17 @@ pub struct ClusterSlots {
 }
 
 impl ClusterSlots {
-    pub fn lookup(&self, slot: Slot) -> Option<Arc<RwLock<SlotPubkeys>>> {
+    pub(crate) fn lookup(&self, slot: Slot) -> Option<Arc<RwLock<SlotPubkeys>>> {
         self.cluster_slots.read().unwrap().get(&slot).cloned()
     }
 
-    pub fn update(&self, root: Slot, cluster_info: &ClusterInfo, bank_forks: &RwLock<BankForks>) {
-        self.update_peers(bank_forks);
+    pub(crate) fn update(&self, root_bank: &Bank, cluster_info: &ClusterInfo) {
+        self.update_peers(root_bank);
         let epoch_slots = {
             let mut cursor = self.cursor.lock().unwrap();
             cluster_info.get_epoch_slots(&mut cursor)
         };
-        self.update_internal(root, epoch_slots);
+        self.update_internal(root_bank.slot(), epoch_slots);
     }
 
     fn update_internal(&self, root: Slot, epoch_slots_list: Vec<EpochSlots>) {
@@ -89,16 +90,6 @@ impl ClusterSlots {
         }
     }
 
-    pub fn collect(&self, id: &Pubkey) -> HashSet<Slot> {
-        self.cluster_slots
-            .read()
-            .unwrap()
-            .iter()
-            .filter(|(_, keys)| keys.read().unwrap().contains_key(id))
-            .map(|(slot, _)| *slot)
-            .collect()
-    }
-
     #[cfg(test)]
     pub(crate) fn insert_node_id(&self, slot: Slot, node_id: Pubkey) {
         let balance = self
@@ -118,8 +109,7 @@ impl ClusterSlots {
         slot_pubkeys.write().unwrap().insert(node_id, balance);
     }
 
-    fn update_peers(&self, bank_forks: &RwLock<BankForks>) {
-        let root_bank = bank_forks.read().unwrap().root_bank();
+    fn update_peers(&self, root_bank: &Bank) {
         let root_epoch = root_bank.epoch();
         let my_epoch = *self.epoch.read().unwrap();
 
@@ -135,7 +125,7 @@ impl ClusterSlots {
         }
     }
 
-    pub fn compute_weights(&self, slot: Slot, repair_peers: &[ContactInfo]) -> Vec<u64> {
+    pub(crate) fn compute_weights(&self, slot: Slot, repair_peers: &[ContactInfo]) -> Vec<u64> {
         if repair_peers.is_empty() {
             return Vec::default();
         }
@@ -165,7 +155,7 @@ impl ClusterSlots {
             .collect()
     }
 
-    pub fn compute_weights_exclude_noncomplete(
+    pub(crate) fn compute_weights_exclude_noncomplete(
         &self,
         slot: Slot,
         repair_peers: &[ContactInfo],
@@ -181,21 +171,6 @@ impl ClusterSlots {
             })
             .collect()
     }
-
-    pub fn generate_repairs_for_missing_slots(
-        &self,
-        self_id: &Pubkey,
-        root: Slot,
-    ) -> Vec<RepairType> {
-        let my_slots = self.collect(self_id);
-        self.cluster_slots
-            .read()
-            .unwrap()
-            .keys()
-            .filter(|x| **x > root && !my_slots.contains(*x))
-            .map(|x| RepairType::HighestShred(*x, 0))
-            .collect()
-    }
 }
 
 #[cfg(test)]
@@ -380,40 +355,4 @@ mod tests {
             Some(&1)
         );
     }
-
-    #[test]
-    fn test_generate_repairs() {
-        let cs = ClusterSlots::default();
-        let mut epoch_slot = EpochSlots::default();
-        epoch_slot.fill(&[1], 0);
-        cs.update_internal(0, vec![epoch_slot]);
-        let self_id = solana_sdk::pubkey::new_rand();
-        assert_eq!(
-            cs.generate_repairs_for_missing_slots(&self_id, 0),
-            vec![RepairType::HighestShred(1, 0)]
-        )
-    }
-
-    #[test]
-    fn test_collect_my_slots() {
-        let cs = ClusterSlots::default();
-        let mut epoch_slot = EpochSlots::default();
-        epoch_slot.fill(&[1], 0);
-        let self_id = epoch_slot.from;
-        cs.update_internal(0, vec![epoch_slot]);
-        let slots: Vec<Slot> = cs.collect(&self_id).into_iter().collect();
-        assert_eq!(slots, vec![1]);
-    }
-
-    #[test]
-    fn test_generate_repairs_existing() {
-        let cs = ClusterSlots::default();
-        let mut epoch_slot = EpochSlots::default();
-        epoch_slot.fill(&[1], 0);
-        let self_id = epoch_slot.from;
-        cs.update_internal(0, vec![epoch_slot]);
-        assert!(cs
-            .generate_repairs_for_missing_slots(&self_id, 0)
-            .is_empty());
-    }
 }
diff --git a/core/src/cluster_slots_service.rs b/core/src/cluster_slots_service.rs
@@ -91,7 +91,6 @@ impl ClusterSlotsService {
                     break;
                 }
             };
-            let new_root = bank_forks.read().unwrap().root();
             let mut lowest_slot_elapsed = Measure::start("lowest_slot_elapsed");
             let lowest_slot = blockstore.lowest_slot();
             Self::update_lowest_slot(lowest_slot, &cluster_info);
@@ -105,7 +104,8 @@ impl ClusterSlotsService {
                     &cluster_info,
                 );
             }
-            cluster_slots.update(new_root, &cluster_info, &bank_forks);
+            let root_bank = bank_forks.read().unwrap().root_bank();
+            cluster_slots.update(&root_bank, &cluster_info);
             process_cluster_slots_updates_elapsed.stop();
 
             cluster_slots_service_timing.update(