Refactor: Rename variables and helper method to `PohRecorder` (#22676)

* Refactor: Rename leader_first_tick_height field

* Refactor: add `PohRecorder::slot_for_tick_height` helper

* Refactor: Add type for poh leader status
This commit is contained in:
Justin Starry 2022-01-23 10:28:50 +08:00 committed by GitHub
parent 31ed4c18f9
commit 1240217a73
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 129 additions and 75 deletions

View File

@ -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

View File

@ -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<Mutex<Poh>>,
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<WorkingBank>,
sender: Sender<WorkingBankEntry>,
leader_first_tick_height: Option<u64>,
leader_first_tick_height_including_grace_ticks: Option<u64>,
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<Pubkey> {
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();
}