Skip to content

Commit 9991b1a

Browse files
alexgghbkchr
andauthored
Make SharedTrieCache/LocalTrieCache work with entire state in memory (#7682)
Part of #6131 (comment), we need to make sure that the TrieCache is able to work with the assumption that more or less the entire current state fits into memory, this can be split into a few parts: - [x] Remove the LocalTrieCache hard coded size and make it configurable from outside. - [x] Decided to have two flavours of building the LocalTrieCache, for the import and block authoring we are using a trusted local cache that grow to hold everything in the block and then propagated everything into the shared trie cache. - ~[x] Everything from LocalTrieCache needs to be propagated back to the SharedTrieCache in a fast and safe manner, so that it doesn't become a bottleneck. Currently, that is done on drop, with the function holding the ShareTrieCache write lock while promoting all keys. Decided to move this on a separate worker thread, so that the hot-path does not have to wait for drop function to propagate everything from the LocalTrieCache to the SharedTrieCache, the flushing of the worker thread happens after the block production and import happens.~ Update this is not need because authoring and imports is bounded, so the numbers is not exaggeratedly big, to make the drop too heavy, see numbers here #7682 (comment). # Todo - [x] Stats to prove most of reads/writes hit the shared or the local trie cache. - [x] Unit testing the new changes. Fixes: #7534 --------- Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io> Co-authored-by: Bastian Köcher <git@kchr.de>
1 parent fc3a207 commit 9991b1a

27 files changed

Lines changed: 953 additions & 151 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cumulus/client/relay-chain-inprocess-interface/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ use polkadot_service::{
4343
use sc_cli::{RuntimeVersion, SubstrateCli};
4444
use sc_client_api::{
4545
blockchain::BlockStatus, Backend, BlockchainEvents, HeaderBackend, ImportNotifications,
46-
StorageProof,
46+
StorageProof, TrieCacheContext,
4747
};
4848
use sc_network::{
4949
config::NetworkBackendType,
@@ -225,7 +225,7 @@ impl RelayChainInterface for RelayChainInProcessInterface {
225225
relay_parent: PHash,
226226
key: &[u8],
227227
) -> RelayChainResult<Option<StorageValue>> {
228-
let state = self.backend.state_at(relay_parent)?;
228+
let state = self.backend.state_at(relay_parent, TrieCacheContext::Untrusted)?;
229229
state.storage(key).map_err(RelayChainError::GenericError)
230230
}
231231

@@ -234,7 +234,7 @@ impl RelayChainInterface for RelayChainInProcessInterface {
234234
relay_parent: PHash,
235235
relevant_keys: &Vec<Vec<u8>>,
236236
) -> RelayChainResult<StorageProof> {
237-
let state_backend = self.backend.state_at(relay_parent)?;
237+
let state_backend = self.backend.state_at(relay_parent, TrieCacheContext::Untrusted)?;
238238

239239
sp_state_machine::prove_read(state_backend, relevant_keys)
240240
.map_err(RelayChainError::StateMachineError)

cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -198,11 +198,11 @@ mod tests {
198198
let recorder_for_test: SizeOnlyRecorderProvider<sp_core::Blake2Hasher> =
199199
SizeOnlyRecorderProvider::default();
200200
let reference_cache: SharedTrieCache<sp_core::Blake2Hasher> =
201-
SharedTrieCache::new(CacheSize::new(1024 * 5));
201+
SharedTrieCache::new(CacheSize::new(1024 * 5), None);
202202
let cache_for_test: SharedTrieCache<sp_core::Blake2Hasher> =
203-
SharedTrieCache::new(CacheSize::new(1024 * 5));
203+
SharedTrieCache::new(CacheSize::new(1024 * 5), None);
204204
{
205-
let local_cache = cache_for_test.local_cache();
205+
let local_cache = cache_for_test.local_cache_untrusted();
206206
let mut trie_cache_for_reference = local_cache.as_trie_db_cache(root);
207207
let mut reference_trie_recorder = reference_recorder.as_trie_recorder(root);
208208
let reference_trie =
@@ -211,7 +211,7 @@ mod tests {
211211
.with_cache(&mut trie_cache_for_reference)
212212
.build();
213213

214-
let local_cache_for_test = reference_cache.local_cache();
214+
let local_cache_for_test = reference_cache.local_cache_untrusted();
215215
let mut trie_cache_for_test = local_cache_for_test.as_trie_db_cache(root);
216216
let mut trie_recorder_under_test = recorder_for_test.as_trie_recorder(root);
217217
let test_trie =

prdoc/pr_7682.prdoc

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
title: Make SharedTrieCache/LocalTrieCache work with entire state in memory
2+
doc:
3+
- audience: Node Dev
4+
description: |-
5+
Extended LocalTrieCache with a trusted configuration that expands to hold everything in memory and
6+
then propagates all the data back to the SharedTrieCache. This new configuration is used on authoring
7+
and import paths.
8+
crates:
9+
- name: sp-state-machine
10+
bump: major
11+
- name: sp-trie
12+
bump: major
13+
- name: sc-client-api
14+
bump: major
15+
- name: sc-client-db
16+
bump: major
17+
- name: cumulus-pallet-parachain-system
18+
bump: major
19+
- name: sc-cli
20+
bump: major
21+
- name: sc-service
22+
bump: major
23+
- name: frame-benchmarking-cli
24+
bump: major
25+
- name: substrate-state-trie-migration-rpc
26+
bump: major
27+
- name: sc-consensus-beefy
28+
bump: major
29+
- name: sc-basic-authorship
30+
bump: major
31+
- name: cumulus-relay-chain-inprocess-interface
32+
bump: major

substrate/bin/node/testing/src/bench.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ impl BenchDb {
389389
state_pruning: Some(PruningMode::ArchiveAll),
390390
source: database_type.into_settings(dir.into()),
391391
blocks_pruning: sc_client_db::BlocksPruning::KeepAll,
392+
metrics_registry: None,
392393
};
393394
let task_executor = TaskExecutor::new();
394395

substrate/client/api/src/backend.rs

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use std::collections::HashSet;
2222

2323
use parking_lot::RwLock;
2424

25+
use sp_api::CallContext;
2526
use sp_consensus::BlockOrigin;
2627
use sp_core::offchain::OffchainStorage;
2728
use sp_runtime::{
@@ -492,6 +493,34 @@ pub trait StorageProvider<Block: BlockT, B: Backend<Block>> {
492493
) -> sp_blockchain::Result<Option<MerkleValue<Block::Hash>>>;
493494
}
494495

496+
/// Specify the desired trie cache context when calling [`Backend::state_at`].
497+
///
498+
/// This is used to determine the size of the local trie cache.
499+
#[derive(Debug, Clone, Copy)]
500+
pub enum TrieCacheContext {
501+
/// This is used when calling [`Backend::state_at`] in a trusted context.
502+
///
503+
/// A trusted context is for example the building or importing of a block.
504+
/// In this case the local trie cache can grow unlimited and all the cached data
505+
/// will be propagated back to the shared trie cache. It is safe to let the local
506+
/// cache grow to hold the entire data, because importing and building blocks is
507+
/// bounded by the block size limit.
508+
Trusted,
509+
/// This is used when calling [`Backend::state_at`] in from untrusted context.
510+
///
511+
/// The local trie cache will be bounded by its preconfigured size.
512+
Untrusted,
513+
}
514+
515+
impl From<CallContext> for TrieCacheContext {
516+
fn from(call_context: CallContext) -> Self {
517+
match call_context {
518+
CallContext::Onchain => TrieCacheContext::Trusted,
519+
CallContext::Offchain => TrieCacheContext::Untrusted,
520+
}
521+
}
522+
}
523+
495524
/// Client backend.
496525
///
497526
/// Manages the data layer.
@@ -584,11 +613,15 @@ pub trait Backend<Block: BlockT>: AuxStore + Send + Sync {
584613

585614
/// Returns true if state for given block is available.
586615
fn have_state_at(&self, hash: Block::Hash, _number: NumberFor<Block>) -> bool {
587-
self.state_at(hash).is_ok()
616+
self.state_at(hash, TrieCacheContext::Untrusted).is_ok()
588617
}
589618

590619
/// Returns state backend with post-state of given block.
591-
fn state_at(&self, hash: Block::Hash) -> sp_blockchain::Result<Self::State>;
620+
fn state_at(
621+
&self,
622+
hash: Block::Hash,
623+
trie_cache_context: TrieCacheContext,
624+
) -> sp_blockchain::Result<Self::State>;
592625

593626
/// Attempts to revert the chain by `n` blocks. If `revert_finalized` is set it will attempt to
594627
/// revert past any finalized block, this is unsafe and can potentially leave the node in an

substrate/client/api/src/in_mem.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ use crate::{
4242
backend::{self, NewBlockState},
4343
blockchain::{self, BlockStatus, HeaderBackend},
4444
leaves::LeafSet,
45-
UsageInfo,
45+
TrieCacheContext, UsageInfo,
4646
};
4747

4848
struct PendingBlock<B: BlockT> {
@@ -652,7 +652,7 @@ impl<Block: BlockT> backend::Backend<Block> for Backend<Block> {
652652
type OffchainStorage = OffchainStorage;
653653

654654
fn begin_operation(&self) -> sp_blockchain::Result<Self::BlockImportOperation> {
655-
let old_state = self.state_at(Default::default())?;
655+
let old_state = self.state_at(Default::default(), TrieCacheContext::Untrusted)?;
656656
Ok(BlockImportOperation {
657657
pending_block: None,
658658
old_state,
@@ -668,7 +668,7 @@ impl<Block: BlockT> backend::Backend<Block> for Backend<Block> {
668668
operation: &mut Self::BlockImportOperation,
669669
block: Block::Hash,
670670
) -> sp_blockchain::Result<()> {
671-
operation.old_state = self.state_at(block)?;
671+
operation.old_state = self.state_at(block, TrieCacheContext::Untrusted)?;
672672
Ok(())
673673
}
674674

@@ -734,7 +734,11 @@ impl<Block: BlockT> backend::Backend<Block> for Backend<Block> {
734734
None
735735
}
736736

737-
fn state_at(&self, hash: Block::Hash) -> sp_blockchain::Result<Self::State> {
737+
fn state_at(
738+
&self,
739+
hash: Block::Hash,
740+
_trie_cache_context: TrieCacheContext,
741+
) -> sp_blockchain::Result<Self::State> {
738742
if hash == Default::default() {
739743
return Ok(Self::State::default())
740744
}

substrate/client/basic-authorship/src/basic_authorship.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,7 @@ mod tests {
606606

607607
use futures::executor::block_on;
608608
use parking_lot::Mutex;
609-
use sc_client_api::Backend;
609+
use sc_client_api::{Backend, TrieCacheContext};
610610
use sc_transaction_pool::BasicPool;
611611
use sc_transaction_pool_api::{ChainEvent, MaintainedTransactionPool, TransactionSource};
612612
use sp_api::Core;
@@ -777,7 +777,7 @@ mod tests {
777777
let api = client.runtime_api();
778778
api.execute_block(genesis_hash, proposal.block).unwrap();
779779

780-
let state = backend.state_at(genesis_hash).unwrap();
780+
let state = backend.state_at(genesis_hash, TrieCacheContext::Untrusted).unwrap();
781781

782782
let storage_changes = api.into_storage_changes(&state, genesis_hash).unwrap();
783783

substrate/client/cli/src/commands/chain_info_cmd.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ impl ChainInfoCmd {
7777
state_pruning: config.state_pruning.clone(),
7878
source: config.database.clone(),
7979
blocks_pruning: config.blocks_pruning,
80+
metrics_registry: None,
8081
};
8182
let backend = sc_service::new_db_backend::<B>(db_config)?;
8283
let info: ChainInfo<B> = backend.blockchain().info().into();

substrate/client/consensus/beefy/src/import.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use sp_runtime::{
2828
EncodedJustification,
2929
};
3030

31-
use sc_client_api::backend::Backend;
31+
use sc_client_api::{backend::Backend, TrieCacheContext};
3232
use sc_consensus::{BlockCheckParams, BlockImport, BlockImportParams, ImportResult};
3333

3434
use crate::{
@@ -149,7 +149,7 @@ where
149149
// Run inner block import.
150150
let inner_import_result = self.inner.import_block(block).await?;
151151

152-
match self.backend.state_at(hash) {
152+
match self.backend.state_at(hash, TrieCacheContext::Untrusted) {
153153
Ok(_) => {},
154154
Err(_) => {
155155
// The block is imported as part of some chain sync.

0 commit comments

Comments
 (0)