diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 084266ef1..9169def8c 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -40,7 +40,7 @@ use { }, solana_measure::measure::Measure, solana_metrics::inc_new_counter_info, - solana_poh::poh_recorder::{PohRecorder, GRACE_TICKS_FACTOR, MAX_GRACE_SLOTS}, + solana_poh::poh_recorder::{PohLeaderStatus, PohRecorder, GRACE_TICKS_FACTOR, MAX_GRACE_SLOTS}, solana_program_runtime::timings::ExecuteTimings, solana_rpc::{ optimistically_confirmed_bank_tracker::{BankNotification, BankNotificationSender}, @@ -1479,13 +1479,17 @@ impl ReplayStage { assert!(!poh_recorder.lock().unwrap().has_bank()); - let (reached_leader_slot, _grace_ticks, poh_slot, parent_slot) = - poh_recorder.lock().unwrap().reached_leader_slot(); + let (poh_slot, parent_slot) = match poh_recorder.lock().unwrap().reached_leader_slot() { + PohLeaderStatus::Reached { + poh_slot, + parent_slot, + } => (poh_slot, parent_slot), + PohLeaderStatus::NotReached => { + trace!("{} poh_recorder hasn't reached_leader_slot", my_pubkey); + return; + } + }; - if !reached_leader_slot { - trace!("{} poh_recorder hasn't reached_leader_slot", my_pubkey); - return; - } trace!("{} reached_leader_slot", my_pubkey); let parent = bank_forks diff --git a/poh/src/poh_recorder.rs b/poh/src/poh_recorder.rs index 49d07b1be..c4afedb20 100644 --- a/poh/src/poh_recorder.rs +++ b/poh/src/poh_recorder.rs @@ -190,6 +190,12 @@ pub struct WorkingBank { pub max_tick_height: u64, } +#[derive(Debug, PartialEq)] +pub enum PohLeaderStatus { + NotReached, + Reached { poh_slot: Slot, parent_slot: Slot }, +} + pub struct PohRecorder { pub poh: Arc>, tick_height: u64, @@ -199,7 +205,7 @@ pub struct PohRecorder { tick_cache: Vec<(Entry, u64)>, // cache of entry and its tick_height working_bank: Option, sender: Sender, - leader_first_tick_height: Option, + leader_first_tick_height_including_grace_ticks: Option, leader_last_tick_height: u64, // zero if none grace_ticks: u64, id: Pubkey, @@ -235,10 +241,14 @@ impl PohRecorder { GRACE_TICKS_FACTOR * MAX_GRACE_SLOTS, ); assert_eq!(self.ticks_per_slot, bank.ticks_per_slot()); - let (leader_first_tick_height, leader_last_tick_height, grace_ticks) = - Self::compute_leader_slot_tick_heights(next_leader_slot, self.ticks_per_slot); + let ( + leader_first_tick_height_including_grace_ticks, + leader_last_tick_height, + grace_ticks, + ) = Self::compute_leader_slot_tick_heights(next_leader_slot, self.ticks_per_slot); self.grace_ticks = grace_ticks; - self.leader_first_tick_height = leader_first_tick_height; + self.leader_first_tick_height_including_grace_ticks = + leader_first_tick_height_including_grace_ticks; self.leader_last_tick_height = leader_last_tick_height; datapoint_info!( @@ -255,18 +265,27 @@ impl PohRecorder { pub fn would_be_leader(&self, within_next_n_ticks: u64) -> bool { self.has_bank() - || self - .leader_first_tick_height - .map_or(false, |leader_first_tick_height| { - let ideal_leader_tick_height = - leader_first_tick_height.saturating_sub(self.grace_ticks); + || self.leader_first_tick_height_including_grace_ticks.map_or( + false, + |leader_first_tick_height_including_grace_ticks| { + let ideal_leader_tick_height = leader_first_tick_height_including_grace_ticks + .saturating_sub(self.grace_ticks); self.tick_height + within_next_n_ticks >= ideal_leader_tick_height && self.tick_height <= self.leader_last_tick_height - }) + }, + ) + } + + // Return the slot for a given tick height + fn slot_for_tick_height(&self, tick_height: u64) -> Slot { + // We need to subtract by one here because, assuming ticks per slot is 64, + // tick heights [1..64] correspond to slot 0. The last tick height of a slot + // is always a multiple of 64. + tick_height.saturating_sub(1) / self.ticks_per_slot } pub fn leader_after_n_slots(&self, slots: u64) -> Option { - let current_slot = self.tick_height.saturating_sub(1) / self.ticks_per_slot; + let current_slot = self.slot_for_tick_height(self.tick_height); self.leader_schedule_cache .slot_leader_at(current_slot + slots, None) } @@ -323,56 +342,57 @@ impl PohRecorder { } } - fn reached_leader_tick(&self, leader_first_tick_height: u64) -> bool { - let target_tick_height = leader_first_tick_height.saturating_sub(1); + fn reached_leader_tick(&self, leader_first_tick_height_including_grace_ticks: u64) -> bool { + let target_tick_height = leader_first_tick_height_including_grace_ticks.saturating_sub(1); let ideal_target_tick_height = target_tick_height.saturating_sub(self.grace_ticks); - let current_slot = self.tick_height / self.ticks_per_slot; + let next_tick_height = self.tick_height.saturating_add(1); + let next_slot = self.slot_for_tick_height(next_tick_height); // We've approached target_tick_height OR poh was reset to run immediately // Or, previous leader didn't transmit in any of its leader slots, so ignore grace ticks self.tick_height >= target_tick_height - || self.start_tick_height + self.grace_ticks == leader_first_tick_height + || self.start_tick_height + self.grace_ticks + == leader_first_tick_height_including_grace_ticks || (self.tick_height >= ideal_target_tick_height - && (self.prev_slot_was_mine(current_slot) - || !self.is_same_fork_as_previous_leader(current_slot))) + && (self.prev_slot_was_mine(next_slot) + || !self.is_same_fork_as_previous_leader(next_slot))) } pub fn start_slot(&self) -> Slot { self.start_bank.slot() } - /// returns if leader slot has been reached, how many grace ticks were afforded, - /// imputed leader_slot and self.start_slot() - /// reached_leader_slot() == true means "ready for a bank" - pub fn reached_leader_slot(&self) -> (bool, u64, Slot, Slot) { + /// Returns if the leader slot has been reached along with the current poh + /// slot and the parent slot (could be a few slots ago if any previous + /// leaders needed to be skipped). + pub fn reached_leader_slot(&self) -> PohLeaderStatus { trace!( - "tick_height {}, start_tick_height {}, leader_first_tick_height {:?}, grace_ticks {}, has_bank {}", + "tick_height {}, start_tick_height {}, leader_first_tick_height_including_grace_ticks {:?}, grace_ticks {}, has_bank {}", self.tick_height, self.start_tick_height, - self.leader_first_tick_height, + self.leader_first_tick_height_including_grace_ticks, self.grace_ticks, self.has_bank() ); let next_tick_height = self.tick_height + 1; - let next_leader_slot = (next_tick_height - 1) / self.ticks_per_slot; - if let Some(leader_first_tick_height) = self.leader_first_tick_height { - let target_tick_height = leader_first_tick_height.saturating_sub(1); - if self.reached_leader_tick(leader_first_tick_height) { + let next_poh_slot = self.slot_for_tick_height(next_tick_height); + if let Some(leader_first_tick_height_including_grace_ticks) = + self.leader_first_tick_height_including_grace_ticks + { + if self.reached_leader_tick(leader_first_tick_height_including_grace_ticks) { assert!(next_tick_height >= self.start_tick_height); - let ideal_target_tick_height = target_tick_height.saturating_sub(self.grace_ticks); - - return ( - true, - self.tick_height.saturating_sub(ideal_target_tick_height), - next_leader_slot, - self.start_slot(), - ); + let poh_slot = next_poh_slot; + let parent_slot = self.start_slot(); + return PohLeaderStatus::Reached { + poh_slot, + parent_slot, + }; } } - (false, 0, next_leader_slot, self.start_slot()) + PohLeaderStatus::NotReached } - // returns (leader_first_tick_height, leader_last_tick_height, grace_ticks) given the next + // returns (leader_first_tick_height_including_grace_ticks, leader_last_tick_height, grace_ticks) given the next // slot this recorder will lead fn compute_leader_slot_tick_heights( next_leader_slot: Option<(Slot, Slot)>, @@ -387,8 +407,10 @@ impl PohRecorder { ticks_per_slot * MAX_GRACE_SLOTS, ticks_per_slot * num_slots / GRACE_TICKS_FACTOR, ); + let leader_first_tick_height_including_grace_ticks = + leader_first_tick_height + grace_ticks; ( - Some(leader_first_tick_height + grace_ticks), + Some(leader_first_tick_height_including_grace_ticks), last_tick_height, grace_ticks, ) @@ -428,10 +450,11 @@ impl PohRecorder { self.tick_height = (self.start_slot() + 1) * self.ticks_per_slot; self.start_tick_height = self.tick_height + 1; - let (leader_first_tick_height, leader_last_tick_height, grace_ticks) = + let (leader_first_tick_height_including_grace_ticks, leader_last_tick_height, grace_ticks) = Self::compute_leader_slot_tick_heights(next_leader_slot, self.ticks_per_slot); self.grace_ticks = grace_ticks; - self.leader_first_tick_height = leader_first_tick_height; + self.leader_first_tick_height_including_grace_ticks = + leader_first_tick_height_including_grace_ticks; self.leader_last_tick_height = leader_last_tick_height; } @@ -534,7 +557,10 @@ impl PohRecorder { self.tick_height += 1; trace!("tick_height {}", self.tick_height); - if self.leader_first_tick_height.is_none() { + if self + .leader_first_tick_height_including_grace_ticks + .is_none() + { self.tick_overhead_us += timing::duration_as_us(&now.elapsed()); return; } @@ -679,7 +705,7 @@ impl PohRecorder { ); let (sender, receiver) = unbounded(); let (record_sender, record_receiver) = unbounded(); - let (leader_first_tick_height, leader_last_tick_height, grace_ticks) = + let (leader_first_tick_height_including_grace_ticks, leader_last_tick_height, grace_ticks) = Self::compute_leader_slot_tick_heights(next_leader_slot, ticks_per_slot); ( Self { @@ -691,7 +717,7 @@ impl PohRecorder { clear_bank_signal, start_bank, start_tick_height: tick_height + 1, - leader_first_tick_height, + leader_first_tick_height_including_grace_ticks, leader_last_tick_height, grace_ticks, id: *id, @@ -1590,12 +1616,18 @@ mod tests { ); // Test that with no next leader slot, we don't reach the leader slot - assert!(!poh_recorder.reached_leader_slot().0); + assert_eq!( + poh_recorder.reached_leader_slot(), + PohLeaderStatus::NotReached + ); // Test that with no next leader slot in reset(), we don't reach the leader slot assert_eq!(bank0.slot(), 0); poh_recorder.reset(bank0.clone(), None); - assert!(!poh_recorder.reached_leader_slot().0); + assert_eq!( + poh_recorder.reached_leader_slot(), + PohLeaderStatus::NotReached + ); // Provide a leader slot one slot down poh_recorder.reset(bank0.clone(), Some((2, 2))); @@ -1623,17 +1655,22 @@ mod tests { .unwrap(); // Test that we don't reach the leader slot because of grace ticks - assert!(!poh_recorder.reached_leader_slot().0); + assert_eq!( + poh_recorder.reached_leader_slot(), + PohLeaderStatus::NotReached + ); // reset poh now. we should immediately be leader let bank1 = Arc::new(Bank::new_from_parent(&bank0, &Pubkey::default(), 1)); assert_eq!(bank1.slot(), 1); poh_recorder.reset(bank1.clone(), Some((2, 2))); - let (reached_leader_slot, grace_ticks, leader_slot, ..) = - poh_recorder.reached_leader_slot(); - assert!(reached_leader_slot); - assert_eq!(grace_ticks, 0); - assert_eq!(leader_slot, 2); + assert_eq!( + poh_recorder.reached_leader_slot(), + PohLeaderStatus::Reached { + poh_slot: 2, + parent_slot: 1, + } + ); // Now test that with grace ticks we can reach leader slot // Set the leader slot one slot down @@ -1645,7 +1682,10 @@ mod tests { } // We are not the leader yet, as expected - assert!(!poh_recorder.reached_leader_slot().0); + assert_eq!( + poh_recorder.reached_leader_slot(), + PohLeaderStatus::NotReached + ); // Send the grace ticks for _ in 0..bank1.ticks_per_slot() / GRACE_TICKS_FACTOR { @@ -1653,11 +1693,14 @@ mod tests { } // We should be the leader now - let (reached_leader_slot, grace_ticks, leader_slot, ..) = - poh_recorder.reached_leader_slot(); - assert!(reached_leader_slot); - assert_eq!(grace_ticks, bank1.ticks_per_slot() / GRACE_TICKS_FACTOR); - assert_eq!(leader_slot, 3); + // without sending more ticks, we should be leader now + assert_eq!( + poh_recorder.reached_leader_slot(), + PohLeaderStatus::Reached { + poh_slot: 3, + parent_slot: 1, + } + ); // Let's test that correct grace ticks are reported // Set the leader slot one slot down @@ -1670,17 +1713,22 @@ mod tests { } // We are not the leader yet, as expected - assert!(!poh_recorder.reached_leader_slot().0); + assert_eq!( + poh_recorder.reached_leader_slot(), + PohLeaderStatus::NotReached + ); let bank3 = Arc::new(Bank::new_from_parent(&bank2, &Pubkey::default(), 3)); assert_eq!(bank3.slot(), 3); poh_recorder.reset(bank3.clone(), Some((4, 4))); // without sending more ticks, we should be leader now - let (reached_leader_slot, grace_ticks, leader_slot, ..) = - poh_recorder.reached_leader_slot(); - assert!(reached_leader_slot); - assert_eq!(grace_ticks, 0); - assert_eq!(leader_slot, 4); + assert_eq!( + poh_recorder.reached_leader_slot(), + PohLeaderStatus::Reached { + poh_slot: 4, + parent_slot: 3, + } + ); // Let's test that if a node overshoots the ticks for its target // leader slot, reached_leader_slot() will return true, because it's overdue @@ -1695,11 +1743,13 @@ mod tests { } // We are overdue to lead - let (reached_leader_slot, grace_ticks, leader_slot, ..) = - poh_recorder.reached_leader_slot(); - assert!(reached_leader_slot); - assert_eq!(grace_ticks, overshoot_factor * bank4.ticks_per_slot()); - assert_eq!(leader_slot, 9); + assert_eq!( + poh_recorder.reached_leader_slot(), + PohLeaderStatus::Reached { + poh_slot: 9, + parent_slot: 4, + } + ); } Blockstore::destroy(&ledger_path).unwrap(); }