diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index 383c4c19a8..97b74f29c3 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -432,8 +432,9 @@ impl BankingStage { if unprocessed_transaction_storage.should_not_process() { return; } - let ((metrics_action, decision), make_decision_time) = - measure!(decision_maker.make_consume_or_forward_decision(slot_metrics_tracker)); + let (decision, make_decision_time) = + measure!(decision_maker.make_consume_or_forward_decision()); + let metrics_action = slot_metrics_tracker.check_leader_slot_boundary(decision.bank_start()); slot_metrics_tracker.increment_make_decision_us(make_decision_time.as_us()); match decision { diff --git a/core/src/banking_stage/decision_maker.rs b/core/src/banking_stage/decision_maker.rs index 819b7618ae..2077095e89 100644 --- a/core/src/banking_stage/decision_maker.rs +++ b/core/src/banking_stage/decision_maker.rs @@ -1,5 +1,4 @@ use { - crate::leader_slot_banking_stage_metrics::{LeaderSlotMetricsTracker, MetricsTrackerAction}, solana_poh::poh_recorder::{BankStart, PohRecorder}, solana_sdk::{ clock::{ @@ -19,6 +18,16 @@ pub enum BufferedPacketsDecision { Hold, } +impl BufferedPacketsDecision { + /// Returns the `BankStart` if the decision is `Consume`. Otherwise, returns `None`. + pub fn bank_start(&self) -> Option<&BankStart> { + match self { + Self::Consume(bank_start) => Some(bank_start), + _ => None, + } + } +} + pub struct DecisionMaker { my_pubkey: Pubkey, poh_recorder: Arc>, @@ -32,10 +41,7 @@ impl DecisionMaker { } } - pub(crate) fn make_consume_or_forward_decision( - &self, - slot_metrics_tracker: &mut LeaderSlotMetricsTracker, - ) -> (MetricsTrackerAction, BufferedPacketsDecision) { + pub(crate) fn make_consume_or_forward_decision(&self) -> BufferedPacketsDecision { let (leader_at_slot_offset, bank_start, would_be_leader, would_be_leader_shortly) = { let poh = self.poh_recorder.read().unwrap(); let bank_start = poh @@ -51,15 +57,12 @@ impl DecisionMaker { ) }; - ( - slot_metrics_tracker.check_leader_slot_boundary(&bank_start), - Self::consume_or_forward_packets( - &self.my_pubkey, - leader_at_slot_offset, - bank_start, - would_be_leader, - would_be_leader_shortly, - ), + Self::consume_or_forward_packets( + &self.my_pubkey, + leader_at_slot_offset, + bank_start, + would_be_leader, + would_be_leader_shortly, ) } @@ -105,6 +108,23 @@ mod tests { std::{sync::Arc, time::Instant}, }; + #[test] + fn test_buffered_packet_decision_bank_start() { + let bank = Arc::new(Bank::default_for_tests()); + let bank_start = BankStart { + working_bank: bank, + bank_creation_time: Arc::new(Instant::now()), + }; + assert!(BufferedPacketsDecision::Consume(bank_start) + .bank_start() + .is_some()); + assert!(BufferedPacketsDecision::Forward.bank_start().is_none()); + assert!(BufferedPacketsDecision::ForwardAndHold + .bank_start() + .is_none()); + assert!(BufferedPacketsDecision::Hold.bank_start().is_none()); + } + #[test] fn test_should_process_or_forward_packets() { let my_pubkey = solana_sdk::pubkey::new_rand(); diff --git a/core/src/leader_slot_banking_stage_metrics.rs b/core/src/leader_slot_banking_stage_metrics.rs index 6e9898c33f..71b531eba2 100644 --- a/core/src/leader_slot_banking_stage_metrics.rs +++ b/core/src/leader_slot_banking_stage_metrics.rs @@ -371,7 +371,7 @@ impl LeaderSlotMetricsTracker { // Check leader slot, return MetricsTrackerAction to be applied by apply_action() pub(crate) fn check_leader_slot_boundary( &mut self, - bank_start: &Option, + bank_start: Option<&BankStart>, ) -> MetricsTrackerAction { match (self.leader_slot_metrics.as_mut(), bank_start) { (None, None) => MetricsTrackerAction::Noop, @@ -896,7 +896,7 @@ mod tests { .. } = setup_test_slot_boundary_banks(); // Test that with no bank being tracked, and no new bank being tracked, nothing is reported - let action = leader_slot_metrics_tracker.check_leader_slot_boundary(&None); + let action = leader_slot_metrics_tracker.check_leader_slot_boundary(None); assert_eq!( mem::discriminant(&MetricsTrackerAction::Noop), mem::discriminant(&action) @@ -917,7 +917,7 @@ mod tests { // Metrics should not be reported because leader slot has not ended assert!(leader_slot_metrics_tracker.leader_slot_metrics.is_none()); let action = - leader_slot_metrics_tracker.check_leader_slot_boundary(&Some(first_poh_recorder_bank)); + leader_slot_metrics_tracker.check_leader_slot_boundary(Some(&first_poh_recorder_bank)); assert_eq!( mem::discriminant(&MetricsTrackerAction::NewTracker(None)), mem::discriminant(&action) @@ -941,12 +941,12 @@ mod tests { { // Setup first_bank let action = leader_slot_metrics_tracker - .check_leader_slot_boundary(&Some(first_poh_recorder_bank)); + .check_leader_slot_boundary(Some(&first_poh_recorder_bank)); assert!(leader_slot_metrics_tracker.apply_action(action).is_none()); } { // Assert reporting if slot has ended - let action = leader_slot_metrics_tracker.check_leader_slot_boundary(&None); + let action = leader_slot_metrics_tracker.check_leader_slot_boundary(None); assert_eq!( mem::discriminant(&MetricsTrackerAction::ReportAndResetTracker), mem::discriminant(&action) @@ -959,7 +959,7 @@ mod tests { } { // Assert no-op if still no new bank - let action = leader_slot_metrics_tracker.check_leader_slot_boundary(&None); + let action = leader_slot_metrics_tracker.check_leader_slot_boundary(None); assert_eq!( mem::discriminant(&MetricsTrackerAction::Noop), mem::discriminant(&action) @@ -981,13 +981,13 @@ mod tests { { // Setup with first_bank let action = leader_slot_metrics_tracker - .check_leader_slot_boundary(&Some(first_poh_recorder_bank.clone())); + .check_leader_slot_boundary(Some(&first_poh_recorder_bank)); assert!(leader_slot_metrics_tracker.apply_action(action).is_none()); } { // Assert nop-op if same bank let action = leader_slot_metrics_tracker - .check_leader_slot_boundary(&Some(first_poh_recorder_bank)); + .check_leader_slot_boundary(Some(&first_poh_recorder_bank)); assert_eq!( mem::discriminant(&MetricsTrackerAction::Noop), mem::discriminant(&action) @@ -996,7 +996,7 @@ mod tests { } { // Assert reporting if slot has ended - let action = leader_slot_metrics_tracker.check_leader_slot_boundary(&None); + let action = leader_slot_metrics_tracker.check_leader_slot_boundary(None); assert_eq!( mem::discriminant(&MetricsTrackerAction::ReportAndResetTracker), mem::discriminant(&action) @@ -1025,13 +1025,13 @@ mod tests { { // Setup with first_bank let action = leader_slot_metrics_tracker - .check_leader_slot_boundary(&Some(first_poh_recorder_bank)); + .check_leader_slot_boundary(Some(&first_poh_recorder_bank)); assert!(leader_slot_metrics_tracker.apply_action(action).is_none()); } { // Assert reporting if new bank let action = leader_slot_metrics_tracker - .check_leader_slot_boundary(&Some(next_poh_recorder_bank)); + .check_leader_slot_boundary(Some(&next_poh_recorder_bank)); assert_eq!( mem::discriminant(&MetricsTrackerAction::ReportAndNewTracker(None)), mem::discriminant(&action) @@ -1044,7 +1044,7 @@ mod tests { } { // Assert reporting if slot has ended - let action = leader_slot_metrics_tracker.check_leader_slot_boundary(&None); + let action = leader_slot_metrics_tracker.check_leader_slot_boundary(None); assert_eq!( mem::discriminant(&MetricsTrackerAction::ReportAndResetTracker), mem::discriminant(&action) @@ -1072,13 +1072,13 @@ mod tests { { // Setup with next_bank let action = leader_slot_metrics_tracker - .check_leader_slot_boundary(&Some(next_poh_recorder_bank)); + .check_leader_slot_boundary(Some(&next_poh_recorder_bank)); assert!(leader_slot_metrics_tracker.apply_action(action).is_none()); } { // Assert reporting if new bank let action = leader_slot_metrics_tracker - .check_leader_slot_boundary(&Some(first_poh_recorder_bank)); + .check_leader_slot_boundary(Some(&first_poh_recorder_bank)); assert_eq!( mem::discriminant(&MetricsTrackerAction::ReportAndNewTracker(None)), mem::discriminant(&action) @@ -1091,7 +1091,7 @@ mod tests { } { // Assert reporting if slot has ended - let action = leader_slot_metrics_tracker.check_leader_slot_boundary(&None); + let action = leader_slot_metrics_tracker.check_leader_slot_boundary(None); assert_eq!( mem::discriminant(&MetricsTrackerAction::ReportAndResetTracker), mem::discriminant(&action)