From 35ca5f7a880a78493594f9bd2428b8a9c36f123a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lech=20G=C5=82owiak?= Date: Tue, 13 Aug 2024 15:33:40 +0200 Subject: [PATCH 1/2] Skip slot before creating inherent data providers during major sync --- prdoc/pr_5344.prdoc | 10 ++++++++++ substrate/client/consensus/slots/src/lib.rs | 8 +------- substrate/client/consensus/slots/src/slots.rs | 17 +++++++++++++---- 3 files changed, 24 insertions(+), 11 deletions(-) create mode 100644 prdoc/pr_5344.prdoc diff --git a/prdoc/pr_5344.prdoc b/prdoc/pr_5344.prdoc new file mode 100644 index 000000000000..9f83c113686d --- /dev/null +++ b/prdoc/pr_5344.prdoc @@ -0,0 +1,10 @@ +title: Fix storage weight reclaim bug. + +doc: + - audience: Node Dev + description: | + Improvement in slot worker loop that will not call create inherent data providers if the major sync is in progress. Before it was called every slot and the results were discarded during major sync. + +crates: + - name: sc-consensus-slots + bump: minor diff --git a/substrate/client/consensus/slots/src/lib.rs b/substrate/client/consensus/slots/src/lib.rs index 7cdf90877dff..2c694a3cd2e8 100644 --- a/substrate/client/consensus/slots/src/lib.rs +++ b/substrate/client/consensus/slots/src/lib.rs @@ -517,16 +517,10 @@ pub async fn start_slot_worker( CIDP: CreateInherentDataProviders + Send + 'static, CIDP::InherentDataProviders: InherentDataProviderExt + Send, { - let mut slots = Slots::new(slot_duration.as_duration(), create_inherent_data_providers, client); + let mut slots = Slots::new(slot_duration.as_duration(), create_inherent_data_providers, client, sync_oracle); loop { let slot_info = slots.next_slot().await; - - if sync_oracle.is_major_syncing() { - debug!(target: LOG_TARGET, "Skipping proposal slot due to sync."); - continue - } - let _ = worker.on_slot(slot_info).await; } } diff --git a/substrate/client/consensus/slots/src/slots.rs b/substrate/client/consensus/slots/src/slots.rs index 203764310601..48a9bbeeb91f 100644 --- a/substrate/client/consensus/slots/src/slots.rs +++ b/substrate/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::SelectChain; +use sp_consensus::{SelectChain, SyncOracle}; use sp_inherents::{CreateInherentDataProviders, InherentDataProvider}; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; @@ -87,21 +87,23 @@ impl SlotInfo { } /// A stream that returns every time there is a new slot. -pub(crate) struct Slots { +pub(crate) struct Slots { last_slot: Slot, slot_duration: Duration, until_next_slot: Option, create_inherent_data_providers: IDP, select_chain: SC, + sync_oracle: SO, _phantom: std::marker::PhantomData, } -impl Slots { +impl Slots { /// Create a new `Slots` stream. pub fn new( slot_duration: Duration, create_inherent_data_providers: IDP, select_chain: SC, + sync_oracle: SO ) -> Self { Slots { last_slot: 0.into(), @@ -109,17 +111,19 @@ impl Slots { until_next_slot: None, create_inherent_data_providers, select_chain, + sync_oracle, _phantom: Default::default(), } } } -impl Slots +impl Slots where Block: BlockT, SC: SelectChain, IDP: CreateInherentDataProviders + 'static, IDP::InherentDataProviders: crate::InherentDataProviderExt, + SO: SyncOracle, { /// Returns a future that fires when the next slot starts. pub async fn next_slot(&mut self) -> SlotInfo { @@ -138,6 +142,11 @@ where let wait_dur = time_until_next_slot(self.slot_duration); self.until_next_slot = Some(Delay::new(wait_dur)); + if self.sync_oracle.is_major_syncing() { + log::debug!(target: LOG_TARGET, "Skipping slot: major sync is in progress."); + continue; + } + let chain_head = match self.select_chain.best_chain().await { Ok(x) => x, Err(e) => { From de507704a81370f4199c95b0498578f16145c75e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lech=20G=C5=82owiak?= Date: Mon, 19 Aug 2024 10:37:49 +0200 Subject: [PATCH 2/2] cargo fmt --- substrate/client/consensus/slots/src/lib.rs | 7 ++++++- substrate/client/consensus/slots/src/slots.rs | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/substrate/client/consensus/slots/src/lib.rs b/substrate/client/consensus/slots/src/lib.rs index 2c694a3cd2e8..06e0756fc968 100644 --- a/substrate/client/consensus/slots/src/lib.rs +++ b/substrate/client/consensus/slots/src/lib.rs @@ -517,7 +517,12 @@ pub async fn start_slot_worker( CIDP: CreateInherentDataProviders + Send + 'static, CIDP::InherentDataProviders: InherentDataProviderExt + Send, { - let mut slots = Slots::new(slot_duration.as_duration(), create_inherent_data_providers, client, sync_oracle); + let mut slots = Slots::new( + slot_duration.as_duration(), + create_inherent_data_providers, + client, + sync_oracle, + ); loop { let slot_info = slots.next_slot().await; diff --git a/substrate/client/consensus/slots/src/slots.rs b/substrate/client/consensus/slots/src/slots.rs index 48a9bbeeb91f..c0b412e8ad5b 100644 --- a/substrate/client/consensus/slots/src/slots.rs +++ b/substrate/client/consensus/slots/src/slots.rs @@ -103,7 +103,7 @@ impl Slots { slot_duration: Duration, create_inherent_data_providers: IDP, select_chain: SC, - sync_oracle: SO + sync_oracle: SO, ) -> Self { Slots { last_slot: 0.into(),