gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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

Browse Source 
* 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)

Co-authored-by: Justin Starry <justin@solana.com>
This commit is contained in:
mergify[bot]
2022-01-23 15:34:44 +08:00
committed by GitHub
parent 620a80b581
commit d9a5f714e1
2 changed files with 129 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
core/src/replay_stage.rs
View File

@@ -38,7 +38,7 @@ use {
}, },
solana_measure::measure::Measure, solana_measure::measure::Measure,
solana_metrics::inc_new_counter_info, 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_program_runtime::timings::ExecuteTimings,
solana_rpc::{ solana_rpc::{
optimistically_confirmed_bank_tracker::{BankNotification, BankNotificationSender}, optimistically_confirmed_bank_tracker::{BankNotification, BankNotificationSender},
@@ -1395,13 +1395,17 @@ impl ReplayStage {
assert!(!poh_recorder.lock().unwrap().has_bank()); assert!(!poh_recorder.lock().unwrap().has_bank());
let (reached_leader_slot, _grace_ticks, poh_slot, parent_slot) = let (poh_slot, parent_slot) = match poh_recorder.lock().unwrap().reached_leader_slot() {
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); trace!("{} reached_leader_slot", my_pubkey);
let parent = bank_forks let parent = bank_forks

186
poh/src/poh_recorder.rs
View File

@@ -193,6 +193,12 @@ pub struct WorkingBank {
pub max_tick_height: u64, pub max_tick_height: u64,
} }
#[derive(Debug, PartialEq)]
pub enum PohLeaderStatus {
NotReached,
Reached { poh_slot: Slot, parent_slot: Slot },
}
pub struct PohRecorder { pub struct PohRecorder {
pub poh: Arc<Mutex<Poh>>, pub poh: Arc<Mutex<Poh>>,
tick_height: u64, tick_height: u64,
@@ -202,7 +208,7 @@ pub struct PohRecorder {
tick_cache: Vec<(Entry, u64)>, // cache of entry and its tick_height tick_cache: Vec<(Entry, u64)>, // cache of entry and its tick_height
working_bank: Option<WorkingBank>, working_bank: Option<WorkingBank>,
sender: Sender<WorkingBankEntry>, 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 leader_last_tick_height: u64, // zero if none
grace_ticks: u64, grace_ticks: u64,
id: Pubkey, id: Pubkey,
@@ -238,10 +244,14 @@ impl PohRecorder {
GRACE_TICKS_FACTOR * MAX_GRACE_SLOTS, GRACE_TICKS_FACTOR * MAX_GRACE_SLOTS,
); );
assert_eq!(self.ticks_per_slot, bank.ticks_per_slot()); assert_eq!(self.ticks_per_slot, bank.ticks_per_slot());
let (leader_first_tick_height, leader_last_tick_height, grace_ticks) = let (
Self::compute_leader_slot_tick_heights(next_leader_slot, self.ticks_per_slot); 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.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; self.leader_last_tick_height = leader_last_tick_height;
datapoint_info!( datapoint_info!(
@@ -258,18 +268,27 @@ impl PohRecorder {
pub fn would_be_leader(&self, within_next_n_ticks: u64) -> bool { pub fn would_be_leader(&self, within_next_n_ticks: u64) -> bool {
self.has_bank() self.has_bank()
|| self || self.leader_first_tick_height_including_grace_ticks.map_or(
.leader_first_tick_height false,
.map_or(false, |leader_first_tick_height| { |leader_first_tick_height_including_grace_ticks| {
let ideal_leader_tick_height = let ideal_leader_tick_height = leader_first_tick_height_including_grace_ticks
leader_first_tick_height.saturating_sub(self.grace_ticks); .saturating_sub(self.grace_ticks);
self.tick_height + within_next_n_ticks >= ideal_leader_tick_height self.tick_height + within_next_n_ticks >= ideal_leader_tick_height
&& self.tick_height <= self.leader_last_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> { 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 self.leader_schedule_cache
.slot_leader_at(current_slot + slots, None) .slot_leader_at(current_slot + slots, None)
} }
@@ -326,56 +345,57 @@ impl PohRecorder {
} }
} }
fn reached_leader_tick(&self, leader_first_tick_height: u64) -> bool { fn reached_leader_tick(&self, leader_first_tick_height_including_grace_ticks: u64) -> bool {
let target_tick_height = leader_first_tick_height.saturating_sub(1); 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 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 // 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 // Or, previous leader didn't transmit in any of its leader slots, so ignore grace ticks
self.tick_height >= target_tick_height 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.tick_height >= ideal_target_tick_height
&& (self.prev_slot_was_mine(current_slot) && (self.prev_slot_was_mine(next_slot)
|| !self.is_same_fork_as_previous_leader(current_slot))) || !self.is_same_fork_as_previous_leader(next_slot)))
} }
pub fn start_slot(&self) -> Slot { pub fn start_slot(&self) -> Slot {
self.start_bank.slot() self.start_bank.slot()
} }
/// returns if leader slot has been reached, how many grace ticks were afforded, /// Returns if the leader slot has been reached along with the current poh
/// imputed leader_slot and self.start_slot() /// slot and the parent slot (could be a few slots ago if any previous
/// reached_leader_slot() == true means "ready for a bank" /// leaders needed to be skipped).
pub fn reached_leader_slot(&self) -> (bool, u64, Slot, Slot) { pub fn reached_leader_slot(&self) -> PohLeaderStatus {
trace!( 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.tick_height,
self.start_tick_height, self.start_tick_height,
self.leader_first_tick_height, self.leader_first_tick_height_including_grace_ticks,
self.grace_ticks, self.grace_ticks,
self.has_bank() self.has_bank()
); );
let next_tick_height = self.tick_height + 1; let next_tick_height = self.tick_height + 1;
let next_leader_slot = (next_tick_height - 1) / self.ticks_per_slot; let next_poh_slot = self.slot_for_tick_height(next_tick_height);
if let Some(leader_first_tick_height) = self.leader_first_tick_height { if let Some(leader_first_tick_height_including_grace_ticks) =
let target_tick_height = leader_first_tick_height.saturating_sub(1); self.leader_first_tick_height_including_grace_ticks
if self.reached_leader_tick(leader_first_tick_height) { {
if self.reached_leader_tick(leader_first_tick_height_including_grace_ticks) {
assert!(next_tick_height >= self.start_tick_height); assert!(next_tick_height >= self.start_tick_height);
let ideal_target_tick_height = target_tick_height.saturating_sub(self.grace_ticks); let poh_slot = next_poh_slot;
let parent_slot = self.start_slot();
return ( return PohLeaderStatus::Reached {
true, poh_slot,
self.tick_height.saturating_sub(ideal_target_tick_height), parent_slot,
next_leader_slot, };
self.start_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 // slot this recorder will lead
fn compute_leader_slot_tick_heights( fn compute_leader_slot_tick_heights(
next_leader_slot: Option<(Slot, Slot)>, next_leader_slot: Option<(Slot, Slot)>,
@@ -390,8 +410,10 @@ impl PohRecorder {
ticks_per_slot * MAX_GRACE_SLOTS, ticks_per_slot * MAX_GRACE_SLOTS,
ticks_per_slot * num_slots / GRACE_TICKS_FACTOR, 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, last_tick_height,
grace_ticks, grace_ticks,
) )
@@ -431,10 +453,11 @@ impl PohRecorder {
self.tick_height = (self.start_slot() + 1) * self.ticks_per_slot; self.tick_height = (self.start_slot() + 1) * self.ticks_per_slot;
self.start_tick_height = self.tick_height + 1; 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::compute_leader_slot_tick_heights(next_leader_slot, self.ticks_per_slot);
self.grace_ticks = grace_ticks; 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; self.leader_last_tick_height = leader_last_tick_height;
} }
@@ -537,7 +560,10 @@ impl PohRecorder {
self.tick_height += 1; self.tick_height += 1;
trace!("tick_height {}", self.tick_height); 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()); self.tick_overhead_us += timing::duration_as_us(&now.elapsed());
return; return;
} }
@@ -682,7 +708,7 @@ impl PohRecorder {
); );
let (sender, receiver) = channel(); let (sender, receiver) = channel();
let (record_sender, record_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::compute_leader_slot_tick_heights(next_leader_slot, ticks_per_slot);
( (
Self { Self {
@@ -694,7 +720,7 @@ impl PohRecorder {
clear_bank_signal, clear_bank_signal,
start_bank, start_bank,
start_tick_height: tick_height + 1, start_tick_height: tick_height + 1,
leader_first_tick_height, leader_first_tick_height_including_grace_ticks,
leader_last_tick_height, leader_last_tick_height,
grace_ticks, grace_ticks,
id: *id, id: *id,
@@ -1593,12 +1619,18 @@ mod tests {
); );
// Test that with no next leader slot, we don't reach the leader slot // 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 // Test that with no next leader slot in reset(), we don't reach the leader slot
assert_eq!(bank0.slot(), 0); assert_eq!(bank0.slot(), 0);
poh_recorder.reset(bank0.clone(), None); 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 // Provide a leader slot one slot down
poh_recorder.reset(bank0.clone(), Some((2, 2))); poh_recorder.reset(bank0.clone(), Some((2, 2)));
@@ -1626,17 +1658,22 @@ mod tests {
.unwrap(); .unwrap();
// Test that we don't reach the leader slot because of grace ticks // 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 // reset poh now. we should immediately be leader
let bank1 = Arc::new(Bank::new_from_parent(&bank0, &Pubkey::default(), 1)); let bank1 = Arc::new(Bank::new_from_parent(&bank0, &Pubkey::default(), 1));
assert_eq!(bank1.slot(), 1); assert_eq!(bank1.slot(), 1);
poh_recorder.reset(bank1.clone(), Some((2, 2))); poh_recorder.reset(bank1.clone(), Some((2, 2)));
let (reached_leader_slot, grace_ticks, leader_slot, ..) = assert_eq!(
poh_recorder.reached_leader_slot(); poh_recorder.reached_leader_slot(),
assert!(reached_leader_slot); PohLeaderStatus::Reached {
assert_eq!(grace_ticks, 0); poh_slot: 2,
assert_eq!(leader_slot, 2); parent_slot: 1,
}
);
// Now test that with grace ticks we can reach leader slot // Now test that with grace ticks we can reach leader slot
// Set the leader slot one slot down // Set the leader slot one slot down
@@ -1648,7 +1685,10 @@ mod tests {
} }
// We are not the leader yet, as expected // 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 // Send the grace ticks
for _ in 0..bank1.ticks_per_slot() / GRACE_TICKS_FACTOR { for _ in 0..bank1.ticks_per_slot() / GRACE_TICKS_FACTOR {
@@ -1656,11 +1696,14 @@ mod tests {
} }
// We should be the leader now // We should be the leader now
let (reached_leader_slot, grace_ticks, leader_slot, ..) = // without sending more ticks, we should be leader now
poh_recorder.reached_leader_slot(); assert_eq!(
assert!(reached_leader_slot); poh_recorder.reached_leader_slot(),
assert_eq!(grace_ticks, bank1.ticks_per_slot() / GRACE_TICKS_FACTOR); PohLeaderStatus::Reached {
assert_eq!(leader_slot, 3); poh_slot: 3,
parent_slot: 1,
}
);
// Let's test that correct grace ticks are reported // Let's test that correct grace ticks are reported
// Set the leader slot one slot down // Set the leader slot one slot down
@@ -1673,17 +1716,22 @@ mod tests {
} }
// We are not the leader yet, as expected // 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)); let bank3 = Arc::new(Bank::new_from_parent(&bank2, &Pubkey::default(), 3));
assert_eq!(bank3.slot(), 3); assert_eq!(bank3.slot(), 3);
poh_recorder.reset(bank3.clone(), Some((4, 4))); poh_recorder.reset(bank3.clone(), Some((4, 4)));
// without sending more ticks, we should be leader now // without sending more ticks, we should be leader now
let (reached_leader_slot, grace_ticks, leader_slot, ..) = assert_eq!(
poh_recorder.reached_leader_slot(); poh_recorder.reached_leader_slot(),
assert!(reached_leader_slot); PohLeaderStatus::Reached {
assert_eq!(grace_ticks, 0); poh_slot: 4,
assert_eq!(leader_slot, 4); parent_slot: 3,
}
);
// Let's test that if a node overshoots the ticks for its target // 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 // leader slot, reached_leader_slot() will return true, because it's overdue
@@ -1698,11 +1746,13 @@ mod tests {
} }
// We are overdue to lead // We are overdue to lead
let (reached_leader_slot, grace_ticks, leader_slot, ..) = assert_eq!(
poh_recorder.reached_leader_slot(); poh_recorder.reached_leader_slot(),
assert!(reached_leader_slot); PohLeaderStatus::Reached {
assert_eq!(grace_ticks, overshoot_factor * bank4.ticks_per_slot()); poh_slot: 9,
assert_eq!(leader_slot, 9); parent_slot: 4,
}
);
} }
Blockstore::destroy(&ledger_path).unwrap(); Blockstore::destroy(&ledger_path).unwrap();
} }