From 46ddc72060bba7da3da7f15afe455e883f038321 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Gawrya=C5=82?= Date: Tue, 9 Jan 2024 16:12:34 +0100 Subject: [PATCH 1/7] Keep overlay level length --- substrate/client/state-db/src/lib.rs | 5 --- substrate/client/state-db/src/noncanonical.rs | 35 +++++++++++-------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/substrate/client/state-db/src/lib.rs b/substrate/client/state-db/src/lib.rs index c656f126ae6eb..677f0a5910b03 100644 --- a/substrate/client/state-db/src/lib.rs +++ b/substrate/client/state-db/src/lib.rs @@ -135,8 +135,6 @@ pub enum StateDbError { InvalidParent, /// Invalid pruning mode specified. Contains expected mode. IncompatiblePruningModes { stored: PruningMode, requested: PruningMode }, - /// Too many unfinalized sibling blocks inserted. - TooManySiblingBlocks { number: u64 }, /// Trying to insert existing block. BlockAlreadyExists, /// Invalid metadata @@ -187,9 +185,6 @@ impl fmt::Debug for StateDbError { "Incompatible pruning modes [stored: {:?}; requested: {:?}]", stored, requested ), - Self::TooManySiblingBlocks { number } => { - write!(f, "Too many sibling blocks at #{number} inserted") - }, Self::BlockAlreadyExists => write!(f, "Block already exists"), Self::Metadata(message) => write!(f, "Invalid metadata: {}", message), Self::BlockUnavailable => { diff --git a/substrate/client/state-db/src/noncanonical.rs b/substrate/client/state-db/src/noncanonical.rs index bdbe8318371ce..6068714a59398 100644 --- a/substrate/client/state-db/src/noncanonical.rs +++ b/substrate/client/state-db/src/noncanonical.rs @@ -28,8 +28,8 @@ use log::trace; use std::collections::{hash_map::Entry, HashMap, VecDeque}; const NON_CANONICAL_JOURNAL: &[u8] = b"noncanonical_journal"; +const NON_CANONICAL_JOURNAL_LEN: &[u8] = b"noncanonical_journal_len"; pub(crate) const LAST_CANONICAL: &[u8] = b"last_canonical"; -const MAX_BLOCKS_PER_LEVEL: u64 = 32; /// See module documentation. pub struct NonCanonicalOverlay { @@ -64,6 +64,10 @@ impl OverlayLevel { self.blocks.remove(index) } + fn level_width(&self) -> u64 { + 64 - self.used_indicies.leading_zeros() as u64 + } + fn new() -> OverlayLevel { OverlayLevel { blocks: Vec::new(), used_indicies: 0 } } @@ -190,7 +194,13 @@ impl NonCanonicalOverlay { block += 1; loop { let mut level = OverlayLevel::new(); - for index in 0..MAX_BLOCKS_PER_LEVEL { + let non_canonical_journal_len_record = db.get_meta(&to_meta_key(NON_CANONICAL_JOURNAL_LEN, &block)) + .map_err(Error::Db)?; + let level_len = match non_canonical_journal_len_record { + Some(data) => Decode::decode(&mut data.as_slice())?, + None => 0, + }; + for index in 0..level_len { let journal_key = to_journal_key(block, index); if let Some(record) = db.get_meta(&journal_key).map_err(Error::Db)? { let record: JournalRecord = @@ -241,8 +251,8 @@ impl NonCanonicalOverlay { }) } - /// Insert a new block into the overlay. If inserted on the second level or lover expects parent - /// to be present in the window. + /// Insert a new block into the overlay. If inserted on the second level or lower, + /// expects parent to be present in the window. pub fn insert( &mut self, hash: &BlockHash, @@ -295,19 +305,16 @@ impl NonCanonicalOverlay { .expect("number is [front_block_number .. front_block_number + levels.len()) is asserted in precondition; qed") }; - if level.blocks.len() >= MAX_BLOCKS_PER_LEVEL as usize { - trace!( - target: LOG_TARGET, - "Too many sibling blocks at #{number}: {:?}", - level.blocks.iter().map(|b| &b.hash).collect::>() - ); - return Err(StateDbError::TooManySiblingBlocks { number }) - } if level.blocks.iter().any(|b| b.hash == *hash) { return Err(StateDbError::BlockAlreadyExists) } let index = level.available_index(); + if index == level.level_width() { + let key = to_meta_key(NON_CANONICAL_JOURNAL_LEN, &number); + let journal_len_at_number = index+1; + commit.meta.inserted.push((key, journal_len_at_number.encode())); + } let journal_key = to_journal_key(number, index); let inserted = changeset.inserted.iter().map(|(k, _)| k.clone()).collect(); @@ -666,7 +673,7 @@ mod tests { let insertion = overlay.insert(&h1, 1, &H256::default(), changeset.clone()).unwrap(); assert_eq!(insertion.data.inserted.len(), 0); assert_eq!(insertion.data.deleted.len(), 0); - assert_eq!(insertion.meta.inserted.len(), 2); + assert_eq!(insertion.meta.inserted.len(), 3); assert_eq!(insertion.meta.deleted.len(), 0); db.commit(&insertion); let mut finalization = CommitSet::default(); @@ -691,7 +698,7 @@ mod tests { .unwrap(), ); db.commit(&overlay.insert(&h2, 11, &h1, make_changeset(&[5], &[3])).unwrap()); - assert_eq!(db.meta_len(), 3); + assert_eq!(db.meta_len(), 5); let overlay2 = NonCanonicalOverlay::::new(&db).unwrap(); assert_eq!(overlay.levels, overlay2.levels); From 07bfd74dfeae6a1f5590ca2e2cfc4ca287fce924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Gawrya=C5=82?= Date: Wed, 10 Jan 2024 15:16:28 +0100 Subject: [PATCH 2/7] Remove journal level length db entries --- substrate/client/state-db/src/noncanonical.rs | 44 ++++++++++++++++--- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/substrate/client/state-db/src/noncanonical.rs b/substrate/client/state-db/src/noncanonical.rs index 6068714a59398..baa214e51f443 100644 --- a/substrate/client/state-db/src/noncanonical.rs +++ b/substrate/client/state-db/src/noncanonical.rs @@ -194,7 +194,8 @@ impl NonCanonicalOverlay { block += 1; loop { let mut level = OverlayLevel::new(); - let non_canonical_journal_len_record = db.get_meta(&to_meta_key(NON_CANONICAL_JOURNAL_LEN, &block)) + let non_canonical_journal_len_record = db + .get_meta(&to_meta_key(NON_CANONICAL_JOURNAL_LEN, &block)) .map_err(Error::Db)?; let level_len = match non_canonical_journal_len_record { Some(data) => Decode::decode(&mut data.as_slice())?, @@ -312,7 +313,7 @@ impl NonCanonicalOverlay { let index = level.available_index(); if index == level.level_width() { let key = to_meta_key(NON_CANONICAL_JOURNAL_LEN, &number); - let journal_len_at_number = index+1; + let journal_len_at_number = index + 1; commit.meta.inserted.push((key, journal_len_at_number.encode())); } let journal_key = to_journal_key(number, index); @@ -454,6 +455,10 @@ impl NonCanonicalOverlay { discarded_journals.push(overlay.journal_key.clone()); } commit.meta.deleted.append(&mut discarded_journals); + commit + .meta + .deleted + .push(to_meta_key(NON_CANONICAL_JOURNAL_LEN, &self.front_block_number())); let canonicalized = (hash.clone(), self.front_block_number()); commit @@ -484,6 +489,7 @@ impl NonCanonicalOverlay { /// Revert a single level. Returns commit set that deletes the journal or `None` if not /// possible. pub fn revert_one(&mut self) -> Option> { + let last_block_number = self.front_block_number() + self.levels.len() as u64 - 1; self.levels.pop_back().map(|level| { let mut commit = CommitSet::default(); for overlay in level.blocks.into_iter() { @@ -491,6 +497,8 @@ impl NonCanonicalOverlay { self.parents.remove(&overlay.hash); discard_values(&mut self.values, overlay.inserted); } + let journal_level_len_key = to_meta_key(NON_CANONICAL_JOURNAL_LEN, &last_block_number); + commit.meta.deleted.push(journal_level_len_key); commit }) } @@ -517,7 +525,10 @@ impl NonCanonicalOverlay { break } if self.levels.back().map_or(false, |l| l.blocks.is_empty()) { + let last_block_number = self.front_block_number() + level_count as u64 - 1; self.levels.pop_back(); + let journal_level_len_key = to_meta_key(NON_CANONICAL_JOURNAL_LEN, &last_block_number); + commit.meta.deleted.push(journal_level_len_key); } if !commit.meta.deleted.is_empty() { Some(commit) @@ -579,10 +590,13 @@ impl NonCanonicalOverlay { mod tests { use super::{to_journal_key, NonCanonicalOverlay}; use crate::{ + noncanonical::{LAST_CANONICAL, NON_CANONICAL_JOURNAL_LEN}, test::{make_changeset, make_db}, - ChangeSet, CommitSet, MetaDb, StateDbError, + to_meta_key, ChangeSet, CommitSet, MetaDb, StateDbError, }; use sp_core::H256; + use std::collections::HashSet; + use codec::Encode; fn contains(overlay: &NonCanonicalOverlay, key: u64) -> bool { overlay.get(&H256::from_low_u64_be(key)) == @@ -673,15 +687,31 @@ mod tests { let insertion = overlay.insert(&h1, 1, &H256::default(), changeset.clone()).unwrap(); assert_eq!(insertion.data.inserted.len(), 0); assert_eq!(insertion.data.deleted.len(), 0); - assert_eq!(insertion.meta.inserted.len(), 3); - assert_eq!(insertion.meta.deleted.len(), 0); + assert_eq!( + insertion.meta.inserted.iter().map(|(k, _)| k).collect::>(), + HashSet::from_iter(&[ + to_meta_key(LAST_CANONICAL, &()), + to_journal_key(1, 0), + to_meta_key(&NON_CANONICAL_JOURNAL_LEN, &1u64), + ]) + ); + assert!(insertion.meta.deleted.is_empty()); db.commit(&insertion); let mut finalization = CommitSet::default(); overlay.canonicalize(&h1, &mut finalization).unwrap(); assert_eq!(finalization.data.inserted.len(), changeset.inserted.len()); assert_eq!(finalization.data.deleted.len(), changeset.deleted.len()); - assert_eq!(finalization.meta.inserted.len(), 1); - assert_eq!(finalization.meta.deleted.len(), 1); + assert_eq!( + finalization.meta.inserted, + vec![(to_meta_key(LAST_CANONICAL, &()),(&h1, 1u64).encode())] + ); + assert_eq!( + HashSet::>::from_iter(finalization.meta.deleted.clone()), + HashSet::from_iter(vec![ + to_journal_key(1, 0), + to_meta_key(&NON_CANONICAL_JOURNAL_LEN, &1u64), + ]) + ); db.commit(&finalization); assert!(db.data_eq(&make_db(&[1, 3, 4]))); } From f6753c25f18c160ca631c4e20ca5a2514009d39f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Gawrya=C5=82?= Date: Wed, 10 Jan 2024 17:25:00 +0100 Subject: [PATCH 3/7] Save journal length only above some threshold --- substrate/client/state-db/src/noncanonical.rs | 91 ++++++++++++------- 1 file changed, 59 insertions(+), 32 deletions(-) diff --git a/substrate/client/state-db/src/noncanonical.rs b/substrate/client/state-db/src/noncanonical.rs index baa214e51f443..86b2fb99a2ff9 100644 --- a/substrate/client/state-db/src/noncanonical.rs +++ b/substrate/client/state-db/src/noncanonical.rs @@ -28,8 +28,11 @@ use log::trace; use std::collections::{hash_map::Entry, HashMap, VecDeque}; const NON_CANONICAL_JOURNAL: &[u8] = b"noncanonical_journal"; -const NON_CANONICAL_JOURNAL_LEN: &[u8] = b"noncanonical_journal_len"; pub(crate) const LAST_CANONICAL: &[u8] = b"last_canonical"; +const OVERLAY_LEVEL_LARGEST_INDEX_USED: &[u8] = b"noncanonical_overlay_largest_idx"; +// To decrease the number of database operations, the largest index used at some overlay +// level will be committed to the database iff it exceeds this threshold. +const OVERLAY_LEVEL_LARGEST_INDEX_THRESHOLD: u64 = 32; /// See module documentation. pub struct NonCanonicalOverlay { @@ -46,12 +49,14 @@ pub struct NonCanonicalOverlay { #[cfg_attr(test, derive(PartialEq, Debug))] struct OverlayLevel { blocks: Vec>, - used_indicies: u64, // Bitmask of available journal indicies. + used_indicies: u64, // Bitmask of available journal indices. + largest_index_ever_used: u64, } impl OverlayLevel { fn push(&mut self, overlay: BlockOverlay) { self.used_indicies |= 1 << overlay.journal_index; + self.largest_index_ever_used = self.largest_index_ever_used.max(overlay.journal_index); self.blocks.push(overlay) } @@ -59,17 +64,25 @@ impl OverlayLevel { self.used_indicies.trailing_ones() as u64 } + fn largest_index_ever_used(&self) -> u64 { + self.largest_index_ever_used + } + fn remove(&mut self, index: usize) -> BlockOverlay { self.used_indicies &= !(1 << self.blocks[index].journal_index); self.blocks.remove(index) } - fn level_width(&self) -> u64 { - 64 - self.used_indicies.leading_zeros() as u64 + fn new_with_largest_index(largest_index: u64) -> OverlayLevel { + OverlayLevel { + blocks: Vec::new(), + used_indicies: 0, + largest_index_ever_used: largest_index, + } } fn new() -> OverlayLevel { - OverlayLevel { blocks: Vec::new(), used_indicies: 0 } + Self::new_with_largest_index(0) } } @@ -193,15 +206,18 @@ impl NonCanonicalOverlay { let mut total: u64 = 0; block += 1; loop { - let mut level = OverlayLevel::new(); - let non_canonical_journal_len_record = db - .get_meta(&to_meta_key(NON_CANONICAL_JOURNAL_LEN, &block)) - .map_err(Error::Db)?; - let level_len = match non_canonical_journal_len_record { - Some(data) => Decode::decode(&mut data.as_slice())?, - None => 0, + let largest_index_used = db + .get_meta(&to_meta_key(OVERLAY_LEVEL_LARGEST_INDEX_USED, &block)) + .map_err(Error::Db)? + .map(|data| Decode::decode(&mut data.as_slice())) + .transpose()?; + let mut level = match largest_index_used { + Some(largest_index) => OverlayLevel::new_with_largest_index(largest_index), + None => OverlayLevel::new(), }; - for index in 0..level_len { + for index in + 0..=largest_index_used.unwrap_or(OVERLAY_LEVEL_LARGEST_INDEX_THRESHOLD - 1) + { let journal_key = to_journal_key(block, index); if let Some(record) = db.get_meta(&journal_key).map_err(Error::Db)? { let record: JournalRecord = @@ -311,11 +327,6 @@ impl NonCanonicalOverlay { } let index = level.available_index(); - if index == level.level_width() { - let key = to_meta_key(NON_CANONICAL_JOURNAL_LEN, &number); - let journal_len_at_number = index + 1; - commit.meta.inserted.push((key, journal_len_at_number.encode())); - } let journal_key = to_journal_key(number, index); let inserted = changeset.inserted.iter().map(|(k, _)| k.clone()).collect(); @@ -334,6 +345,15 @@ impl NonCanonicalOverlay { inserted: changeset.inserted, deleted: changeset.deleted, }; + + if index >= OVERLAY_LEVEL_LARGEST_INDEX_THRESHOLD && + index == level.largest_index_ever_used() + { + let key = to_meta_key(OVERLAY_LEVEL_LARGEST_INDEX_USED, &number); + let overlay_len_at_number = index + 1; + commit.meta.inserted.push((key, overlay_len_at_number.encode())); + } + commit.meta.inserted.push((journal_key, journal_record.encode())); trace!( target: LOG_TARGET, @@ -415,6 +435,7 @@ impl NonCanonicalOverlay { self.pinned_canonincalized.push(hash.clone()); let mut discarded_journals = Vec::new(); + let level_largest_index_ever_used = level.largest_index_ever_used(); for (i, overlay) in level.blocks.into_iter().enumerate() { let mut pinned_children = 0; // That's the one we need to canonicalize @@ -455,10 +476,10 @@ impl NonCanonicalOverlay { discarded_journals.push(overlay.journal_key.clone()); } commit.meta.deleted.append(&mut discarded_journals); - commit - .meta - .deleted - .push(to_meta_key(NON_CANONICAL_JOURNAL_LEN, &self.front_block_number())); + if level_largest_index_ever_used >= OVERLAY_LEVEL_LARGEST_INDEX_THRESHOLD { + let key = to_meta_key(OVERLAY_LEVEL_LARGEST_INDEX_USED, &self.front_block_number()); + commit.meta.deleted.push(key); + } let canonicalized = (hash.clone(), self.front_block_number()); commit @@ -497,8 +518,10 @@ impl NonCanonicalOverlay { self.parents.remove(&overlay.hash); discard_values(&mut self.values, overlay.inserted); } - let journal_level_len_key = to_meta_key(NON_CANONICAL_JOURNAL_LEN, &last_block_number); - commit.meta.deleted.push(journal_level_len_key); + if level.largest_index_ever_used >= OVERLAY_LEVEL_LARGEST_INDEX_THRESHOLD { + let key = to_meta_key(OVERLAY_LEVEL_LARGEST_INDEX_USED, &last_block_number); + commit.meta.deleted.push(key); + } commit }) } @@ -526,9 +549,13 @@ impl NonCanonicalOverlay { } if self.levels.back().map_or(false, |l| l.blocks.is_empty()) { let last_block_number = self.front_block_number() + level_count as u64 - 1; + if self.levels.back().map_or(0, |l| l.largest_index_ever_used) >= + OVERLAY_LEVEL_LARGEST_INDEX_THRESHOLD + { + let key = to_meta_key(OVERLAY_LEVEL_LARGEST_INDEX_USED, &last_block_number); + commit.meta.deleted.push(key); + } self.levels.pop_back(); - let journal_level_len_key = to_meta_key(NON_CANONICAL_JOURNAL_LEN, &last_block_number); - commit.meta.deleted.push(journal_level_len_key); } if !commit.meta.deleted.is_empty() { Some(commit) @@ -590,13 +617,13 @@ impl NonCanonicalOverlay { mod tests { use super::{to_journal_key, NonCanonicalOverlay}; use crate::{ - noncanonical::{LAST_CANONICAL, NON_CANONICAL_JOURNAL_LEN}, + noncanonical::LAST_CANONICAL, test::{make_changeset, make_db}, to_meta_key, ChangeSet, CommitSet, MetaDb, StateDbError, }; + use codec::Encode; use sp_core::H256; use std::collections::HashSet; - use codec::Encode; fn contains(overlay: &NonCanonicalOverlay, key: u64) -> bool { overlay.get(&H256::from_low_u64_be(key)) == @@ -692,7 +719,7 @@ mod tests { HashSet::from_iter(&[ to_meta_key(LAST_CANONICAL, &()), to_journal_key(1, 0), - to_meta_key(&NON_CANONICAL_JOURNAL_LEN, &1u64), + // to_meta_key(&OVERLAY_LEVEL_LARGEST_INDEX_USED, &1u64), ]) ); assert!(insertion.meta.deleted.is_empty()); @@ -703,13 +730,13 @@ mod tests { assert_eq!(finalization.data.deleted.len(), changeset.deleted.len()); assert_eq!( finalization.meta.inserted, - vec![(to_meta_key(LAST_CANONICAL, &()),(&h1, 1u64).encode())] + vec![(to_meta_key(LAST_CANONICAL, &()), (&h1, 1u64).encode())] ); assert_eq!( HashSet::>::from_iter(finalization.meta.deleted.clone()), HashSet::from_iter(vec![ to_journal_key(1, 0), - to_meta_key(&NON_CANONICAL_JOURNAL_LEN, &1u64), + // to_meta_key(&OVERLAY_LEVEL_LARGEST_INDEX_USED, &1u64), ]) ); db.commit(&finalization); @@ -728,7 +755,7 @@ mod tests { .unwrap(), ); db.commit(&overlay.insert(&h2, 11, &h1, make_changeset(&[5], &[3])).unwrap()); - assert_eq!(db.meta_len(), 5); + assert_eq!(db.meta_len(), 3); //TODO let overlay2 = NonCanonicalOverlay::::new(&db).unwrap(); assert_eq!(overlay.levels, overlay2.levels); From 0052ff4a51e2c7466b72754f4727597900315131 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Gawrya=C5=82?= Date: Thu, 11 Jan 2024 14:30:54 +0100 Subject: [PATCH 4/7] Tests --- substrate/client/state-db/src/noncanonical.rs | 185 ++++++++++++------ 1 file changed, 122 insertions(+), 63 deletions(-) diff --git a/substrate/client/state-db/src/noncanonical.rs b/substrate/client/state-db/src/noncanonical.rs index 86b2fb99a2ff9..8789a72432627 100644 --- a/substrate/client/state-db/src/noncanonical.rs +++ b/substrate/client/state-db/src/noncanonical.rs @@ -29,10 +29,10 @@ use std::collections::{hash_map::Entry, HashMap, VecDeque}; const NON_CANONICAL_JOURNAL: &[u8] = b"noncanonical_journal"; pub(crate) const LAST_CANONICAL: &[u8] = b"last_canonical"; -const OVERLAY_LEVEL_LARGEST_INDEX_USED: &[u8] = b"noncanonical_overlay_largest_idx"; -// To decrease the number of database operations, the largest index used at some overlay -// level will be committed to the database iff it exceeds this threshold. -const OVERLAY_LEVEL_LARGEST_INDEX_THRESHOLD: u64 = 32; +const OVERLAY_LEVEL_SPAN: &[u8] = b"noncanonical_overlay_span"; +/// To decrease the number of database operations, the span of some overlay +/// level will be committed to the database iff index with this number belongs to it. +const OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN: u64 = 32; /// See module documentation. pub struct NonCanonicalOverlay { @@ -50,13 +50,13 @@ pub struct NonCanonicalOverlay { struct OverlayLevel { blocks: Vec>, used_indicies: u64, // Bitmask of available journal indices. - largest_index_ever_used: u64, + span: u64, // Largest index ever used + 1 (or 0 when None) } impl OverlayLevel { fn push(&mut self, overlay: BlockOverlay) { self.used_indicies |= 1 << overlay.journal_index; - self.largest_index_ever_used = self.largest_index_ever_used.max(overlay.journal_index); + self.span = self.span.max(overlay.journal_index + 1); self.blocks.push(overlay) } @@ -64,8 +64,8 @@ impl OverlayLevel { self.used_indicies.trailing_ones() as u64 } - fn largest_index_ever_used(&self) -> u64 { - self.largest_index_ever_used + fn span(&self) -> u64 { + self.span } fn remove(&mut self, index: usize) -> BlockOverlay { @@ -73,16 +73,12 @@ impl OverlayLevel { self.blocks.remove(index) } - fn new_with_largest_index(largest_index: u64) -> OverlayLevel { - OverlayLevel { - blocks: Vec::new(), - used_indicies: 0, - largest_index_ever_used: largest_index, - } + fn new_with_span(span: u64) -> OverlayLevel { + OverlayLevel { blocks: Vec::new(), used_indicies: 0, span } } fn new() -> OverlayLevel { - Self::new_with_largest_index(0) + Self::new_with_span(0) } } @@ -206,18 +202,13 @@ impl NonCanonicalOverlay { let mut total: u64 = 0; block += 1; loop { - let largest_index_used = db - .get_meta(&to_meta_key(OVERLAY_LEVEL_LARGEST_INDEX_USED, &block)) + let level_span = db + .get_meta(&to_meta_key(OVERLAY_LEVEL_SPAN, &block)) .map_err(Error::Db)? .map(|data| Decode::decode(&mut data.as_slice())) .transpose()?; - let mut level = match largest_index_used { - Some(largest_index) => OverlayLevel::new_with_largest_index(largest_index), - None => OverlayLevel::new(), - }; - for index in - 0..=largest_index_used.unwrap_or(OVERLAY_LEVEL_LARGEST_INDEX_THRESHOLD - 1) - { + let mut level = OverlayLevel::new_with_span(level_span.unwrap_or(0)); + for index in 0..level_span.unwrap_or(OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN) { let journal_key = to_journal_key(block, index); if let Some(record) = db.get_meta(&journal_key).map_err(Error::Db)? { let record: JournalRecord = @@ -327,8 +318,13 @@ impl NonCanonicalOverlay { } let index = level.available_index(); - let journal_key = to_journal_key(number, index); + if index >= OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN && index == level.span() { + let key = to_meta_key(OVERLAY_LEVEL_SPAN, &number); + let new_span = index + 1; + commit.meta.inserted.push((key, new_span.encode())); + } + let journal_key = to_journal_key(number, index); let inserted = changeset.inserted.iter().map(|(k, _)| k.clone()).collect(); let overlay = BlockOverlay { hash: hash.clone(), @@ -346,14 +342,6 @@ impl NonCanonicalOverlay { deleted: changeset.deleted, }; - if index >= OVERLAY_LEVEL_LARGEST_INDEX_THRESHOLD && - index == level.largest_index_ever_used() - { - let key = to_meta_key(OVERLAY_LEVEL_LARGEST_INDEX_USED, &number); - let overlay_len_at_number = index + 1; - commit.meta.inserted.push((key, overlay_len_at_number.encode())); - } - commit.meta.inserted.push((journal_key, journal_record.encode())); trace!( target: LOG_TARGET, @@ -435,7 +423,7 @@ impl NonCanonicalOverlay { self.pinned_canonincalized.push(hash.clone()); let mut discarded_journals = Vec::new(); - let level_largest_index_ever_used = level.largest_index_ever_used(); + let span = level.span(); for (i, overlay) in level.blocks.into_iter().enumerate() { let mut pinned_children = 0; // That's the one we need to canonicalize @@ -476,8 +464,8 @@ impl NonCanonicalOverlay { discarded_journals.push(overlay.journal_key.clone()); } commit.meta.deleted.append(&mut discarded_journals); - if level_largest_index_ever_used >= OVERLAY_LEVEL_LARGEST_INDEX_THRESHOLD { - let key = to_meta_key(OVERLAY_LEVEL_LARGEST_INDEX_USED, &self.front_block_number()); + if span > OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN { + let key = to_meta_key(OVERLAY_LEVEL_SPAN, &self.front_block_number()); commit.meta.deleted.push(key); } @@ -518,8 +506,8 @@ impl NonCanonicalOverlay { self.parents.remove(&overlay.hash); discard_values(&mut self.values, overlay.inserted); } - if level.largest_index_ever_used >= OVERLAY_LEVEL_LARGEST_INDEX_THRESHOLD { - let key = to_meta_key(OVERLAY_LEVEL_LARGEST_INDEX_USED, &last_block_number); + if level.span > OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN { + let key = to_meta_key(OVERLAY_LEVEL_SPAN, &last_block_number); commit.meta.deleted.push(key); } commit @@ -549,10 +537,8 @@ impl NonCanonicalOverlay { } if self.levels.back().map_or(false, |l| l.blocks.is_empty()) { let last_block_number = self.front_block_number() + level_count as u64 - 1; - if self.levels.back().map_or(0, |l| l.largest_index_ever_used) >= - OVERLAY_LEVEL_LARGEST_INDEX_THRESHOLD - { - let key = to_meta_key(OVERLAY_LEVEL_LARGEST_INDEX_USED, &last_block_number); + if self.levels.back().map_or(0, |l| l.span) > OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN { + let key = to_meta_key(OVERLAY_LEVEL_SPAN, &last_block_number); commit.meta.deleted.push(key); } self.levels.pop_back(); @@ -617,7 +603,7 @@ impl NonCanonicalOverlay { mod tests { use super::{to_journal_key, NonCanonicalOverlay}; use crate::{ - noncanonical::LAST_CANONICAL, + noncanonical::{LAST_CANONICAL, OVERLAY_LEVEL_SPAN, OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN}, test::{make_changeset, make_db}, to_meta_key, ChangeSet, CommitSet, MetaDb, StateDbError, }; @@ -714,31 +700,15 @@ mod tests { let insertion = overlay.insert(&h1, 1, &H256::default(), changeset.clone()).unwrap(); assert_eq!(insertion.data.inserted.len(), 0); assert_eq!(insertion.data.deleted.len(), 0); - assert_eq!( - insertion.meta.inserted.iter().map(|(k, _)| k).collect::>(), - HashSet::from_iter(&[ - to_meta_key(LAST_CANONICAL, &()), - to_journal_key(1, 0), - // to_meta_key(&OVERLAY_LEVEL_LARGEST_INDEX_USED, &1u64), - ]) - ); - assert!(insertion.meta.deleted.is_empty()); + assert_eq!(insertion.meta.inserted.len(), 2); + assert_eq!(insertion.meta.deleted.len(), 0); db.commit(&insertion); let mut finalization = CommitSet::default(); overlay.canonicalize(&h1, &mut finalization).unwrap(); assert_eq!(finalization.data.inserted.len(), changeset.inserted.len()); assert_eq!(finalization.data.deleted.len(), changeset.deleted.len()); - assert_eq!( - finalization.meta.inserted, - vec![(to_meta_key(LAST_CANONICAL, &()), (&h1, 1u64).encode())] - ); - assert_eq!( - HashSet::>::from_iter(finalization.meta.deleted.clone()), - HashSet::from_iter(vec![ - to_journal_key(1, 0), - // to_meta_key(&OVERLAY_LEVEL_LARGEST_INDEX_USED, &1u64), - ]) - ); + assert_eq!(finalization.meta.inserted.len(), 1); + assert_eq!(finalization.meta.deleted.len(), 1); db.commit(&finalization); assert!(db.data_eq(&make_db(&[1, 3, 4]))); } @@ -1192,4 +1162,93 @@ mod tests { db.commit(&overlay.remove(&h2).unwrap()); assert!(!contains(&overlay, 2)); } + + #[test] + fn insert_canonicalize_long_span() { + let mut db = make_db(&[]); + let mut overlay = NonCanonicalOverlay::<(u64, u64), H256>::new(&db).unwrap(); + let indices_per_level = OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN+2; + + for i in 0..indices_per_level { + let changeset = make_changeset(&[i], &[]); + let insertion = overlay.insert(&(1,i), 1, &(0,0), changeset).unwrap(); + db.commit(&insertion); + let expected_insertion_meta_len = match i { + 0 => 2, // last canonical, journal key + OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN.. => 2, // journal key, span + _ => 1, // just a journal key + }; + assert_eq!(insertion.meta.inserted.len(), expected_insertion_meta_len); + } + + let mut commit = CommitSet::default(); + overlay.canonicalize(&(1,indices_per_level-1), &mut commit).unwrap(); + + let expected_meta_deleted = (0..indices_per_level) + .map(|i| to_journal_key(1, i)) + .chain([to_meta_key(OVERLAY_LEVEL_SPAN, &1u64)]) + .collect::>(); + assert_eq!(commit.meta.inserted.len(), 1); + assert_eq!(HashSet::from_iter(commit.meta.deleted), expected_meta_deleted); + } + + #[test] + fn restore_from_journal_long_span() { + let mut db = make_db(&[]); + let mut overlay = NonCanonicalOverlay::<(u64, u64), H256>::new(&db).unwrap(); + let indices_per_level = OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN+2; + + for i in 0..indices_per_level { + let changeset = make_changeset(&[i], &[]); + let insertion = overlay.insert(&(1,i), 1, &(0,0), changeset).unwrap(); + db.commit(&insertion); + } + + for i in 0..indices_per_level-1 { + db.commit(&overlay.remove(&(1,i)).unwrap()); + } + + // Restore into a new overlay and check that journaled value exists. + let mut overlay = NonCanonicalOverlay::<(u64, u64), H256>::new(&db).unwrap(); + let mut commit = CommitSet::default(); + overlay.canonicalize(&(1,indices_per_level-1), &mut commit).unwrap(); + db.commit(&commit); + + assert!(db.data_eq(& make_db(&[indices_per_level-1]))); + assert_eq!(commit.meta.inserted.len(), 1); + assert_eq!(commit.meta.deleted.len(), 2); // journal key, span + } + + #[test] + fn overlay_level_span_entries_are_cleared_from_db() { + let mut db = make_db(&[]); + let mut overlay = NonCanonicalOverlay::<(u64, u64), H256>::new(&db).unwrap(); + let indices_per_level = OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN + 1; + for level in 1..4 { + for index in 0..indices_per_level { + let changeset = make_changeset(&[1], &[]); + let insertion = + overlay.insert(&(level, index), level, &(level - 1, 0), changeset).unwrap(); + db.commit(&insertion); + } + } + + db.commit(&overlay.revert_one().unwrap()); + for index in 0..indices_per_level { + db.commit(&overlay.remove(&(2, index)).unwrap()); + } + for index in OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN..indices_per_level { + db.commit(&overlay.remove(&(1, index)).unwrap()); + } + let mut overlay = NonCanonicalOverlay::<(u64, u64), H256>::new(&db).unwrap(); + let mut commit = CommitSet::default(); + overlay.canonicalize(&(1, 1), &mut commit).unwrap(); + db.commit(&commit); + + assert_eq!(db.meta_len(), 1); + assert_eq!( + db.get_meta(&to_meta_key(LAST_CANONICAL, &())), + Ok(Some((&(1u64, 1u64), 1u64).encode())) + ); + } } From 4b97495d7b842f31e7286816fedc9f1d54ef840d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Gawrya=C5=82?= Date: Thu, 11 Jan 2024 15:59:44 +0100 Subject: [PATCH 5/7] Replace indices bit mask --- substrate/client/state-db/src/noncanonical.rs | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/substrate/client/state-db/src/noncanonical.rs b/substrate/client/state-db/src/noncanonical.rs index 8789a72432627..11862be2740d0 100644 --- a/substrate/client/state-db/src/noncanonical.rs +++ b/substrate/client/state-db/src/noncanonical.rs @@ -25,13 +25,13 @@ use crate::{LOG_TARGET, LOG_TARGET_PIN}; use super::{to_meta_key, ChangeSet, CommitSet, DBValue, Error, Hash, MetaDb, StateDbError}; use codec::{Decode, Encode}; use log::trace; -use std::collections::{hash_map::Entry, HashMap, VecDeque}; +use std::collections::{hash_map::Entry, BTreeSet, HashMap, VecDeque}; const NON_CANONICAL_JOURNAL: &[u8] = b"noncanonical_journal"; pub(crate) const LAST_CANONICAL: &[u8] = b"last_canonical"; const OVERLAY_LEVEL_SPAN: &[u8] = b"noncanonical_overlay_span"; /// To decrease the number of database operations, the span of some overlay -/// level will be committed to the database iff index with this number belongs to it. +/// level will be committed to the database iff span > `OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN` const OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN: u64 = 32; /// See module documentation. @@ -49,19 +49,19 @@ pub struct NonCanonicalOverlay { #[cfg_attr(test, derive(PartialEq, Debug))] struct OverlayLevel { blocks: Vec>, - used_indicies: u64, // Bitmask of available journal indices. - span: u64, // Largest index ever used + 1 (or 0 when None) + span: u64, // largest index ever used + 1 (or 0 when None) + available_indices: BTreeSet, // available indices in [0; span) } impl OverlayLevel { fn push(&mut self, overlay: BlockOverlay) { - self.used_indicies |= 1 << overlay.journal_index; self.span = self.span.max(overlay.journal_index + 1); + self.available_indices.remove(&overlay.journal_index); self.blocks.push(overlay) } fn available_index(&self) -> u64 { - self.used_indicies.trailing_ones() as u64 + *self.available_indices.first().unwrap_or(&self.span) } fn span(&self) -> u64 { @@ -69,12 +69,13 @@ impl OverlayLevel { } fn remove(&mut self, index: usize) -> BlockOverlay { - self.used_indicies &= !(1 << self.blocks[index].journal_index); + self.available_indices.insert(self.blocks[index].journal_index); self.blocks.remove(index) } fn new_with_span(span: u64) -> OverlayLevel { - OverlayLevel { blocks: Vec::new(), used_indicies: 0, span } + let available_indices = (0..span).collect::>(); + OverlayLevel { blocks: Vec::new(), span, available_indices } } fn new() -> OverlayLevel { @@ -1167,11 +1168,11 @@ mod tests { fn insert_canonicalize_long_span() { let mut db = make_db(&[]); let mut overlay = NonCanonicalOverlay::<(u64, u64), H256>::new(&db).unwrap(); - let indices_per_level = OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN+2; + let indices = 130; - for i in 0..indices_per_level { + for i in 0..indices { let changeset = make_changeset(&[i], &[]); - let insertion = overlay.insert(&(1,i), 1, &(0,0), changeset).unwrap(); + let insertion = overlay.insert(&(1, i), 1, &(0, 0), changeset).unwrap(); db.commit(&insertion); let expected_insertion_meta_len = match i { 0 => 2, // last canonical, journal key @@ -1182,9 +1183,9 @@ mod tests { } let mut commit = CommitSet::default(); - overlay.canonicalize(&(1,indices_per_level-1), &mut commit).unwrap(); + overlay.canonicalize(&(1, indices - 1), &mut commit).unwrap(); - let expected_meta_deleted = (0..indices_per_level) + let expected_meta_deleted = (0..indices) .map(|i| to_journal_key(1, i)) .chain([to_meta_key(OVERLAY_LEVEL_SPAN, &1u64)]) .collect::>(); @@ -1196,25 +1197,25 @@ mod tests { fn restore_from_journal_long_span() { let mut db = make_db(&[]); let mut overlay = NonCanonicalOverlay::<(u64, u64), H256>::new(&db).unwrap(); - let indices_per_level = OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN+2; + let indices = OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN + 2; - for i in 0..indices_per_level { + for i in 0..indices { let changeset = make_changeset(&[i], &[]); - let insertion = overlay.insert(&(1,i), 1, &(0,0), changeset).unwrap(); + let insertion = overlay.insert(&(1, i), 1, &(0, 0), changeset).unwrap(); db.commit(&insertion); } - for i in 0..indices_per_level-1 { - db.commit(&overlay.remove(&(1,i)).unwrap()); + for i in 0..indices - 1 { + db.commit(&overlay.remove(&(1, i)).unwrap()); } // Restore into a new overlay and check that journaled value exists. let mut overlay = NonCanonicalOverlay::<(u64, u64), H256>::new(&db).unwrap(); let mut commit = CommitSet::default(); - overlay.canonicalize(&(1,indices_per_level-1), &mut commit).unwrap(); + overlay.canonicalize(&(1, indices - 1), &mut commit).unwrap(); db.commit(&commit); - assert!(db.data_eq(& make_db(&[indices_per_level-1]))); + assert!(db.data_eq(&make_db(&[indices - 1]))); assert_eq!(commit.meta.inserted.len(), 1); assert_eq!(commit.meta.deleted.len(), 2); // journal key, span } @@ -1233,6 +1234,7 @@ mod tests { } } + // leave only the 1st level, with not too many blocks, and restore into a new overlay db.commit(&overlay.revert_one().unwrap()); for index in 0..indices_per_level { db.commit(&overlay.remove(&(2, index)).unwrap()); @@ -1241,6 +1243,7 @@ mod tests { db.commit(&overlay.remove(&(1, index)).unwrap()); } let mut overlay = NonCanonicalOverlay::<(u64, u64), H256>::new(&db).unwrap(); + let mut commit = CommitSet::default(); overlay.canonicalize(&(1, 1), &mut commit).unwrap(); db.commit(&commit); From 8a51306c8ff3989d799c0ce3c059f00f911cbb5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Gawrya=C5=82?= Date: Thu, 11 Jan 2024 16:47:16 +0100 Subject: [PATCH 6/7] Comments --- substrate/client/state-db/src/noncanonical.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/substrate/client/state-db/src/noncanonical.rs b/substrate/client/state-db/src/noncanonical.rs index 11862be2740d0..97f38a47e75d0 100644 --- a/substrate/client/state-db/src/noncanonical.rs +++ b/substrate/client/state-db/src/noncanonical.rs @@ -77,10 +77,6 @@ impl OverlayLevel { let available_indices = (0..span).collect::>(); OverlayLevel { blocks: Vec::new(), span, available_indices } } - - fn new() -> OverlayLevel { - Self::new_with_span(0) - } } #[derive(Encode, Decode)] @@ -208,6 +204,8 @@ impl NonCanonicalOverlay { .map_err(Error::Db)? .map(|data| Decode::decode(&mut data.as_slice())) .transpose()?; + // Since we don't update the overlay level span on block removal, + // we have to restore it there let mut level = OverlayLevel::new_with_span(level_span.unwrap_or(0)); for index in 0..level_span.unwrap_or(OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN) { let journal_key = to_journal_key(block, index); @@ -307,7 +305,7 @@ impl NonCanonicalOverlay { let level = if self.levels.is_empty() || number == front_block_number + self.levels.len() as u64 { - self.levels.push_back(OverlayLevel::new()); + self.levels.push_back(OverlayLevel::new_with_span(0)); self.levels.back_mut().expect("can't be empty after insertion; qed") } else { self.levels.get_mut((number - front_block_number) as usize) @@ -342,7 +340,6 @@ impl NonCanonicalOverlay { inserted: changeset.inserted, deleted: changeset.deleted, }; - commit.meta.inserted.push((journal_key, journal_record.encode())); trace!( target: LOG_TARGET, @@ -726,7 +723,7 @@ mod tests { .unwrap(), ); db.commit(&overlay.insert(&h2, 11, &h1, make_changeset(&[5], &[3])).unwrap()); - assert_eq!(db.meta_len(), 3); //TODO + assert_eq!(db.meta_len(), 3); let overlay2 = NonCanonicalOverlay::::new(&db).unwrap(); assert_eq!(overlay.levels, overlay2.levels); From d3b00a57d72782b6495285ccf0697fdebcc43de5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Gawrya=C5=82?= Date: Fri, 12 Jan 2024 08:50:35 +0100 Subject: [PATCH 7/7] Update doc --- substrate/client/state-db/src/lib.rs | 1 - substrate/client/state-db/src/noncanonical.rs | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/client/state-db/src/lib.rs b/substrate/client/state-db/src/lib.rs index 677f0a5910b03..81fd13537a90c 100644 --- a/substrate/client/state-db/src/lib.rs +++ b/substrate/client/state-db/src/lib.rs @@ -22,7 +22,6 @@ //! Canonicalization window tracks a tree of blocks identified by header hash. The in-memory //! overlay allows to get any trie node that was inserted in any of the blocks within the window. //! The overlay is journaled to the backing database and rebuilt on startup. -//! There's a limit of 32 blocks that may have the same block number in the canonicalization window. //! //! Canonicalization function selects one root from the top of the tree and discards all other roots //! and their subtrees. Upon canonicalization all trie nodes that were inserted in the block are diff --git a/substrate/client/state-db/src/noncanonical.rs b/substrate/client/state-db/src/noncanonical.rs index 97f38a47e75d0..d18b17df4e873 100644 --- a/substrate/client/state-db/src/noncanonical.rs +++ b/substrate/client/state-db/src/noncanonical.rs @@ -30,6 +30,7 @@ use std::collections::{hash_map::Entry, BTreeSet, HashMap, VecDeque}; const NON_CANONICAL_JOURNAL: &[u8] = b"noncanonical_journal"; pub(crate) const LAST_CANONICAL: &[u8] = b"last_canonical"; const OVERLAY_LEVEL_SPAN: &[u8] = b"noncanonical_overlay_span"; +/// Threshold for storing overlay level span in db. /// To decrease the number of database operations, the span of some overlay /// level will be committed to the database iff span > `OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN` const OVERLAY_LEVEL_STORE_SPANS_LONGER_THAN: u64 = 32;