From 9e25cbda0163d047013edc223090f929a599ea3c Mon Sep 17 00:00:00 2001 From: Illia Bobyr Date: Fri, 10 May 2024 21:38:50 -0700 Subject: [PATCH] turbine::broadcast_stage: `then_some/ok_or_else` => `if` `if/else` seems easier to follow, as it produces less nesting. Assignment of `self.chained_merkle_root` into `chained_merkle_root`, that is then assigned back into `self.chained_merkle_root` is a no-op. While I can see how it is separating the computation of the `chained_merkle_root` for the current step from the update of the `self`, I think, overall, it is more confusing then helpful. The `Finish previous slot ...` comment is actually part of a multi-paragraph sequence of steps, that document the whole `process_receive_results()`. Step number was accidentally removed in `#1057`: commit 6b45dc95d89f8721e4e5dfcdb939a6e37bc21be9 Author: behzad nouri Date: Fri May 3 20:20:10 2024 +0000 retains chained Merkle root across leader slots (#1057) --- .../broadcast_stage/standard_broadcast_run.rs | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/turbine/src/broadcast_stage/standard_broadcast_run.rs b/turbine/src/broadcast_stage/standard_broadcast_run.rs index 57163e56d7d8af..11491fd290d60d 100644 --- a/turbine/src/broadcast_stage/standard_broadcast_run.rs +++ b/turbine/src/broadcast_stage/standard_broadcast_run.rs @@ -2,7 +2,7 @@ use { super::{ - broadcast_utils::{self, ReceiveResults}, + broadcast_utils::{self, get_chained_merkle_root_from_parent, ReceiveResults}, *, }, crate::cluster_nodes::ClusterNodesCache, @@ -207,8 +207,9 @@ impl StandardBroadcastRun { let mut to_shreds_time = Measure::start("broadcast_to_shreds"); let cluster_type = bank.cluster_type(); + // 1) Finish previous slot. if self.slot != bank.slot() { - // Finish previous slot if it was interrupted. + // Was it interrupted? if !self.completed { let shreds = self.finish_prev_slot( keypair, @@ -242,29 +243,30 @@ impl StandardBroadcastRun { // Refrain from generating shreds for the slot. return Err(Error::DuplicateSlotBroadcast(bank.slot())); } + // Reinitialize state for this slot. - let chained_merkle_root = (self.slot == bank.parent_slot()) - .then_some(self.chained_merkle_root) - .ok_or_else(|| { - broadcast_utils::get_chained_merkle_root_from_parent( - bank.slot(), - bank.parent_slot(), - blockstore, - ) - }) + + if self.slot != bank.parent_slot() { + self.chained_merkle_root = get_chained_merkle_root_from_parent( + bank.slot(), + bank.parent_slot(), + blockstore, + ) .unwrap_or_else(|err| { error!("Unknown chained Merkle root: {err:?}"); process_stats.err_unknown_chained_merkle_root += 1; Hash::default() }); + } + self.slot = bank.slot(); self.parent = bank.parent_slot(); - self.chained_merkle_root = chained_merkle_root; self.next_shred_index = 0u32; self.next_code_index = 0u32; self.completed = false; self.slot_broadcast_start = Instant::now(); self.num_batches = 0; + receive_elapsed = Duration::ZERO; coalesce_elapsed = Duration::ZERO; }