From 69ca09b388d2b57693de2d92e9a9204cd69e1e3b Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Sun, 23 Jun 2024 14:36:34 +0300 Subject: [PATCH] Block import cleanups (#4842) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I carried these things in a fork for a long time, I think wouldn't hurt to have it upstream. Originally submitted as part of https://github.com/paritytech/polkadot-sdk/pull/1598 that went nowhere. --------- Co-authored-by: Bastian Köcher --- Cargo.lock | 1 - substrate/client/consensus/common/Cargo.toml | 1 - .../consensus/common/src/import_queue.rs | 3 +- .../common/src/import_queue/basic_queue.rs | 30 +++++-------------- 4 files changed, 9 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 42960f3aa1ed..0ee39fe2fb66 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16998,7 +16998,6 @@ version = "0.33.0" dependencies = [ "async-trait", "futures", - "futures-timer", "log", "mockall 0.11.4", "parking_lot 0.12.1", diff --git a/substrate/client/consensus/common/Cargo.toml b/substrate/client/consensus/common/Cargo.toml index 6d642ec78fef..f45998e7d75c 100644 --- a/substrate/client/consensus/common/Cargo.toml +++ b/substrate/client/consensus/common/Cargo.toml @@ -18,7 +18,6 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] async-trait = "0.1.79" futures = { version = "0.3.30", features = ["thread-pool"] } -futures-timer = "3.0.1" log = { workspace = true, default-features = true } mockall = "0.11.3" parking_lot = "0.12.1" diff --git a/substrate/client/consensus/common/src/import_queue.rs b/substrate/client/consensus/common/src/import_queue.rs index 371465536c35..1ddda04126a9 100644 --- a/substrate/client/consensus/common/src/import_queue.rs +++ b/substrate/client/consensus/common/src/import_queue.rs @@ -104,7 +104,8 @@ pub trait Verifier: Send { /// /// The `import_*` methods can be called in order to send elements for the import queue to verify. pub trait ImportQueueService: Send { - /// Import bunch of blocks. + /// Import bunch of blocks, every next block must be an ancestor of the previous block in the + /// list. fn import_blocks(&mut self, origin: BlockOrigin, blocks: Vec>); /// Import block justifications. diff --git a/substrate/client/consensus/common/src/import_queue/basic_queue.rs b/substrate/client/consensus/common/src/import_queue/basic_queue.rs index f4f618d1b318..e5eac3896cc8 100644 --- a/substrate/client/consensus/common/src/import_queue/basic_queue.rs +++ b/substrate/client/consensus/common/src/import_queue/basic_queue.rs @@ -19,7 +19,6 @@ use futures::{ prelude::*, task::{Context, Poll}, }; -use futures_timer::Delay; use log::{debug, trace}; use prometheus_endpoint::Registry; use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver, TracingUnboundedSender}; @@ -28,7 +27,7 @@ use sp_runtime::{ traits::{Block as BlockT, Header as HeaderT, NumberFor}, Justification, Justifications, }; -use std::{pin::Pin, time::Duration}; +use std::pin::Pin; use crate::{ import_queue::{ @@ -224,7 +223,6 @@ async fn block_import_process( mut result_sender: BufferedLinkSender, mut block_import_receiver: TracingUnboundedReceiver>, metrics: Option, - delay_between_blocks: Duration, ) { loop { let worker_messages::ImportBlocks(origin, blocks) = match block_import_receiver.next().await @@ -239,15 +237,9 @@ async fn block_import_process( }, }; - let res = import_many_blocks( - &mut block_import, - origin, - blocks, - &mut verifier, - delay_between_blocks, - metrics.clone(), - ) - .await; + let res = + import_many_blocks(&mut block_import, origin, blocks, &mut verifier, metrics.clone()) + .await; result_sender.blocks_processed(res.imported, res.block_count, res.results); } @@ -276,13 +268,11 @@ impl BlockImportWorker { let (justification_sender, mut justification_port) = tracing_unbounded("mpsc_import_queue_worker_justification", 100_000); - let (block_import_sender, block_import_port) = + let (block_import_sender, block_import_receiver) = tracing_unbounded("mpsc_import_queue_worker_blocks", 100_000); let mut worker = BlockImportWorker { result_sender, justification_import, metrics }; - let delay_between_blocks = Duration::default(); - let future = async move { // Let's initialize `justification_import` if let Some(justification_import) = worker.justification_import.as_mut() { @@ -295,9 +285,8 @@ impl BlockImportWorker { block_import, verifier, worker.result_sender.clone(), - block_import_port, + block_import_receiver, worker.metrics.clone(), - delay_between_blocks, ); futures::pin_mut!(block_import_process); @@ -394,7 +383,6 @@ async fn import_many_blocks>( blocks_origin: BlockOrigin, blocks: Vec>, verifier: &mut V, - delay_between_blocks: Duration, metrics: Option, ) -> ImportManyBlocksResult { let count = blocks.len(); @@ -460,11 +448,7 @@ async fn import_many_blocks>( results.push((import_result, block_hash)); - if delay_between_blocks != Duration::default() && !has_error { - Delay::new(delay_between_blocks).await; - } else { - Yield::new().await - } + Yield::new().await } }