From 8c62ed33e3edeb832a2c5a0dbdfdc17b6cfc3f73 Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Mon, 16 Jan 2023 16:08:57 +0100 Subject: [PATCH 1/3] Refactory of `next_slot` method Prevents slot worker exit if inherent data provider creation fails --- client/consensus/slots/src/slots.rs | 47 +++++++++++++++++------------ 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/client/consensus/slots/src/slots.rs b/client/consensus/slots/src/slots.rs index 9bb5650b313d5..c5a45db0a8a4e 100644 --- a/client/consensus/slots/src/slots.rs +++ b/client/consensus/slots/src/slots.rs @@ -124,24 +124,20 @@ where /// Returns a future that fires when the next slot starts. pub async fn next_slot(&mut self) -> Result, Error> { loop { - self.inner_delay = match self.inner_delay.take() { - None => { - // schedule wait. + // Wait for slot timeout + self.inner_delay + .take() + .unwrap_or_else(|| { + // Schedule first timeout. let wait_dur = time_until_next_slot(self.slot_duration); - Some(Delay::new(wait_dur)) - }, - Some(d) => Some(d), - }; - - if let Some(inner_delay) = self.inner_delay.take() { - inner_delay.await; - } - // timeout has fired. + Delay::new(wait_dur) + }) + .await; - let ends_in = time_until_next_slot(self.slot_duration); + // Schedule delay for next slot. + let wait_dur = time_until_next_slot(self.slot_duration); + self.inner_delay = Some(Delay::new(wait_dur)); - // reschedule delay for next slot. - self.inner_delay = Some(Delay::new(ends_in)); let chain_head = match self.select_chain.best_chain().await { Ok(x) => x, Err(e) => { @@ -150,20 +146,31 @@ where "Unable to author block in slot. No best block header: {}", e, ); - // Let's try at the next slot.. - self.inner_delay.take(); + // Let's retry at the next slot. continue }, }; - let inherent_data_providers = self + let inherent_data_providers = match self .create_inherent_data_providers .create_inherent_data_providers(chain_head.hash(), ()) - .await?; + .await + { + Ok(x) => x, + Err(e) => { + log::warn!( + target: LOG_TARGET, + "Unable to author block in slot. Failure creating inherent data provider: {}", + e, + ); + // Let's retry at the next slot. + continue + }, + }; let slot = inherent_data_providers.slot(); - // never yield the same slot twice. + // Never yield the same slot twice. if slot > self.last_slot { self.last_slot = slot; From aa7425896add1f98e67e03a897f2cd815ddd2f2d Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Tue, 17 Jan 2023 10:00:11 +0100 Subject: [PATCH 2/3] Failure is not possible anymore --- client/consensus/slots/src/lib.rs | 8 +------- client/consensus/slots/src/slots.rs | 8 ++++---- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 6126647e6190d..30d2c1761c712 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -524,13 +524,7 @@ pub async fn start_slot_worker( let mut slots = Slots::new(slot_duration.as_duration(), create_inherent_data_providers, client); loop { - let slot_info = match slots.next_slot().await { - Ok(r) => r, - Err(e) => { - warn!(target: LOG_TARGET, "Error while polling for next slot: {}", e); - return - }, - }; + let slot_info = slots.next_slot().await; if sync_oracle.is_major_syncing() { debug!(target: LOG_TARGET, "Skipping proposal slot due to sync."); diff --git a/client/consensus/slots/src/slots.rs b/client/consensus/slots/src/slots.rs index c5a45db0a8a4e..9da31edb8cf75 100644 --- a/client/consensus/slots/src/slots.rs +++ b/client/consensus/slots/src/slots.rs @@ -21,7 +21,7 @@ //! This is used instead of `futures_timer::Interval` because it was unreliable. use super::{InherentDataProviderExt, Slot, LOG_TARGET}; -use sp_consensus::{Error, SelectChain}; +use sp_consensus::SelectChain; use sp_inherents::{CreateInherentDataProviders, InherentDataProvider}; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; @@ -122,7 +122,7 @@ where IDP::InherentDataProviders: crate::InherentDataProviderExt, { /// Returns a future that fires when the next slot starts. - pub async fn next_slot(&mut self) -> Result, Error> { + pub async fn next_slot(&mut self) -> SlotInfo { loop { // Wait for slot timeout self.inner_delay @@ -174,13 +174,13 @@ where if slot > self.last_slot { self.last_slot = slot; - break Ok(SlotInfo::new( + break SlotInfo::new( slot, Box::new(inherent_data_providers), self.slot_duration, chain_head, None, - )) + ) } } } From 57a3cf565d586b7ae56018325cfc2a13df1e757c Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Tue, 17 Jan 2023 15:34:06 +0100 Subject: [PATCH 3/3] Rename struct member --- client/consensus/slots/src/slots.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/consensus/slots/src/slots.rs b/client/consensus/slots/src/slots.rs index 9da31edb8cf75..7bb263b2bdba7 100644 --- a/client/consensus/slots/src/slots.rs +++ b/client/consensus/slots/src/slots.rs @@ -90,7 +90,7 @@ impl SlotInfo { pub(crate) struct Slots { last_slot: Slot, slot_duration: Duration, - inner_delay: Option, + until_next_slot: Option, create_inherent_data_providers: IDP, select_chain: SC, _phantom: std::marker::PhantomData, @@ -106,7 +106,7 @@ impl Slots { Slots { last_slot: 0.into(), slot_duration, - inner_delay: None, + until_next_slot: None, create_inherent_data_providers, select_chain, _phantom: Default::default(), @@ -125,7 +125,7 @@ where pub async fn next_slot(&mut self) -> SlotInfo { loop { // Wait for slot timeout - self.inner_delay + self.until_next_slot .take() .unwrap_or_else(|| { // Schedule first timeout. @@ -136,7 +136,7 @@ where // Schedule delay for next slot. let wait_dur = time_until_next_slot(self.slot_duration); - self.inner_delay = Some(Delay::new(wait_dur)); + self.until_next_slot = Some(Delay::new(wait_dur)); let chain_head = match self.select_chain.best_chain().await { Ok(x) => x,