From 835edbe5ae9fa466caee8e339bf74a257570c9c4 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Tue, 11 Jun 2024 15:02:11 +0200 Subject: [PATCH] finalization: Skip tree route calculation if no forks present (#4721) ## Issue Currently, syncing parachains from scratch can lead to a very long finalization time once they reach the tip of the chain. The problem is that we try to finalize everything from 0 to the tip, which can be thousands or even millions of blocks. We finalize sequentially and try to compute displaced branches during finalization. So for every block on the way, we compute an expensive tree route. ## Proposed Improvements In this PR, I propose improvements that solve this situation: - **Skip tree route calculation if `leaves().len() == 1`:** This should be enough for 90% of cases where there is only one leaf after sync. - **Optimize finalization for long distances:** It can happen that the parachain has imported some leaf and then receives a relay chain notification with the finalized block. In that case, the previous optimization will not trigger. A second mechanism should ensure that we do not need to compute the full tree route. If the finalization distance is long, we check the lowest common ancestor of all the leaves. If it is above the to-be-finalized block, we know that there are no displaced leaves. This is fast because forks are short and close to the tip, so we can leverage the header cache. ## Alternative Approach - The problem was introduced in #3962. Reverting that PR is another possible strategy. - We could store for every fork where it begins, however sounds a bit more involved to me. fixes #4614 --- prdoc/pr_4721.prdoc | 19 +++ substrate/client/db/src/lib.rs | 121 +++++++++++++++++- .../primitives/blockchain/src/backend.rs | 40 +++++- .../blockchain/src/header_metadata.rs | 26 +++- 4 files changed, 198 insertions(+), 8 deletions(-) create mode 100644 prdoc/pr_4721.prdoc diff --git a/prdoc/pr_4721.prdoc b/prdoc/pr_4721.prdoc new file mode 100644 index 0000000000000..730ac4d830869 --- /dev/null +++ b/prdoc/pr_4721.prdoc @@ -0,0 +1,19 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Skip tree route calculation if no forks present + +doc: + - audience: Node Operator + description: | + Fixes an issue with synchronisation on parachains. Once they reached the tip of the chain, + nodes would show `Preparing 0.0 bps`. This is shown because the node is blocked on calculating + the tree route from genesis to the tip of the chain many times. This PR solves that by skipping + tree route calculation if there is only one leave. In addition, further optimizations have been + done to alleviate long finalization distances. + +crates: + - name: sp-blockchain + bump: minor + - name: sc-client-db + bump: none diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index 36f9aea817c9c..8d8b7a2aff88f 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -2547,7 +2547,7 @@ pub(crate) mod tests { backend::{Backend as BTrait, BlockImportOperation as Op}, blockchain::Backend as BLBTrait, }; - use sp_blockchain::{lowest_common_ancestor, tree_route}; + use sp_blockchain::{lowest_common_ancestor, lowest_common_ancestor_multiblock, tree_route}; use sp_core::H256; use sp_runtime::{ testing::{Block as RawBlock, ExtrinsicWrapper, Header}, @@ -3108,6 +3108,125 @@ pub(crate) mod tests { } } + #[test] + fn lowest_common_ancestors_multiblock_works() { + let backend = Backend::::new_test(1000, 100); + let blockchain = backend.blockchain(); + let block0 = insert_header(&backend, 0, Default::default(), None, Default::default()); + + // fork from genesis: 3 prong. + // block 0 -> a1 -> a2 -> a3 + // | + // -> b1 -> b2 -> c1 -> c2 + // | + // -> d1 -> d2 + let a1 = insert_header(&backend, 1, block0, None, Default::default()); + let a2 = insert_header(&backend, 2, a1, None, Default::default()); + let a3 = insert_header(&backend, 3, a2, None, Default::default()); + + // fork from genesis: 2 prong. + let b1 = insert_header(&backend, 1, block0, None, H256::from([1; 32])); + let b2 = insert_header(&backend, 2, b1, None, Default::default()); + + // fork from b2. + let c1 = insert_header(&backend, 3, b2, None, H256::from([2; 32])); + let c2 = insert_header(&backend, 4, c1, None, Default::default()); + + // fork from b1. + let d1 = insert_header(&backend, 2, b1, None, H256::from([3; 32])); + let d2 = insert_header(&backend, 3, d1, None, Default::default()); + { + let lca = lowest_common_ancestor_multiblock(blockchain, vec![a3, b2]).unwrap().unwrap(); + + assert_eq!(lca.hash, block0); + assert_eq!(lca.number, 0); + } + + { + let lca = lowest_common_ancestor_multiblock(blockchain, vec![a1, a3]).unwrap().unwrap(); + + assert_eq!(lca.hash, a1); + assert_eq!(lca.number, 1); + } + + { + let lca = lowest_common_ancestor_multiblock(blockchain, vec![a3, a1]).unwrap().unwrap(); + + assert_eq!(lca.hash, a1); + assert_eq!(lca.number, 1); + } + + { + let lca = lowest_common_ancestor_multiblock(blockchain, vec![a2, a3]).unwrap().unwrap(); + + assert_eq!(lca.hash, a2); + assert_eq!(lca.number, 2); + } + + { + let lca = lowest_common_ancestor_multiblock(blockchain, vec![a2, a1]).unwrap().unwrap(); + + assert_eq!(lca.hash, a1); + assert_eq!(lca.number, 1); + } + + { + let lca = lowest_common_ancestor_multiblock(blockchain, vec![a2, a2]).unwrap().unwrap(); + + assert_eq!(lca.hash, a2); + assert_eq!(lca.number, 2); + } + + { + let lca = lowest_common_ancestor_multiblock(blockchain, vec![a3, d2, c2]) + .unwrap() + .unwrap(); + + assert_eq!(lca.hash, block0); + assert_eq!(lca.number, 0); + } + + { + let lca = lowest_common_ancestor_multiblock(blockchain, vec![c2, d2, b2]) + .unwrap() + .unwrap(); + + assert_eq!(lca.hash, b1); + assert_eq!(lca.number, 1); + } + + { + let lca = lowest_common_ancestor_multiblock(blockchain, vec![a1, a2, a3]) + .unwrap() + .unwrap(); + + assert_eq!(lca.hash, a1); + assert_eq!(lca.number, 1); + } + + { + let lca = lowest_common_ancestor_multiblock(blockchain, vec![b1, b2, d1]) + .unwrap() + .unwrap(); + + assert_eq!(lca.hash, b1); + assert_eq!(lca.number, 1); + } + + { + let lca = lowest_common_ancestor_multiblock(blockchain, vec![]); + + assert_eq!(true, matches!(lca, Ok(None))); + } + + { + let lca = lowest_common_ancestor_multiblock(blockchain, vec![a1]).unwrap().unwrap(); + + assert_eq!(lca.hash, a1); + assert_eq!(lca.number, 1); + } + } + #[test] fn test_tree_route_regression() { // NOTE: this is a test for a regression introduced in #3665, the result diff --git a/substrate/primitives/blockchain/src/backend.rs b/substrate/primitives/blockchain/src/backend.rs index 933e41e2ab453..76393420da740 100644 --- a/substrate/primitives/blockchain/src/backend.rs +++ b/substrate/primitives/blockchain/src/backend.rs @@ -21,16 +21,15 @@ use log::warn; use parking_lot::RwLock; use sp_runtime::{ generic::BlockId, - traits::{Block as BlockT, Header as HeaderT, NumberFor, Zero}, + traits::{Block as BlockT, CheckedSub, Header as HeaderT, NumberFor, Zero}, Justifications, }; use std::collections::{btree_map::BTreeMap, btree_set::BTreeSet}; -use crate::header_metadata::HeaderMetadata; - use crate::{ error::{Error, Result}, - tree_route, TreeRoute, + header_metadata::{self, HeaderMetadata}, + lowest_common_ancestor_multiblock, tree_route, TreeRoute, }; /// Blockchain database header backend. Does not perform any validation. @@ -229,12 +228,41 @@ pub trait Backend: ) -> std::result::Result, Error> { let mut result = DisplacedLeavesAfterFinalization::default(); - if finalized_block_number == Zero::zero() { + let leaves = self.leaves()?; + + // If we have only one leaf there are no forks, and we can return early. + if finalized_block_number == Zero::zero() || leaves.len() == 1 { return Ok(result) } + let first_leaf = leaves.first().ok_or(Error::Backend( + "Unable to find any leaves. This should not happen.".to_string(), + ))?; + let leaf_block_header = self.expect_header(*first_leaf)?; + + // If the distance between the leafs and the finalized block is large, calculating + // tree routes can be very expensive. In that case, we will try to find the + // lowest common ancestor between all the leaves. The assumption here is that the forks are + // close to the tip and not long. So the LCA can be computed from the header cache. If the + // LCA is above the finalized block, we know that there are no displaced leaves by the + // finalization. + if leaf_block_header + .number() + .checked_sub(&finalized_block_number) + .unwrap_or(0u32.into()) > + header_metadata::LRU_CACHE_SIZE.into() + { + if let Some(lca) = lowest_common_ancestor_multiblock(self, leaves.clone())? { + if lca.number > finalized_block_number { + return Ok(result) + } else { + log::warn!("The distance between leafs and finalized block is large. Finalization can take a long time."); + } + }; + } + // For each leaf determine whether it belongs to a non-canonical branch. - for leaf_hash in self.leaves()? { + for leaf_hash in leaves { let leaf_block_header = self.expect_header(leaf_hash)?; let leaf_number = *leaf_block_header.number(); diff --git a/substrate/primitives/blockchain/src/header_metadata.rs b/substrate/primitives/blockchain/src/header_metadata.rs index 27caaae71add1..c2054445b0676 100644 --- a/substrate/primitives/blockchain/src/header_metadata.rs +++ b/substrate/primitives/blockchain/src/header_metadata.rs @@ -23,7 +23,7 @@ use schnellru::{ByLength, LruMap}; use sp_runtime::traits::{Block as BlockT, Header, NumberFor, One}; /// Set to the expected max difference between `best` and `finalized` blocks at sync. -const LRU_CACHE_SIZE: u32 = 5_000; +pub(crate) const LRU_CACHE_SIZE: u32 = 5_000; /// Get lowest common ancestor between two blocks in the tree. /// @@ -96,6 +96,30 @@ pub fn lowest_common_ancestor + ?Sized>( Ok(HashAndNumber { hash: header_one.hash, number: header_one.number }) } +/// Get lowest common ancestor between multiple blocks. +pub fn lowest_common_ancestor_multiblock + ?Sized>( + backend: &T, + hashes: Vec, +) -> Result>, T::Error> { + // Ensure the list of hashes is not empty + let mut hashes_iter = hashes.into_iter(); + + let first_hash = match hashes_iter.next() { + Some(hash) => hash, + None => return Ok(None), + }; + + // Start with the first hash as the initial LCA + let first_cached = backend.header_metadata(first_hash)?; + let mut lca = HashAndNumber { number: first_cached.number, hash: first_cached.hash }; + for hash in hashes_iter { + // Calculate the LCA of the current LCA and the next hash + lca = lowest_common_ancestor(backend, lca.hash, hash)?; + } + + Ok(Some(lca)) +} + /// Compute a tree-route between two blocks. See tree-route docs for more details. pub fn tree_route + ?Sized>( backend: &T,