From 7ebb49ccb8dacd0492ce8faf053549fe1f581f34 Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Thu, 30 Jun 2022 11:22:25 +0100 Subject: [PATCH 1/5] Limit number of elements loaded from the stagnant key This will likely be required if we enable stagnant prunning as currently database has way too many entries to be prunned in a single iteration --- node/core/chain-selection/src/backend.rs | 3 ++- .../core/chain-selection/src/db_backend/v1.rs | 22 +++++++++++++------ node/core/chain-selection/src/lib.rs | 8 ++++--- node/core/chain-selection/src/tests.rs | 5 ++++- node/core/chain-selection/src/tree.rs | 9 +++++++- 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/node/core/chain-selection/src/backend.rs b/node/core/chain-selection/src/backend.rs index 71b6797d3526..6c5396a5c64d 100644 --- a/node/core/chain-selection/src/backend.rs +++ b/node/core/chain-selection/src/backend.rs @@ -46,10 +46,11 @@ pub(super) trait Backend { /// Load the stagnant list at the given timestamp. fn load_stagnant_at(&self, timestamp: Timestamp) -> Result, Error>; /// Load all stagnant lists up to and including the given Unix timestamp - /// in ascending order. + /// in ascending order. Stop fetching stagnant entries upon reaching `max_elements`. fn load_stagnant_at_up_to( &self, up_to: Timestamp, + max_elements: usize, ) -> Result)>, Error>; /// Load the earliest kept block number. fn load_first_block_number(&self) -> Result, Error>; diff --git a/node/core/chain-selection/src/db_backend/v1.rs b/node/core/chain-selection/src/db_backend/v1.rs index 8b4591f5c3a9..8037561afa07 100644 --- a/node/core/chain-selection/src/db_backend/v1.rs +++ b/node/core/chain-selection/src/db_backend/v1.rs @@ -229,6 +229,7 @@ impl Backend for DbBackend { fn load_stagnant_at_up_to( &self, up_to: crate::Timestamp, + max_elements: usize, ) -> Result)>, Error> { let stagnant_at_iter = self.inner.iter_with_prefix(self.config.col_data, &STAGNANT_AT_PREFIX[..]); @@ -240,7 +241,9 @@ impl Backend for DbBackend { _ => None, } }) - .take_while(|(at, _)| *at <= up_to.into()) + .enumerate() + .take_while(|(idx, (at, _))| *at <= up_to.into() && *idx < max_elements) + .map(|(_, v)| v) .collect::>(); Ok(val) @@ -528,7 +531,7 @@ mod tests { let mut backend = DbBackend::new(db, config); // Prove that it's cheap - assert!(backend.load_stagnant_at_up_to(Timestamp::max_value()).unwrap().is_empty()); + assert!(backend.load_stagnant_at_up_to(Timestamp::max_value(), usize::MAX).unwrap().is_empty()); backend .write(vec![ @@ -539,7 +542,7 @@ mod tests { .unwrap(); assert_eq!( - backend.load_stagnant_at_up_to(Timestamp::max_value()).unwrap(), + backend.load_stagnant_at_up_to(Timestamp::max_value(), usize::MAX).unwrap(), vec![ (2, vec![Hash::repeat_byte(1)]), (5, vec![Hash::repeat_byte(2)]), @@ -548,7 +551,7 @@ mod tests { ); assert_eq!( - backend.load_stagnant_at_up_to(10).unwrap(), + backend.load_stagnant_at_up_to(10, usize::MAX).unwrap(), vec![ (2, vec![Hash::repeat_byte(1)]), (5, vec![Hash::repeat_byte(2)]), @@ -557,21 +560,26 @@ mod tests { ); assert_eq!( - backend.load_stagnant_at_up_to(9).unwrap(), + backend.load_stagnant_at_up_to(9, usize::MAX).unwrap(), vec![(2, vec![Hash::repeat_byte(1)]), (5, vec![Hash::repeat_byte(2)]),] ); + assert_eq!( + backend.load_stagnant_at_up_to(9, 1).unwrap(), + vec![(2, vec![Hash::repeat_byte(1)]),] + ); + backend.write(vec![BackendWriteOp::DeleteStagnantAt(2)]).unwrap(); assert_eq!( - backend.load_stagnant_at_up_to(5).unwrap(), + backend.load_stagnant_at_up_to(5, usize::MAX).unwrap(), vec![(5, vec![Hash::repeat_byte(2)]),] ); backend.write(vec![BackendWriteOp::WriteStagnantAt(5, vec![])]).unwrap(); assert_eq!( - backend.load_stagnant_at_up_to(10).unwrap(), + backend.load_stagnant_at_up_to(10, usize::MAX).unwrap(), vec![(10, vec![Hash::repeat_byte(3)]),] ); } diff --git a/node/core/chain-selection/src/lib.rs b/node/core/chain-selection/src/lib.rs index a8646d874d8f..f9fe8188c8f4 100644 --- a/node/core/chain-selection/src/lib.rs +++ b/node/core/chain-selection/src/lib.rs @@ -50,6 +50,8 @@ type Timestamp = u64; // If a block isn't approved in 120 seconds, nodes will abandon it // and begin building on another chain. const STAGNANT_TIMEOUT: Timestamp = 120; +// Maximum number of stagnant entries cleaned during one `STAGNANT_TIMEOUT` iteration +const MAX_STAGNANT_ENTRIES: usize = 1000; #[derive(Debug, Clone)] enum Approval { @@ -435,7 +437,7 @@ where } } _ = stagnant_check_stream.next().fuse() => { - detect_stagnant(backend, clock.timestamp_now())?; + detect_stagnant(backend, clock.timestamp_now(), MAX_STAGNANT_ENTRIES)?; } } } @@ -637,9 +639,9 @@ fn handle_approved_block(backend: &mut impl Backend, approved_block: Hash) -> Re backend.write(ops) } -fn detect_stagnant(backend: &mut impl Backend, now: Timestamp) -> Result<(), Error> { +fn detect_stagnant(backend: &mut impl Backend, now: Timestamp, max_elements: usize) -> Result<(), Error> { let ops = { - let overlay = tree::detect_stagnant(&*backend, now)?; + let overlay = tree::detect_stagnant(&*backend, now, max_elements)?; overlay.into_write_ops() }; diff --git a/node/core/chain-selection/src/tests.rs b/node/core/chain-selection/src/tests.rs index 0b8947a200cf..20c4700dff57 100644 --- a/node/core/chain-selection/src/tests.rs +++ b/node/core/chain-selection/src/tests.rs @@ -139,13 +139,16 @@ impl Backend for TestBackend { fn load_stagnant_at_up_to( &self, up_to: Timestamp, + max_elements: usize, ) -> Result)>, Error> { Ok(self .inner .lock() .stagnant_at .range(..=up_to) - .map(|(t, v)| (*t, v.clone())) + .enumerate() + .take_while(|(idx, _)| *idx < max_elements) + .map(|(_, (t, v))| (*t, v.clone())) .collect()) } fn load_first_block_number(&self) -> Result, Error> { diff --git a/node/core/chain-selection/src/tree.rs b/node/core/chain-selection/src/tree.rs index d6f19b792a75..4a1384e513df 100644 --- a/node/core/chain-selection/src/tree.rs +++ b/node/core/chain-selection/src/tree.rs @@ -534,12 +534,15 @@ pub(super) fn approve_block( pub(super) fn detect_stagnant<'a, B: 'a + Backend>( backend: &'a B, up_to: Timestamp, + max_elements: usize, ) -> Result, Error> { - let stagnant_up_to = backend.load_stagnant_at_up_to(up_to)?; + let stagnant_up_to = backend.load_stagnant_at_up_to(up_to, max_elements)?; let mut backend = OverlayedBackend::new(backend); // As this is in ascending order, only the earliest stagnant // blocks will involve heavy viability propagations. + gum::debug!(target: LOG_TARGET, ?up_to, "Loaded {} stagnant entries", stagnant_up_to.len()); + for (timestamp, maybe_stagnant) in stagnant_up_to { backend.delete_stagnant_at(timestamp); @@ -550,6 +553,7 @@ pub(super) fn detect_stagnant<'a, B: 'a + Backend>( entry.viability.approval = Approval::Stagnant; } let is_viable = entry.viability.is_viable(); + gum::trace!(target: LOG_TARGET, ?block_hash, ?was_viable, is_viable, "Found existing stagnant entry"); if was_viable && !is_viable { propagate_viability_update(&mut backend, entry)?; @@ -557,6 +561,9 @@ pub(super) fn detect_stagnant<'a, B: 'a + Backend>( backend.write_block_entry(entry); } } + else { + gum::trace!(target: LOG_TARGET, ?block_hash, "Found non-existing stagnant entry"); + } } } From 2bb6f2619e83480810bf6d47ffd50ca39290979b Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Thu, 30 Jun 2022 11:24:28 +0100 Subject: [PATCH 2/5] Fmt run --- node/core/chain-selection/src/db_backend/v1.rs | 5 ++++- node/core/chain-selection/src/lib.rs | 6 +++++- node/core/chain-selection/src/tree.rs | 11 ++++++++--- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/node/core/chain-selection/src/db_backend/v1.rs b/node/core/chain-selection/src/db_backend/v1.rs index 8037561afa07..db117ff945df 100644 --- a/node/core/chain-selection/src/db_backend/v1.rs +++ b/node/core/chain-selection/src/db_backend/v1.rs @@ -531,7 +531,10 @@ mod tests { let mut backend = DbBackend::new(db, config); // Prove that it's cheap - assert!(backend.load_stagnant_at_up_to(Timestamp::max_value(), usize::MAX).unwrap().is_empty()); + assert!(backend + .load_stagnant_at_up_to(Timestamp::max_value(), usize::MAX) + .unwrap() + .is_empty()); backend .write(vec![ diff --git a/node/core/chain-selection/src/lib.rs b/node/core/chain-selection/src/lib.rs index f9fe8188c8f4..be6509e54a29 100644 --- a/node/core/chain-selection/src/lib.rs +++ b/node/core/chain-selection/src/lib.rs @@ -639,7 +639,11 @@ fn handle_approved_block(backend: &mut impl Backend, approved_block: Hash) -> Re backend.write(ops) } -fn detect_stagnant(backend: &mut impl Backend, now: Timestamp, max_elements: usize) -> Result<(), Error> { +fn detect_stagnant( + backend: &mut impl Backend, + now: Timestamp, + max_elements: usize, +) -> Result<(), Error> { let ops = { let overlay = tree::detect_stagnant(&*backend, now, max_elements)?; diff --git a/node/core/chain-selection/src/tree.rs b/node/core/chain-selection/src/tree.rs index 4a1384e513df..1b09e650f212 100644 --- a/node/core/chain-selection/src/tree.rs +++ b/node/core/chain-selection/src/tree.rs @@ -553,15 +553,20 @@ pub(super) fn detect_stagnant<'a, B: 'a + Backend>( entry.viability.approval = Approval::Stagnant; } let is_viable = entry.viability.is_viable(); - gum::trace!(target: LOG_TARGET, ?block_hash, ?was_viable, is_viable, "Found existing stagnant entry"); + gum::trace!( + target: LOG_TARGET, + ?block_hash, + ?was_viable, + is_viable, + "Found existing stagnant entry" + ); if was_viable && !is_viable { propagate_viability_update(&mut backend, entry)?; } else { backend.write_block_entry(entry); } - } - else { + } else { gum::trace!(target: LOG_TARGET, ?block_hash, "Found non-existing stagnant entry"); } } From 5b388bce2dd9c8fcf4c6ec3a4e11bf5760a11738 Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Fri, 1 Jul 2022 09:33:45 +0100 Subject: [PATCH 3/5] Slightly improve logging --- node/core/chain-selection/src/tree.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/node/core/chain-selection/src/tree.rs b/node/core/chain-selection/src/tree.rs index 1b09e650f212..e36526963a35 100644 --- a/node/core/chain-selection/src/tree.rs +++ b/node/core/chain-selection/src/tree.rs @@ -539,9 +539,22 @@ pub(super) fn detect_stagnant<'a, B: 'a + Backend>( let stagnant_up_to = backend.load_stagnant_at_up_to(up_to, max_elements)?; let mut backend = OverlayedBackend::new(backend); + let (min_ts, max_ts) = match stagnant_up_to.len() { + 0 => (0 as Timestamp, 0 as Timestamp), + 1 => (stagnant_up_to[0].0, stagnant_up_to[0].0), + n => (stagnant_up_to[0].0, stagnant_up_to[n - 1].0), + }; + // As this is in ascending order, only the earliest stagnant // blocks will involve heavy viability propagations. - gum::debug!(target: LOG_TARGET, ?up_to, "Loaded {} stagnant entries", stagnant_up_to.len()); + gum::debug!( + target: LOG_TARGET, + ?up_to, + ?min_ts, + ?max_ts, + "Prepared {} stagnant entries for pruning", + stagnant_up_to.len() + ); for (timestamp, maybe_stagnant) in stagnant_up_to { backend.delete_stagnant_at(timestamp); From a7427c3bb5c1c288797ad7054fab8e3f51cf2fa2 Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Fri, 1 Jul 2022 09:54:43 +0100 Subject: [PATCH 4/5] Some more debug nits --- node/core/chain-selection/src/tree.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/node/core/chain-selection/src/tree.rs b/node/core/chain-selection/src/tree.rs index e36526963a35..6ea71bfeb8c5 100644 --- a/node/core/chain-selection/src/tree.rs +++ b/node/core/chain-selection/src/tree.rs @@ -569,8 +569,9 @@ pub(super) fn detect_stagnant<'a, B: 'a + Backend>( gum::trace!( target: LOG_TARGET, ?block_hash, + ?timestamp, ?was_viable, - is_viable, + ?is_viable, "Found existing stagnant entry" ); @@ -580,7 +581,7 @@ pub(super) fn detect_stagnant<'a, B: 'a + Backend>( backend.write_block_entry(entry); } } else { - gum::trace!(target: LOG_TARGET, ?block_hash, "Found non-existing stagnant entry"); + gum::trace!(target: LOG_TARGET, ?block_hash, ?timestamp, "Found non-existing stagnant entry"); } } } From 2461fcf901c2cc76d2f2fcb1c5ce4bc1b6259c85 Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Fri, 1 Jul 2022 09:56:51 +0100 Subject: [PATCH 5/5] Fmt pass --- node/core/chain-selection/src/tree.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/node/core/chain-selection/src/tree.rs b/node/core/chain-selection/src/tree.rs index 6ea71bfeb8c5..5edb6748934d 100644 --- a/node/core/chain-selection/src/tree.rs +++ b/node/core/chain-selection/src/tree.rs @@ -581,7 +581,12 @@ pub(super) fn detect_stagnant<'a, B: 'a + Backend>( backend.write_block_entry(entry); } } else { - gum::trace!(target: LOG_TARGET, ?block_hash, ?timestamp, "Found non-existing stagnant entry"); + gum::trace!( + target: LOG_TARGET, + ?block_hash, + ?timestamp, + "Found non-existing stagnant entry" + ); } } }