From 7268a77bdaf601c8deb8c2127a582495cad26343 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Tue, 3 Oct 2023 13:45:14 +0200 Subject: [PATCH] fixes after review --- .../consensus/beacon/src/engine/test_utils.rs | 4 +- crates/primitives/src/lib.rs | 4 +- crates/primitives/src/prune/mod.rs | 28 ++++ crates/prune/src/event.rs | 4 +- crates/prune/src/pruner.rs | 135 +++++++++--------- 5 files changed, 100 insertions(+), 75 deletions(-) diff --git a/crates/consensus/beacon/src/engine/test_utils.rs b/crates/consensus/beacon/src/engine/test_utils.rs index 96d840327c172..7bb1606054798 100644 --- a/crates/consensus/beacon/src/engine/test_utils.rs +++ b/crates/consensus/beacon/src/engine/test_utils.rs @@ -19,7 +19,7 @@ use reth_interfaces::{ test_utils::{NoopFullBlockClient, TestConsensus}, }; use reth_payload_builder::test_utils::spawn_test_payload_service; -use reth_primitives::{BlockNumber, ChainSpec, PruneModes, B256, MAINNET, U256}; +use reth_primitives::{BlockNumber, ChainSpec, PruneModes, B256, U256}; use reth_provider::{ providers::BlockchainProvider, test_utils::TestExecutorFactory, BlockExecutor, BundleStateWithReceipts, ExecutorFactory, ProviderFactory, PrunableBlockExecutor, @@ -520,7 +520,7 @@ where self.base_config.chain_spec.clone(), 5, PruneModes::none(), - MAINNET.prune_delete_limit, + self.base_config.chain_spec.prune_delete_limit, watch::channel(None).1, ); diff --git a/crates/primitives/src/lib.rs b/crates/primitives/src/lib.rs index 38b2ce557d00d..c2dd60b8e2dfd 100644 --- a/crates/primitives/src/lib.rs +++ b/crates/primitives/src/lib.rs @@ -74,8 +74,8 @@ pub use net::{ }; pub use peer::{PeerId, WithPeerId}; pub use prune::{ - PruneCheckpoint, PruneMode, PruneModes, PrunePart, PrunePartError, ReceiptsLogPruneConfig, - MINIMUM_PRUNING_DISTANCE, + PruneCheckpoint, PruneMode, PruneModes, PrunePart, PrunePartError, PruneProgress, + ReceiptsLogPruneConfig, MINIMUM_PRUNING_DISTANCE, }; pub use receipt::{Receipt, ReceiptWithBloom, ReceiptWithBloomRef, Receipts}; pub use serde_helper::JsonU256; diff --git a/crates/primitives/src/prune/mod.rs b/crates/primitives/src/prune/mod.rs index 48bdacdb9e894..4b58ca81c186d 100644 --- a/crates/primitives/src/prune/mod.rs +++ b/crates/primitives/src/prune/mod.rs @@ -86,3 +86,31 @@ impl ReceiptsLogPruneConfig { Ok(lowest.map(|lowest| lowest.max(pruned_block))) } } + +/// Progress of pruning. +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum PruneProgress { + /// There is more data to prune. + HasMoreData, + /// Pruning has been finished. + Finished, +} + +impl PruneProgress { + /// Creates new [PruneProgress] from `done` boolean value. + /// + /// If `done == true`, returns [PruneProgress::Finished], otherwise + /// [PruneProgress::HasMoreData] is returned. + pub fn from_done(done: bool) -> Self { + if done { + Self::Finished + } else { + Self::HasMoreData + } + } + + /// Returns `true` if pruning has been finished. + pub fn is_finished(&self) -> bool { + matches!(self, Self::Finished) + } +} diff --git a/crates/prune/src/event.rs b/crates/prune/src/event.rs index 6aa5c767e3bfe..a59d98c4c4596 100644 --- a/crates/prune/src/event.rs +++ b/crates/prune/src/event.rs @@ -1,4 +1,4 @@ -use reth_primitives::{BlockNumber, PrunePart}; +use reth_primitives::{BlockNumber, PrunePart, PruneProgress}; use std::{collections::BTreeMap, time::Duration}; /// An event emitted by a [Pruner][crate::Pruner]. @@ -8,6 +8,6 @@ pub enum PrunerEvent { Finished { tip_block_number: BlockNumber, elapsed: Duration, - parts: BTreeMap, + parts: BTreeMap, }, } diff --git a/crates/prune/src/pruner.rs b/crates/prune/src/pruner.rs index b105b42de4192..12549c65024b9 100644 --- a/crates/prune/src/pruner.rs +++ b/crates/prune/src/pruner.rs @@ -14,7 +14,7 @@ use reth_db::{ use reth_interfaces::RethResult; use reth_primitives::{ listener::EventListeners, BlockNumber, ChainSpec, PruneCheckpoint, PruneMode, PruneModes, - PrunePart, TxNumber, MINIMUM_PRUNING_DISTANCE, + PrunePart, PruneProgress, TxNumber, MINIMUM_PRUNING_DISTANCE, }; use reth_provider::{ BlockReader, DatabaseProviderRW, ProviderFactory, PruneCheckpointReader, PruneCheckpointWriter, @@ -26,17 +26,10 @@ use tokio_stream::wrappers::UnboundedReceiverStream; use tracing::{debug, error, instrument, trace}; /// Result of [Pruner::run] execution. -/// -/// Returns `true` if pruning has been completed up to the target block, -/// and `false` if there's more data to prune in further runs. -pub type PrunerResult = Result; +pub type PrunerResult = Result; /// Result of part pruning. -/// -/// Returns `true` if pruning has been completed up to the target block, -/// and `false` if there's more data to prune in further runs. Also returns number of entries pruned -/// (deleted from the database). -type PrunePartResult = Result<(bool, usize), PrunerError>; +type PrunePartResult = Result<(PruneProgress, usize), PrunerError>; /// The pruner type itself with the result of [Pruner::run] pub type PrunerWithResult = (Pruner, PrunerResult); @@ -93,7 +86,7 @@ impl Pruner { self.last_pruned_block_number = Some(tip_block_number); trace!(target: "pruner", %tip_block_number, "Nothing to prune yet"); - return Ok(true) + return Ok(PruneProgress::Finished) } trace!(target: "pruner", %tip_block_number, "Pruner started"); @@ -124,32 +117,32 @@ impl Pruner { ); let part_start = Instant::now(); - let (part_done, deleted) = + let (part_progress, deleted) = self.prune_receipts(&provider, to_block, prune_mode, delete_limit)?; self.metrics .get_prune_part_metrics(PrunePart::Receipts) .duration_seconds .record(part_start.elapsed()); - done = done && part_done; + done = done && part_progress.is_finished(); delete_limit = delete_limit.saturating_sub(deleted); - parts.insert(PrunePart::Receipts, (part_done, deleted)); + parts.insert(PrunePart::Receipts, (part_progress, deleted)); } else { trace!(target: "pruner", prune_part = ?PrunePart::Receipts, "No target block to prune"); } if !self.modes.receipts_log_filter.is_empty() { let part_start = Instant::now(); - let (part_done, deleted) = + let (part_progress, deleted) = self.prune_receipts_by_logs(&provider, tip_block_number, delete_limit)?; self.metrics .get_prune_part_metrics(PrunePart::ContractLogs) .duration_seconds .record(part_start.elapsed()); - done = done && part_done; + done = done && part_progress.is_finished(); delete_limit = delete_limit.saturating_sub(deleted); - parts.insert(PrunePart::ContractLogs, (part_done, deleted)); + parts.insert(PrunePart::ContractLogs, (part_progress, deleted)); } else { trace!(target: "pruner", prune_part = ?PrunePart::ContractLogs, "No filter to prune"); } @@ -166,16 +159,16 @@ impl Pruner { ); let part_start = Instant::now(); - let (part_done, deleted) = + let (part_progress, deleted) = self.prune_transaction_lookup(&provider, to_block, prune_mode, delete_limit)?; self.metrics .get_prune_part_metrics(PrunePart::TransactionLookup) .duration_seconds .record(part_start.elapsed()); - done = done && part_done; + done = done && part_progress.is_finished(); delete_limit = delete_limit.saturating_sub(deleted); - parts.insert(PrunePart::TransactionLookup, (part_done, deleted)); + parts.insert(PrunePart::TransactionLookup, (part_progress, deleted)); } else { trace!( target: "pruner", @@ -196,16 +189,16 @@ impl Pruner { ); let part_start = Instant::now(); - let (part_done, deleted) = + let (part_progress, deleted) = self.prune_transaction_senders(&provider, to_block, prune_mode, delete_limit)?; self.metrics .get_prune_part_metrics(PrunePart::SenderRecovery) .duration_seconds .record(part_start.elapsed()); - done = done && part_done; + done = done && part_progress.is_finished(); delete_limit = delete_limit.saturating_sub(deleted); - parts.insert(PrunePart::SenderRecovery, (part_done, deleted)); + parts.insert(PrunePart::SenderRecovery, (part_progress, deleted)); } else { trace!( target: "pruner", @@ -226,16 +219,16 @@ impl Pruner { ); let part_start = Instant::now(); - let (part_done, deleted) = + let (part_progress, deleted) = self.prune_account_history(&provider, to_block, prune_mode, delete_limit)?; self.metrics .get_prune_part_metrics(PrunePart::AccountHistory) .duration_seconds .record(part_start.elapsed()); - done = done && part_done; + done = done && part_progress.is_finished(); delete_limit = delete_limit.saturating_sub(deleted); - parts.insert(PrunePart::AccountHistory, (part_done, deleted)); + parts.insert(PrunePart::AccountHistory, (part_progress, deleted)); } else { trace!( target: "pruner", @@ -256,16 +249,16 @@ impl Pruner { ); let part_start = Instant::now(); - let (part_done, deleted) = + let (part_progress, deleted) = self.prune_storage_history(&provider, to_block, prune_mode, delete_limit)?; self.metrics .get_prune_part_metrics(PrunePart::StorageHistory) .duration_seconds .record(part_start.elapsed()); - done = done && part_done; + done = done && part_progress.is_finished(); delete_limit = delete_limit.saturating_sub(deleted); - parts.insert(PrunePart::StorageHistory, (part_done, deleted)); + parts.insert(PrunePart::StorageHistory, (part_progress, deleted)); } else { trace!( target: "pruner", @@ -292,7 +285,7 @@ impl Pruner { self.listeners.notify(PrunerEvent::Finished { tip_block_number, elapsed, parts }); - Ok(done) + Ok(PruneProgress::from_done(done)) } /// Returns `true` if the pruning is needed at the provided tip block number. @@ -405,7 +398,7 @@ impl Pruner { Some(range) => range, None => { trace!(target: "pruner", "No receipts to prune"); - return Ok((true, 0)) + return Ok((PruneProgress::Finished, 0)) } }; let tx_range_end = *tx_range.end(); @@ -438,7 +431,7 @@ impl Pruner { // limit their pruning start point. provider.save_prune_checkpoint(PrunePart::ContractLogs, prune_checkpoint)?; - Ok((done, deleted)) + Ok((PruneProgress::from_done(done), deleted)) } /// Prune receipts up to the provided block, inclusive, by filtering logs. Works as in inclusion @@ -622,7 +615,7 @@ impl Pruner { prune_mode: PruneMode::Before(prune_mode_block), }, )?; - Ok((done, delete_limit - limit)) + Ok((PruneProgress::from_done(done), delete_limit - limit)) } /// Prune transaction lookup entries up to the provided block, inclusive, respecting the batch @@ -643,7 +636,7 @@ impl Pruner { Some(range) => range, None => { trace!(target: "pruner", "No transaction lookup entries to prune"); - return Ok((true, 0)) + return Ok((PruneProgress::Finished, 0)) } } .into_inner(); @@ -695,7 +688,7 @@ impl Pruner { }, )?; - Ok((done, deleted)) + Ok((PruneProgress::from_done(done), deleted)) } /// Prune transaction senders up to the provided block, inclusive. @@ -715,7 +708,7 @@ impl Pruner { Some(range) => range, None => { trace!(target: "pruner", "No transaction senders to prune"); - return Ok((true, 0)) + return Ok((PruneProgress::Finished, 0)) } }; let tx_range_end = *tx_range.end(); @@ -745,7 +738,7 @@ impl Pruner { }, )?; - Ok((done, deleted)) + Ok((PruneProgress::from_done(done), deleted)) } /// Prune account history up to the provided block, inclusive. @@ -765,7 +758,7 @@ impl Pruner { Some(range) => range, None => { trace!(target: "pruner", "No account history to prune"); - return Ok((true, 0)) + return Ok((PruneProgress::Finished, 0)) } }; let range_end = *range.end(); @@ -804,7 +797,7 @@ impl Pruner { }, )?; - Ok((done, deleted_changesets + deleted_indices)) + Ok((PruneProgress::from_done(done), deleted_changesets + deleted_indices)) } /// Prune storage history up to the provided block, inclusive. @@ -824,7 +817,7 @@ impl Pruner { Some(range) => range, None => { trace!(target: "pruner", "No storage history to prune"); - return Ok((true, 0)) + return Ok((PruneProgress::Finished, 0)) } }; let range_end = *range.end(); @@ -863,7 +856,7 @@ impl Pruner { }, )?; - Ok((done, deleted_changesets + deleted_indices)) + Ok((PruneProgress::from_done(done), deleted_changesets + deleted_indices)) } /// Prune history indices up to the provided block, inclusive. @@ -990,8 +983,8 @@ mod tests { }, }; use reth_primitives::{ - BlockNumber, PruneCheckpoint, PruneMode, PruneModes, PrunePart, ReceiptsLogPruneConfig, - TxNumber, B256, MAINNET, + BlockNumber, PruneCheckpoint, PruneMode, PruneModes, PrunePart, PruneProgress, + ReceiptsLogPruneConfig, TxNumber, B256, MAINNET, }; use reth_provider::{PruneCheckpointReader, TransactionsProvider}; use reth_stages::test_utils::TestTransaction; @@ -1048,7 +1041,7 @@ mod tests { tx.table::().unwrap().len() ); - let test_prune = |to_block: BlockNumber, expected_result: (bool, usize)| { + let test_prune = |to_block: BlockNumber, expected_result: (PruneProgress, usize)| { let prune_mode = PruneMode::Before(to_block); let pruner = Pruner::new( tx.inner_raw(), @@ -1100,7 +1093,7 @@ mod tests { assert_eq!(result, expected_result); let last_pruned_block_number = - last_pruned_block_number.checked_sub(if result.0 { 0 } else { 1 }); + last_pruned_block_number.checked_sub(if result.0.is_finished() { 0 } else { 1 }); assert_eq!( tx.table::().unwrap().len(), @@ -1116,9 +1109,9 @@ mod tests { ); }; - test_prune(6, (false, 10)); - test_prune(6, (true, 2)); - test_prune(10, (true, 8)); + test_prune(6, (PruneProgress::HasMoreData, 10)); + test_prune(6, (PruneProgress::Finished, 2)); + test_prune(10, (PruneProgress::Finished, 8)); } #[test] @@ -1146,7 +1139,7 @@ mod tests { tx.table::().unwrap().len() ); - let test_prune = |to_block: BlockNumber, expected_result: (bool, usize)| { + let test_prune = |to_block: BlockNumber, expected_result: (PruneProgress, usize)| { let prune_mode = PruneMode::Before(to_block); let pruner = Pruner::new( tx.inner_raw(), @@ -1203,7 +1196,7 @@ mod tests { assert_eq!(result, expected_result); let last_pruned_block_number = - last_pruned_block_number.checked_sub(if result.0 { 0 } else { 1 }); + last_pruned_block_number.checked_sub(if result.0.is_finished() { 0 } else { 1 }); assert_eq!( tx.table::().unwrap().len(), @@ -1219,9 +1212,9 @@ mod tests { ); }; - test_prune(6, (false, 10)); - test_prune(6, (true, 2)); - test_prune(10, (true, 8)); + test_prune(6, (PruneProgress::HasMoreData, 10)); + test_prune(6, (PruneProgress::Finished, 2)); + test_prune(10, (PruneProgress::Finished, 8)); } #[test] @@ -1253,7 +1246,7 @@ mod tests { tx.table::().unwrap().len() ); - let test_prune = |to_block: BlockNumber, expected_result: (bool, usize)| { + let test_prune = |to_block: BlockNumber, expected_result: (PruneProgress, usize)| { let prune_mode = PruneMode::Before(to_block); let pruner = Pruner::new( tx.inner_raw(), @@ -1310,7 +1303,7 @@ mod tests { assert_eq!(result, expected_result); let last_pruned_block_number = - last_pruned_block_number.checked_sub(if result.0 { 0 } else { 1 }); + last_pruned_block_number.checked_sub(if result.0.is_finished() { 0 } else { 1 }); assert_eq!( tx.table::().unwrap().len(), @@ -1326,9 +1319,9 @@ mod tests { ); }; - test_prune(6, (false, 10)); - test_prune(6, (true, 2)); - test_prune(10, (true, 8)); + test_prune(6, (PruneProgress::HasMoreData, 10)); + test_prune(6, (PruneProgress::Finished, 2)); + test_prune(10, (PruneProgress::Finished, 8)); } #[test] @@ -1368,7 +1361,9 @@ mod tests { let original_shards = tx.table::().unwrap(); - let test_prune = |to_block: BlockNumber, run: usize, expected_result: (bool, usize)| { + let test_prune = |to_block: BlockNumber, + run: usize, + expected_result: (PruneProgress, usize)| { let prune_mode = PruneMode::Before(to_block); let pruner = Pruner::new( tx.inner_raw(), @@ -1416,7 +1411,7 @@ mod tests { let last_pruned_block_number = pruned_changesets .next() - .map(|(block_number, _)| if result.0 { + .map(|(block_number, _)| if result.0.is_finished() { *block_number } else { block_number.saturating_sub(1) @@ -1462,9 +1457,9 @@ mod tests { ); }; - test_prune(998, 1, (false, 1000)); - test_prune(998, 2, (true, 998)); - test_prune(1400, 3, (true, 804)); + test_prune(998, 1, (PruneProgress::HasMoreData, 1000)); + test_prune(998, 2, (PruneProgress::Finished, 998)); + test_prune(1400, 3, (PruneProgress::Finished, 804)); } #[test] @@ -1504,7 +1499,9 @@ mod tests { let original_shards = tx.table::().unwrap(); - let test_prune = |to_block: BlockNumber, run: usize, expected_result: (bool, usize)| { + let test_prune = |to_block: BlockNumber, + run: usize, + expected_result: (PruneProgress, usize)| { let prune_mode = PruneMode::Before(to_block); let pruner = Pruner::new( tx.inner_raw(), @@ -1554,7 +1551,7 @@ mod tests { let last_pruned_block_number = pruned_changesets .next() - .map(|(block_number, _, _)| if result.0 { + .map(|(block_number, _, _)| if result.0.is_finished() { *block_number } else { block_number.saturating_sub(1) @@ -1600,9 +1597,9 @@ mod tests { ); }; - test_prune(998, 1, (false, 1000)); - test_prune(998, 2, (true, 998)); - test_prune(1400, 3, (true, 804)); + test_prune(998, 1, (PruneProgress::HasMoreData, 1000)); + test_prune(998, 2, (PruneProgress::Finished, 998)); + test_prune(1400, 3, (PruneProgress::Finished, 804)); } #[test] @@ -1686,7 +1683,7 @@ mod tests { ((pruned_tx + 1) - unprunable) as usize ); - result.0 + result.0.is_finished() }; while !run_prune() {}