From e949c126202e17a004c06394da92c60fd05bedaa Mon Sep 17 00:00:00 2001 From: arkpar Date: Mon, 9 Nov 2020 14:39:16 +0100 Subject: [PATCH 1/4] Remove header query --- client/service/src/client/client.rs | 4 ++-- primitives/blockchain/src/backend.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/service/src/client/client.rs b/client/service/src/client/client.rs index d423fdee39b6c..e8d748011bc17 100644 --- a/client/service/src/client/client.rs +++ b/client/service/src/client/client.rs @@ -1159,12 +1159,12 @@ impl Client where /// Prepare in-memory header that is used in execution environment. fn prepare_environment_block(&self, parent: &BlockId) -> sp_blockchain::Result { - let parent_header = self.backend.blockchain().expect_header(*parent)?; + let parent_hash = self.backend.blockchain().expect_block_hash_from_id(parent)?; Ok(<::Header as HeaderT>::new( self.backend.blockchain().expect_block_number_from_id(parent)? + One::one(), Default::default(), Default::default(), - parent_header.hash(), + parent_hash, Default::default(), )) } diff --git a/primitives/blockchain/src/backend.rs b/primitives/blockchain/src/backend.rs index 1328dfb5752fc..326acd6b9bd43 100644 --- a/primitives/blockchain/src/backend.rs +++ b/primitives/blockchain/src/backend.rs @@ -53,7 +53,7 @@ pub trait HeaderBackend: Send + Sync { /// Convert an arbitrary block ID into a block hash. fn block_number_from_id(&self, id: &BlockId) -> Result>> { match *id { - BlockId::Hash(_) => Ok(self.header(*id)?.map(|h| h.number().clone())), + BlockId::Hash(h) => self.number(h), BlockId::Number(n) => Ok(Some(n)), } } From 90d9197c38a60242c4819e784352d28819a46b02 Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 10 Nov 2020 13:55:00 +0100 Subject: [PATCH 2/4] Header cache --- client/db/src/lib.rs | 57 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 11 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index 8196a750557a8..1182e38117318 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -49,6 +49,9 @@ use std::sync::Arc; use std::path::{Path, PathBuf}; use std::io; use std::collections::{HashMap, HashSet}; +use parking_lot::{Mutex, RwLock}; +use linked_hash_map::LinkedHashMap; +use log::{trace, debug, warn}; use sc_client_api::{ UsageInfo, MemoryInfo, IoInfo, MemorySize, @@ -63,7 +66,6 @@ use codec::{Decode, Encode}; use hash_db::Prefix; use sp_trie::{MemoryDB, PrefixedMemoryDB, prefixed_key}; use sp_database::Transaction; -use parking_lot::RwLock; use sp_core::ChangesTrieConfiguration; use sp_core::offchain::storage::{OffchainOverlayedChange, OffchainOverlayedChanges}; use sp_core::storage::{well_known_keys, ChildInfo}; @@ -83,7 +85,6 @@ use sc_state_db::StateDb; use sp_blockchain::{CachedHeaderMetadata, HeaderMetadata, HeaderMetadataCache}; use crate::storage_cache::{CachingState, SyncingCachingState, SharedCache, new_shared_cache}; use crate::stats::StateUsageStats; -use log::{trace, debug, warn}; // Re-export the Database trait so that one can pass an implementation of it. pub use sp_database::Database; @@ -93,6 +94,7 @@ pub use sc_state_db::PruningMode; pub use bench::BenchmarkingState; const MIN_BLOCKS_TO_KEEP_CHANGES_TRIES_FOR: u32 = 32768; +const CACHE_HEADERS: usize = 8; /// Default value for storage cache child ratio. const DEFAULT_CHILD_RATIO: (usize, usize) = (1, 10); @@ -358,6 +360,7 @@ pub struct BlockchainDb { meta: Arc, Block::Hash>>>, leaves: RwLock>>, header_metadata_cache: Arc>, + header_cache: Mutex>>, } impl BlockchainDb { @@ -369,6 +372,7 @@ impl BlockchainDb { leaves: RwLock::new(leaves), meta: Arc::new(RwLock::new(meta)), header_metadata_cache: Arc::new(HeaderMetadataCache::default()), + header_cache: Default::default(), }) } @@ -403,11 +407,38 @@ impl BlockchainDb { header.digest().log(DigestItem::as_changes_trie_root) .cloned())) } + + fn cache_header(&self, hash: Block::Hash, header: Option) { + let mut cache = self.header_cache.lock(); + cache.insert(hash, header); + while cache.len() > CACHE_HEADERS { + cache.pop_front(); + } + } } impl sc_client_api::blockchain::HeaderBackend for BlockchainDb { fn header(&self, id: BlockId) -> ClientResult> { - utils::read_header(&*self.db, columns::KEY_LOOKUP, columns::HEADER, id) + match &id { + BlockId::Hash(h) => { + { + let mut cache = self.header_cache.lock(); + if let Some(result) = cache.get_refresh(h) { + return Ok(result.clone()); + } + } + match utils::read_header(&*self.db, columns::KEY_LOOKUP, columns::HEADER, id) { + Ok(header) => { + self.cache_header(h.clone(), header.clone()); + Ok(header) + } + e @Err(_) => e, + } + } + BlockId::Number(_) => { + utils::read_header(&*self.db, columns::KEY_LOOKUP, columns::HEADER, id) + } + } } fn info(&self) -> sc_client_api::blockchain::Info { @@ -515,7 +546,7 @@ impl HeaderMetadata for BlockchainDb { } fn insert_header_metadata(&self, hash: Block::Hash, metadata: CachedHeaderMetadata) { - self.header_metadata_cache.insert_header_metadata(hash, metadata) + self.header_metadata_cache.insert_header_metadata(hash, metadata); } fn remove_header_metadata(&self, hash: Block::Hash) { @@ -1117,12 +1148,6 @@ impl Backend { hash, )?; - let header_metadata = CachedHeaderMetadata::from(&pending_block.header); - self.blockchain.insert_header_metadata( - header_metadata.hash, - header_metadata, - ); - transaction.set_from_vec(columns::HEADER, &lookup_key, pending_block.header.encode()); if let Some(body) = &pending_block.body { transaction.set_from_vec(columns::BODY, &lookup_key, body.encode()); @@ -1271,7 +1296,7 @@ impl Backend { meta_updates.push((hash, number, pending_block.leaf_state.is_best(), finalized)); - Some((number, hash, enacted, retracted, displaced_leaf, is_best, cache)) + Some((pending_block.header, number, hash, enacted, retracted, displaced_leaf, is_best, cache)) } else { None }; @@ -1297,7 +1322,11 @@ impl Backend { self.storage.db.commit(transaction)?; + // Apply all in-memory state shanges. + // Code beyond this point can't fail. + if let Some(( + header, number, hash, enacted, @@ -1306,6 +1335,12 @@ impl Backend { is_best, mut cache, )) = imported { + let header_metadata = CachedHeaderMetadata::from(&header); + self.blockchain.insert_header_metadata( + header_metadata.hash, + header_metadata, + ); + self.blockchain.cache_header(hash, Some(header)); cache.sync_cache( &enacted, &retracted, From b3ced925c49e7df68e15aaa69247774a1180a936 Mon Sep 17 00:00:00 2001 From: arkpar Date: Wed, 11 Nov 2020 12:10:24 +0100 Subject: [PATCH 3/4] Fix potential race issue --- client/db/src/lib.rs | 48 +++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index 1182e38117318..d280bff4d4972 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -354,6 +354,17 @@ impl<'a> sc_state_db::MetaDb for StateMetaDb<'a> { } } +fn cache_header( + cache: &mut LinkedHashMap>, + hash: Hash, + header: Option
+) { + cache.insert(hash, header); + while cache.len() > CACHE_HEADERS { + cache.pop_front(); + } +} + /// Block database pub struct BlockchainDb { db: Arc>, @@ -407,29 +418,19 @@ impl BlockchainDb { header.digest().log(DigestItem::as_changes_trie_root) .cloned())) } - - fn cache_header(&self, hash: Block::Hash, header: Option) { - let mut cache = self.header_cache.lock(); - cache.insert(hash, header); - while cache.len() > CACHE_HEADERS { - cache.pop_front(); - } - } } impl sc_client_api::blockchain::HeaderBackend for BlockchainDb { fn header(&self, id: BlockId) -> ClientResult> { match &id { BlockId::Hash(h) => { - { - let mut cache = self.header_cache.lock(); - if let Some(result) = cache.get_refresh(h) { - return Ok(result.clone()); - } + let mut cache = self.header_cache.lock(); + if let Some(result) = cache.get_refresh(h) { + return Ok(result.clone()); } match utils::read_header(&*self.db, columns::KEY_LOOKUP, columns::HEADER, id) { Ok(header) => { - self.cache_header(h.clone(), header.clone()); + cache_header(&mut cache, h.clone(), header.clone()); Ok(header) } e @Err(_) => e, @@ -455,12 +456,17 @@ impl sc_client_api::blockchain::HeaderBackend for Blockcha fn status(&self, id: BlockId) -> ClientResult { let exists = match id { - BlockId::Hash(_) => read_db( - &*self.db, - columns::KEY_LOOKUP, - columns::HEADER, - id - )?.is_some(), + BlockId::Hash(h) => match self.header_cache.lock().get(&h) { + Some(header) => header.is_some(), + None => { + read_db( + &*self.db, + columns::KEY_LOOKUP, + columns::HEADER, + id + )?.is_some() + } + } BlockId::Number(n) => n <= self.meta.read().best_number, }; match exists { @@ -1340,7 +1346,7 @@ impl Backend { header_metadata.hash, header_metadata, ); - self.blockchain.cache_header(hash, Some(header)); + cache_header(&mut self.blockchain.header_cache.lock(), hash, Some(header)); cache.sync_cache( &enacted, &retracted, From 5e14df4a9dd9c6c926522aa7a0bfbac2885fdf3b Mon Sep 17 00:00:00 2001 From: arkpar Date: Wed, 11 Nov 2020 14:24:11 +0100 Subject: [PATCH 4/4] Simplify status query --- client/db/src/lib.rs | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index d280bff4d4972..983459cfebe33 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -357,7 +357,7 @@ impl<'a> sc_state_db::MetaDb for StateMetaDb<'a> { fn cache_header( cache: &mut LinkedHashMap>, hash: Hash, - header: Option
+ header: Option
, ) { cache.insert(hash, header); while cache.len() > CACHE_HEADERS { @@ -428,13 +428,9 @@ impl sc_client_api::blockchain::HeaderBackend for Blockcha if let Some(result) = cache.get_refresh(h) { return Ok(result.clone()); } - match utils::read_header(&*self.db, columns::KEY_LOOKUP, columns::HEADER, id) { - Ok(header) => { - cache_header(&mut cache, h.clone(), header.clone()); - Ok(header) - } - e @Err(_) => e, - } + let header = utils::read_header(&*self.db, columns::KEY_LOOKUP, columns::HEADER, id)?; + cache_header(&mut cache, h.clone(), header.clone()); + Ok(header) } BlockId::Number(_) => { utils::read_header(&*self.db, columns::KEY_LOOKUP, columns::HEADER, id) @@ -456,17 +452,7 @@ impl sc_client_api::blockchain::HeaderBackend for Blockcha fn status(&self, id: BlockId) -> ClientResult { let exists = match id { - BlockId::Hash(h) => match self.header_cache.lock().get(&h) { - Some(header) => header.is_some(), - None => { - read_db( - &*self.db, - columns::KEY_LOOKUP, - columns::HEADER, - id - )?.is_some() - } - } + BlockId::Hash(_) => self.header(id)?.is_some(), BlockId::Number(n) => n <= self.meta.read().best_number, }; match exists { @@ -552,7 +538,7 @@ impl HeaderMetadata for BlockchainDb { } fn insert_header_metadata(&self, hash: Block::Hash, metadata: CachedHeaderMetadata) { - self.header_metadata_cache.insert_header_metadata(hash, metadata); + self.header_metadata_cache.insert_header_metadata(hash, metadata) } fn remove_header_metadata(&self, hash: Block::Hash) {