Browse Source

Make ClusterSlots initialize itself at creation (#7489)

- Remove Default trait from for ClusterSlots
- Introduce new() constructor that performs initialization
- Introduce DCOU default_for_tests() function for compatibility with
  existing tests

Co-authored-by: steviez <steven@anza.xyz>
puhtaytow 2 months ago
parent
commit
eb62c2ea05

+ 26 - 1
core/src/cluster_slots_service/cluster_slots.rs

@@ -74,7 +74,7 @@ struct RootEpoch {
     number: Epoch,
     number: Epoch,
     schedule: EpochSchedule,
     schedule: EpochSchedule,
 }
 }
-#[derive(Default)]
+
 pub struct ClusterSlots {
 pub struct ClusterSlots {
     // ring buffer storing, per slot, which stakes were committed to a certain slot.
     // ring buffer storing, per slot, which stakes were committed to a certain slot.
     cluster_slots: RwLock<VecDeque<RowContent>>,
     cluster_slots: RwLock<VecDeque<RowContent>>,
@@ -95,6 +95,31 @@ struct RowContent {
 }
 }
 
 
 impl ClusterSlots {
 impl ClusterSlots {
+    pub fn new(root_bank: &Bank, cluster_info: &ClusterInfo) -> Self {
+        let cluster_slots = Self::default();
+        cluster_slots.update(root_bank, cluster_info);
+        cluster_slots
+    }
+
+    // Intentionally private default function to disallow uninitialized construction
+    fn default() -> Self {
+        Self {
+            cluster_slots: RwLock::new(VecDeque::new()),
+            epoch_metadata: RwLock::new(HashMap::new()),
+            current_slot: AtomicU64::default(),
+            root_epoch: RwLock::new(None),
+            cursor: Mutex::new(Cursor::default()),
+            metrics_last_report: AtomicInterval::default(),
+            metric_allocations: AtomicU64::default(),
+            metric_write_locks: AtomicU64::default(),
+        }
+    }
+
+    #[cfg(feature = "dev-context-only-utils")]
+    pub fn default_for_tests() -> Self {
+        Self::default()
+    }
+
     #[inline]
     #[inline]
     pub(crate) fn lookup(&self, slot: Slot) -> Option<Arc<SlotSupporters>> {
     pub(crate) fn lookup(&self, slot: Slot) -> Option<Arc<SlotSupporters>> {
         let cluster_slots = self.cluster_slots.read().unwrap();
         let cluster_slots = self.cluster_slots.read().unwrap();

+ 2 - 2
core/src/repair/ancestor_hashes_service.rs

@@ -1174,7 +1174,7 @@ mod test {
     #[test]
     #[test]
     fn test_ancestor_hashes_service_find_epoch_slots_frozen_dead_slots() {
     fn test_ancestor_hashes_service_find_epoch_slots_frozen_dead_slots() {
         let vote_simulator = VoteSimulator::new(3);
         let vote_simulator = VoteSimulator::new(3);
-        let cluster_slots = ClusterSlots::default();
+        let cluster_slots = ClusterSlots::default_for_tests();
         let mut dead_slot_pool = HashSet::new();
         let mut dead_slot_pool = HashSet::new();
         let mut repairable_dead_slot_pool = HashSet::new();
         let mut repairable_dead_slot_pool = HashSet::new();
         let root_bank = vote_simulator.bank_forks.read().unwrap().root_bank();
         let root_bank = vote_simulator.bank_forks.read().unwrap().root_bank();
@@ -1387,7 +1387,7 @@ mod test {
             let repair_info = RepairInfo {
             let repair_info = RepairInfo {
                 bank_forks,
                 bank_forks,
                 cluster_info: requester_cluster_info,
                 cluster_info: requester_cluster_info,
-                cluster_slots: Arc::new(ClusterSlots::default()),
+                cluster_slots: Arc::new(ClusterSlots::default_for_tests()),
                 epoch_schedule,
                 epoch_schedule,
                 ancestor_duplicate_slots_sender,
                 ancestor_duplicate_slots_sender,
                 repair_validators: None,
                 repair_validators: None,

+ 2 - 2
core/src/repair/repair_service.rs

@@ -1641,7 +1641,7 @@ mod test {
         let bank_forks = BankForks::new_rw_arc(bank);
         let bank_forks = BankForks::new_rw_arc(bank);
         let ledger_path = get_tmp_ledger_path_auto_delete!();
         let ledger_path = get_tmp_ledger_path_auto_delete!();
         let blockstore = Arc::new(Blockstore::open(ledger_path.path()).unwrap());
         let blockstore = Arc::new(Blockstore::open(ledger_path.path()).unwrap());
-        let cluster_slots = ClusterSlots::default();
+        let cluster_slots = ClusterSlots::default_for_tests();
         let cluster_info = Arc::new(new_test_cluster_info());
         let cluster_info = Arc::new(new_test_cluster_info());
         let identity_keypair = cluster_info.keypair().clone();
         let identity_keypair = cluster_info.keypair().clone();
         let serve_repair = {
         let serve_repair = {
@@ -1759,7 +1759,7 @@ mod test {
         // Signal that this peer has confirmed the dead slot, and is thus
         // Signal that this peer has confirmed the dead slot, and is thus
         // a valid target for repair
         // a valid target for repair
         let dead_slot = 9;
         let dead_slot = 9;
-        let cluster_slots = ClusterSlots::default();
+        let cluster_slots = ClusterSlots::default_for_tests();
         cluster_slots.fake_epoch_info_for_tests(HashMap::from([(*valid_repair_peer.pubkey(), 42)]));
         cluster_slots.fake_epoch_info_for_tests(HashMap::from([(*valid_repair_peer.pubkey(), 42)]));
         cluster_slots.insert_node_id(dead_slot, *valid_repair_peer.pubkey());
         cluster_slots.insert_node_id(dead_slot, *valid_repair_peer.pubkey());
         cluster_info.insert_info(valid_repair_peer);
         cluster_info.insert_info(valid_repair_peer);

+ 2 - 2
core/src/repair/serve_repair.rs

@@ -1881,7 +1881,7 @@ mod tests {
         let GenesisConfigInfo { genesis_config, .. } = create_genesis_config(10_000);
         let GenesisConfigInfo { genesis_config, .. } = create_genesis_config(10_000);
         let bank = Bank::new_for_tests(&genesis_config);
         let bank = Bank::new_for_tests(&genesis_config);
         let bank_forks = BankForks::new_rw_arc(bank);
         let bank_forks = BankForks::new_rw_arc(bank);
-        let cluster_slots = ClusterSlots::default();
+        let cluster_slots = ClusterSlots::default_for_tests();
         let cluster_info = Arc::new(new_test_cluster_info());
         let cluster_info = Arc::new(new_test_cluster_info());
         let serve_repair = ServeRepair::new_for_test(
         let serve_repair = ServeRepair::new_for_test(
             cluster_info.clone(),
             cluster_info.clone(),
@@ -2182,7 +2182,7 @@ mod tests {
         let GenesisConfigInfo { genesis_config, .. } = create_genesis_config(10_000);
         let GenesisConfigInfo { genesis_config, .. } = create_genesis_config(10_000);
         let bank = Bank::new_for_tests(&genesis_config);
         let bank = Bank::new_for_tests(&genesis_config);
         let bank_forks = BankForks::new_rw_arc(bank);
         let bank_forks = BankForks::new_rw_arc(bank);
-        let cluster_slots = ClusterSlots::default();
+        let cluster_slots = ClusterSlots::default_for_tests();
         let cluster_info = Arc::new(new_test_cluster_info());
         let cluster_info = Arc::new(new_test_cluster_info());
         let me = cluster_info.my_contact_info();
         let me = cluster_info.my_contact_info();
         let (repair_request_quic_sender, _) = tokio::sync::mpsc::channel(/*buffer:*/ 128);
         let (repair_request_quic_sender, _) = tokio::sync::mpsc::channel(/*buffer:*/ 128);

+ 13 - 13
core/src/replay_stage.rs

@@ -5496,7 +5496,7 @@ pub(crate) mod tests {
             &mut tower,
             &mut tower,
             &mut progress,
             &mut progress,
             &VoteTracker::default(),
             &VoteTracker::default(),
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
             &bank_forks,
             &bank_forks,
             &mut heaviest_subtree_fork_choice,
             &mut heaviest_subtree_fork_choice,
             &mut latest_validator_votes_for_frozen_banks,
             &mut latest_validator_votes_for_frozen_banks,
@@ -5547,7 +5547,7 @@ pub(crate) mod tests {
             &mut tower,
             &mut tower,
             &mut progress,
             &mut progress,
             &VoteTracker::default(),
             &VoteTracker::default(),
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
             &bank_forks,
             &bank_forks,
             &mut heaviest_subtree_fork_choice,
             &mut heaviest_subtree_fork_choice,
             &mut latest_validator_votes_for_frozen_banks,
             &mut latest_validator_votes_for_frozen_banks,
@@ -5582,7 +5582,7 @@ pub(crate) mod tests {
             &mut tower,
             &mut tower,
             &mut progress,
             &mut progress,
             &VoteTracker::default(),
             &VoteTracker::default(),
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
             &bank_forks,
             &bank_forks,
             &mut heaviest_subtree_fork_choice,
             &mut heaviest_subtree_fork_choice,
             &mut latest_validator_votes_for_frozen_banks,
             &mut latest_validator_votes_for_frozen_banks,
@@ -5621,7 +5621,7 @@ pub(crate) mod tests {
             &mut tower,
             &mut tower,
             &mut vote_simulator.progress,
             &mut vote_simulator.progress,
             &VoteTracker::default(),
             &VoteTracker::default(),
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
             &vote_simulator.bank_forks,
             &vote_simulator.bank_forks,
             heaviest_subtree_fork_choice,
             heaviest_subtree_fork_choice,
             &mut latest_validator_votes_for_frozen_banks,
             &mut latest_validator_votes_for_frozen_banks,
@@ -5689,7 +5689,7 @@ pub(crate) mod tests {
             &mut tower,
             &mut tower,
             &mut vote_simulator.progress,
             &mut vote_simulator.progress,
             &VoteTracker::default(),
             &VoteTracker::default(),
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
             &vote_simulator.bank_forks,
             &vote_simulator.bank_forks,
             &mut vote_simulator.tbft_structs.heaviest_subtree_fork_choice,
             &mut vote_simulator.tbft_structs.heaviest_subtree_fork_choice,
             &mut vote_simulator.latest_validator_votes_for_frozen_banks,
             &mut vote_simulator.latest_validator_votes_for_frozen_banks,
@@ -5980,7 +5980,7 @@ pub(crate) mod tests {
             10,
             10,
             &bank_forks_arc,
             &bank_forks_arc,
             &vote_tracker,
             &vote_tracker,
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
         );
         );
 
 
         let propagated_stats = &progress_map.get(&10).unwrap().propagated_stats;
         let propagated_stats = &progress_map.get(&10).unwrap().propagated_stats;
@@ -6074,7 +6074,7 @@ pub(crate) mod tests {
             10,
             10,
             &bank_forks_arc,
             &bank_forks_arc,
             &vote_tracker,
             &vote_tracker,
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
         );
         );
 
 
         for i in 1..=10 {
         for i in 1..=10 {
@@ -6161,7 +6161,7 @@ pub(crate) mod tests {
             10,
             10,
             &bank_forks_arc,
             &bank_forks_arc,
             &vote_tracker,
             &vote_tracker,
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
         );
         );
 
 
         // Only the first 5 banks should have reached the threshold
         // Only the first 5 banks should have reached the threshold
@@ -6814,7 +6814,7 @@ pub(crate) mod tests {
             &mut tower,
             &mut tower,
             &mut progress,
             &mut progress,
             &vote_tracker,
             &vote_tracker,
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
             &bank_forks,
             &bank_forks,
             &mut HeaviestSubtreeForkChoice::new_from_bank_forks(bank_forks.clone()),
             &mut HeaviestSubtreeForkChoice::new_from_bank_forks(bank_forks.clone()),
             &mut LatestValidatorVotesForFrozenBanks::default(),
             &mut LatestValidatorVotesForFrozenBanks::default(),
@@ -7429,7 +7429,7 @@ pub(crate) mod tests {
             &mut tower,
             &mut tower,
             &mut progress,
             &mut progress,
             &VoteTracker::default(),
             &VoteTracker::default(),
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
             &bank_forks,
             &bank_forks,
             &mut tbft_structs.heaviest_subtree_fork_choice,
             &mut tbft_structs.heaviest_subtree_fork_choice,
             &mut latest_validator_votes_for_frozen_banks,
             &mut latest_validator_votes_for_frozen_banks,
@@ -7553,7 +7553,7 @@ pub(crate) mod tests {
             &mut tower,
             &mut tower,
             &mut progress,
             &mut progress,
             &VoteTracker::default(),
             &VoteTracker::default(),
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
             &bank_forks,
             &bank_forks,
             &mut tbft_structs.heaviest_subtree_fork_choice,
             &mut tbft_structs.heaviest_subtree_fork_choice,
             &mut latest_validator_votes_for_frozen_banks,
             &mut latest_validator_votes_for_frozen_banks,
@@ -8337,7 +8337,7 @@ pub(crate) mod tests {
             &mut tower,
             &mut tower,
             &mut progress,
             &mut progress,
             &VoteTracker::default(),
             &VoteTracker::default(),
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
             &bank_forks,
             &bank_forks,
             &mut tbft_structs.heaviest_subtree_fork_choice,
             &mut tbft_structs.heaviest_subtree_fork_choice,
             &mut latest_validator_votes_for_frozen_banks,
             &mut latest_validator_votes_for_frozen_banks,
@@ -8861,7 +8861,7 @@ pub(crate) mod tests {
             tower,
             tower,
             progress,
             progress,
             &VoteTracker::default(),
             &VoteTracker::default(),
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
             bank_forks,
             bank_forks,
             heaviest_subtree_fork_choice,
             heaviest_subtree_fork_choice,
             latest_validator_votes_for_frozen_banks,
             latest_validator_votes_for_frozen_banks,

+ 1 - 1
core/src/tvu.rs

@@ -536,7 +536,7 @@ pub mod tests {
         let max_complete_transaction_status_slot = Arc::new(AtomicU64::default());
         let max_complete_transaction_status_slot = Arc::new(AtomicU64::default());
         let ignored_prioritization_fee_cache = Arc::new(PrioritizationFeeCache::new(0u64));
         let ignored_prioritization_fee_cache = Arc::new(PrioritizationFeeCache::new(0u64));
         let outstanding_repair_requests = Arc::<RwLock<OutstandingShredRepairs>>::default();
         let outstanding_repair_requests = Arc::<RwLock<OutstandingShredRepairs>>::default();
-        let cluster_slots = Arc::new(ClusterSlots::default());
+        let cluster_slots = Arc::new(ClusterSlots::default_for_tests());
         let wen_restart_repair_slots = if enable_wen_restart {
         let wen_restart_repair_slots = if enable_wen_restart {
             Some(Arc::new(RwLock::new(vec![])))
             Some(Arc::new(RwLock::new(vec![])))
         } else {
         } else {

+ 7 - 2
core/src/validator.rs

@@ -1498,8 +1498,13 @@ impl Validator {
 
 
         let outstanding_repair_requests =
         let outstanding_repair_requests =
             Arc::<RwLock<repair::repair_service::OutstandingShredRepairs>>::default();
             Arc::<RwLock<repair::repair_service::OutstandingShredRepairs>>::default();
-        let cluster_slots =
-            Arc::new(crate::cluster_slots_service::cluster_slots::ClusterSlots::default());
+        let root_bank = bank_forks.read().unwrap().root_bank();
+        let cluster_slots = Arc::new({
+            crate::cluster_slots_service::cluster_slots::ClusterSlots::new(
+                &root_bank,
+                &cluster_info,
+            )
+        });
 
 
         // If RPC is supported and ConnectionCache is used, pass ConnectionCache for being warmup inside Tvu.
         // If RPC is supported and ConnectionCache is used, pass ConnectionCache for being warmup inside Tvu.
         let connection_cache_for_warmup =
         let connection_cache_for_warmup =

+ 1 - 1
core/src/vote_simulator.rs

@@ -192,7 +192,7 @@ impl VoteSimulator {
             tower,
             tower,
             &mut self.progress,
             &mut self.progress,
             &VoteTracker::default(),
             &VoteTracker::default(),
-            &ClusterSlots::default(),
+            &ClusterSlots::default_for_tests(),
             &self.bank_forks,
             &self.bank_forks,
             &mut self.tbft_structs.heaviest_subtree_fork_choice,
             &mut self.tbft_structs.heaviest_subtree_fork_choice,
             &mut self.latest_validator_votes_for_frozen_banks,
             &mut self.latest_validator_votes_for_frozen_banks,

+ 1 - 0
validator/Cargo.toml

@@ -99,6 +99,7 @@ assert_cmd = { workspace = true }
 predicates = { workspace = true }
 predicates = { workspace = true }
 pretty_assertions = { workspace = true }
 pretty_assertions = { workspace = true }
 solana-account-decoder = { workspace = true }
 solana-account-decoder = { workspace = true }
+solana-core = { workspace = true, features = ["dev-context-only-utils"] }
 solana-program-option = { workspace = true }
 solana-program-option = { workspace = true }
 solana-program-pack = { workspace = true }
 solana-program-pack = { workspace = true }
 solana-runtime = { workspace = true, features = ["dev-context-only-utils"] }
 solana-runtime = { workspace = true, features = ["dev-context-only-utils"] }

+ 1 - 1
validator/src/admin_rpc_service.rs

@@ -1068,7 +1068,7 @@ mod tests {
                         RwLock<repair_service::OutstandingShredRepairs>,
                         RwLock<repair_service::OutstandingShredRepairs>,
                     >::default(),
                     >::default(),
                     cluster_slots: Arc::new(
                     cluster_slots: Arc::new(
-                        solana_core::cluster_slots_service::cluster_slots::ClusterSlots::default(),
+                        solana_core::cluster_slots_service::cluster_slots::ClusterSlots::default_for_tests(),
                     ),
                     ),
                     node: None,
                     node: None,
                     banking_stage: Arc::new(RwLock::new(None)),
                     banking_stage: Arc::new(RwLock::new(None)),