From 2ab5f9aeca6df4ad976b32e3e4613eafbefbc639 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 2 Jun 2021 20:13:47 +0200 Subject: [PATCH] Transactionpool: Make `ready_at` return earlier (#8995) `ready_at` returns when we have processed the requested block. However, on startup we already have processed the best block and there are no transactions in the pool on startup anyway. So, we can set `updated_at` to the best block on startup. Besides that `ready_at` now returns early when there are no ready nor any future transactions in the pool. --- .../client/consensus/manual-seal/src/lib.rs | 10 ++- substrate/client/transaction-pool/src/lib.rs | 66 ++++++++++++++----- 2 files changed, 58 insertions(+), 18 deletions(-) diff --git a/substrate/client/consensus/manual-seal/src/lib.rs b/substrate/client/consensus/manual-seal/src/lib.rs index 45628e90a6..2473ac848c 100644 --- a/substrate/client/consensus/manual-seal/src/lib.rs +++ b/substrate/client/consensus/manual-seal/src/lib.rs @@ -296,7 +296,13 @@ mod tests { let client = Arc::new(client); let spawner = sp_core::testing::TaskExecutor::new(); let pool = Arc::new(BasicPool::with_revalidation_type( - Options::default(), true.into(), api(), None, RevalidationType::Full, spawner.clone(), + Options::default(), + true.into(), + api(), + None, + RevalidationType::Full, + spawner.clone(), + 0, )); let env = ProposerFactory::new( spawner.clone(), @@ -373,6 +379,7 @@ mod tests { None, RevalidationType::Full, spawner.clone(), + 0, )); let env = ProposerFactory::new( spawner.clone(), @@ -453,6 +460,7 @@ mod tests { None, RevalidationType::Full, spawner.clone(), + 0, )); let env = ProposerFactory::new( spawner.clone(), diff --git a/substrate/client/transaction-pool/src/lib.rs b/substrate/client/transaction-pool/src/lib.rs index bc5f6e367f..32bea107d8 100644 --- a/substrate/client/transaction-pool/src/lib.rs +++ b/substrate/client/transaction-pool/src/lib.rs @@ -98,6 +98,13 @@ impl Default for ReadyPoll { } impl ReadyPoll { + fn new(best_block_number: NumberFor) -> Self { + Self { + updated_at: best_block_number, + pollers: Default::default(), + } + } + fn trigger(&mut self, number: NumberFor, iterator_factory: impl Fn() -> T) { self.updated_at = number; @@ -189,6 +196,7 @@ impl BasicPool prometheus: Option<&PrometheusRegistry>, revalidation_type: RevalidationType, spawner: impl SpawnNamed, + best_block_number: NumberFor, ) -> Self { let pool = Arc::new(sc_transaction_graph::Pool::new(options, is_validator, pool_api.clone())); let (revalidation_queue, background_task) = match revalidation_type { @@ -213,7 +221,7 @@ impl BasicPool RevalidationType::Full => RevalidationStrategy::Always, } )), - ready_poll: Default::default(), + ready_poll: Arc::new(Mutex::new(ReadyPoll::new(best_block_number))), metrics: PrometheusMetrics::new(prometheus), } } @@ -309,21 +317,29 @@ impl TransactionPool for BasicPool } fn ready_at(&self, at: NumberFor) -> PolledIterator { + let status = self.status(); + // If there are no transactions in the pool, it is fine to return early. + // + // There could be transaction being added because of some re-org happening at the relevant + // block, but this is relative unlikely. + if status.ready == 0 && status.future == 0 { + return async { Box::new(std::iter::empty()) as Box<_> }.boxed() + } + if self.ready_poll.lock().updated_at() >= at { log::trace!(target: "txpool", "Transaction pool already processed block #{}", at); let iterator: ReadyIteratorFor = Box::new(self.pool.validated_pool().ready()); - return Box::pin(futures::future::ready(iterator)); + return async move { iterator }.boxed(); } - Box::pin( - self.ready_poll - .lock() - .add(at) - .map(|received| received.unwrap_or_else(|e| { - log::warn!("Error receiving pending set: {:?}", e); - Box::new(vec![].into_iter()) - })) - ) + self.ready_poll + .lock() + .add(at) + .map(|received| received.unwrap_or_else(|e| { + log::warn!("Error receiving pending set: {:?}", e); + Box::new(std::iter::empty()) + })) + .boxed() } fn ready(&self) -> ReadyIteratorFor { @@ -334,7 +350,7 @@ impl TransactionPool for BasicPool impl LightPool where Block: BlockT, - Client: sp_blockchain::HeaderBackend + 'static, + Client: sp_blockchain::HeaderBackend + sc_client_api::UsageProvider + 'static, Fetcher: sc_client_api::Fetcher + 'static, { /// Create new basic transaction pool for a light node with the provided api. @@ -345,9 +361,15 @@ where client: Arc, fetcher: Arc, ) -> Self { - let pool_api = Arc::new(LightChainApi::new(client, fetcher)); + let pool_api = Arc::new(LightChainApi::new(client.clone(), fetcher)); Self::with_revalidation_type( - options, false.into(), pool_api, prometheus, RevalidationType::Light, spawner, + options, + false.into(), + pool_api, + prometheus, + RevalidationType::Light, + spawner, + client.usage_info().chain.best_number, ) } } @@ -357,8 +379,12 @@ where Block: BlockT, Client: sp_api::ProvideRuntimeApi + sc_client_api::BlockBackend - + sp_runtime::traits::BlockIdTo, - Client: sc_client_api::ExecutorProvider + Send + Sync + 'static, + + sp_runtime::traits::BlockIdTo + + sc_client_api::ExecutorProvider + + sc_client_api::UsageProvider + + Send + + Sync + + 'static, Client::Api: sp_transaction_pool::runtime_api::TaggedTransactionQueue, { /// Create new basic transaction pool for a full node with the provided api. @@ -371,7 +397,13 @@ where ) -> Arc { let pool_api = Arc::new(FullChainApi::new(client.clone(), prometheus)); let pool = Arc::new(Self::with_revalidation_type( - options, is_validator, pool_api, prometheus, RevalidationType::Full, spawner + options, + is_validator, + pool_api, + prometheus, + RevalidationType::Full, + spawner, + client.usage_info().chain.best_number, )); // make transaction pool available for off-chain runtime calls.