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

* 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

(cherry picked from commit 1240217a73)

# Conflicts:
#	core/src/replay_stage.rs
#	poh/src/poh_recorder.rs

* resolve conflicts

Co-authored-by: Justin Starry <justin@solana.com>
This commit is contained in:
mergify[bot]
2022-01-23 04:16:53 +00:00
committed by GitHub
parent de46df6b1b
commit 5d27a7f4c4
2 changed files with 129 additions and 75 deletions

View File

@ -36,7 +36,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_rpc::{
optimistically_confirmed_bank_tracker::{BankNotification, BankNotificationSender},
rpc_subscriptions::RpcSubscriptions,
@ -1120,13 +1120,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();
if !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;
}
};
trace!("{} reached_leader_slot", my_pubkey);
let parent = bank_forks

View File

@ -152,6 +152,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,
@ -161,7 +167,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,
@ -197,10 +203,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!(
@ -217,18 +227,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)
}
@ -284,56 +303,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 last_reset_slot(&self) -> Slot {
self.start_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)>,
@ -348,8 +368,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,
)
@ -389,10 +411,11 @@ impl PohRecorder {
self.tick_height = (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;
}
@ -500,7 +523,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;
}
@ -645,7 +671,7 @@ impl PohRecorder {
);
let (sender, receiver) = channel();
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 {
@ -657,7 +683,7 @@ impl PohRecorder {
clear_bank_signal,
start_slot,
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,
@ -1522,11 +1548,17 @@ 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
poh_recorder.reset(bank.last_blockhash(), 0, 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(bank.last_blockhash(), 0, Some((2, 2)));
@ -1554,15 +1586,20 @@ 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
poh_recorder.reset(bank.last_blockhash(), 1, 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
@ -1574,7 +1611,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..bank.ticks_per_slot() / GRACE_TICKS_FACTOR {
@ -1582,11 +1622,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, bank.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
@ -1598,15 +1641,20 @@ 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
);
poh_recorder.reset(bank.last_blockhash(), 3, 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
@ -1620,11 +1668,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 * bank.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();
}