Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Remove discarded blocks and states from database by default #11983

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
bc5ba8e
1.Add pruning param "canonical" in sc-cli.
hzy1919 Aug 5, 2022
d762b63
Merge branch 'master' into Remove-discarded-blocks-and-states-from-da…
hzy1919 Aug 5, 2022
e425634
Update tests in sc-state-db.
hzy1919 Aug 8, 2022
70b21b8
Merge branch 'master' into Remove-discarded-blocks-and-states-from-da…
hzy1919 Aug 8, 2022
124f586
Merge branch 'master' into Remove-discarded-blocks-and-states-from-da…
hzy1919 Aug 8, 2022
e2c5674
Update tests in sc-state-db.
hzy1919 Aug 8, 2022
7bff3cb
Merge branch 'master' into Remove-discarded-blocks-and-states-from-da…
hzy1919 Aug 19, 2022
da0c426
Merge branch 'master' into Remove-discarded-blocks-and-states-from-da…
hzy1919 Sep 1, 2022
c519937
Merge branch 'master' into Remove-discarded-blocks-and-states-from-da…
hzy1919 Sep 8, 2022
3f03c82
1.Add a new value `AllWithNonFinalized` in `enum BlocksPruning` which…
hzy1919 Sep 9, 2022
940d9db
Merge branch 'master' into Remove-discarded-blocks-and-states-from-da…
hzy1919 Sep 9, 2022
458e078
Make some corresponding adjustments based on the content in the conve…
hzy1919 Sep 10, 2022
a228701
Update client/db/src/lib.rs
hzy1919 Sep 14, 2022
6ae25a1
Apply suggestions from code review.
hzy1919 Sep 14, 2022
54a5a2b
Merge branch 'master' into Remove-discarded-blocks-and-states-from-da…
hzy1919 Sep 15, 2022
9e3e8eb
1.Change `blocks_pruning` to be like `state_pruning` .
hzy1919 Sep 21, 2022
ac58113
Merge branch 'master' into Remove-discarded-blocks-and-states-from-da…
hzy1919 Sep 22, 2022
4c3810c
Fmt and add some doc.
hzy1919 Sep 24, 2022
f4ea09b
Merge branch 'master' into Remove-discarded-blocks-and-states-from-da…
hzy1919 Sep 24, 2022
2b0c25c
Update client/cli/src/params/pruning_params.rs
hzy1919 Sep 25, 2022
6cbc62e
Update client/cli/src/params/pruning_params.rs
hzy1919 Sep 25, 2022
df5ffec
Update doc.
hzy1919 Sep 25, 2022
cb62c23
Change `new_test_with_tx_storage` to take `BlocksPruning`.
hzy1919 Sep 25, 2022
79afe8b
Merge branch 'master' into Remove-discarded-blocks-and-states-from-da…
hzy1919 Sep 25, 2022
3dff0f0
Fmt
hzy1919 Sep 25, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/node/cli/benches/block_production.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase {
database: DatabaseSource::RocksDb { path: root.join("db"), cache_size: 128 },
trie_cache_maximum_size: Some(64 * 1024 * 1024),
state_pruning: Some(PruningMode::ArchiveAll),
blocks_pruning: BlocksPruning::All,
blocks_pruning: BlocksPruning::AllWithNonFinalized,
chain_spec: spec,
wasm_method: WasmExecutionMethod::Compiled {
instantiation_strategy: WasmtimeInstantiationStrategy::PoolingCopyOnWrite,
Expand Down
2 changes: 1 addition & 1 deletion bin/node/cli/benches/transaction_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase {
database: DatabaseSource::RocksDb { path: root.join("db"), cache_size: 128 },
trie_cache_maximum_size: Some(64 * 1024 * 1024),
state_pruning: Some(PruningMode::ArchiveAll),
blocks_pruning: BlocksPruning::All,
blocks_pruning: BlocksPruning::AllWithNonFinalized,
chain_spec: spec,
wasm_method: WasmExecutionMethod::Interpreted,
// NOTE: we enforce the use of the native runtime to make the errors more debuggable
Expand Down
2 changes: 1 addition & 1 deletion bin/node/testing/src/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ impl BenchDb {
trie_cache_maximum_size: Some(16 * 1024 * 1024),
state_pruning: Some(PruningMode::ArchiveAll),
source: database_type.into_settings(dir.into()),
blocks_pruning: sc_client_db::BlocksPruning::All,
blocks_pruning: sc_client_db::BlocksPruning::AllWithNonFinalized,
};
let task_executor = TaskExecutor::new();

Expand Down
4 changes: 2 additions & 2 deletions client/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,11 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
/// Get the block pruning mode.
///
/// By default this is retrieved from `block_pruning` if it is available. Otherwise its
/// `BlocksPruning::All`.
/// `BlocksPruning::AllFinalized`.
fn blocks_pruning(&self) -> Result<BlocksPruning> {
self.pruning_params()
.map(|x| x.blocks_pruning())
.unwrap_or_else(|| Ok(BlocksPruning::All))
.unwrap_or_else(|| Ok(BlocksPruning::AllFinalized))
}

/// Get the chain ID (string).
Expand Down
7 changes: 4 additions & 3 deletions client/cli/src/params/pruning_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub struct PruningParams {
pub state_pruning: Option<String>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc for this param should be updated with the new final option.

/// Specify the number of finalized blocks to keep in the database.
///
/// Default is to keep all blocks.
/// Default is to keep all of finalized blocks.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we should add that 0 keep all blocks.
Generally this is not very intuitive.
Maybe we should do something like state_pruning: use the same string: 'archive' for 'keep_all' and 'final' fo 'keep_finalized'. (subject to change of name too).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm,,It's really not very intuitive that 0 keep all blocks. IMO it's a compromise approach now.
I agree that change blocks_pruning to be like state_pruning.

hzy1919 marked this conversation as resolved.
Show resolved Hide resolved
///
/// NOTE: only finalized blocks are subject for removal!
#[clap(alias = "keep-blocks", long, value_name = "COUNT")]
Expand All @@ -46,7 +46,7 @@ impl PruningParams {
.as_ref()
.map(|s| match s.as_str() {
"archive" => Ok(PruningMode::ArchiveAll),
"canonical" => Ok(PruningMode::ArchiveCanonical),
"final" => Ok(PruningMode::ArchiveCanonical),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the doc there is also a "archive-canonical" mention that did not look linked, I would maybe use it instead of "final".
But tbh I did not find a good name. Since it is the new default it should be shorter or more attractive than 'archive', but 'final' is not really saying much to me.
If anybody got other ideas?
(ideally former 'archive' becomes 'archive-all' and 'final' wil be 'archive', but it breaks compatibility)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it should be called archive-canonical

bc => bc
.parse()
.map_err(|_| error::Error::Input("Invalid pruning mode specified".to_string()))
Expand All @@ -58,8 +58,9 @@ impl PruningParams {
/// Get the block pruning value from the parameters
pub fn blocks_pruning(&self) -> error::Result<BlocksPruning> {
Ok(match self.blocks_pruning {
Some(0) => BlocksPruning::AllWithNonFinalized,
Some(n) => BlocksPruning::Some(n),
None => BlocksPruning::All,
None => BlocksPruning::AllFinalized,
})
}
}
2 changes: 1 addition & 1 deletion client/db/benches/state_access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ fn create_backend(config: BenchmarkConfig, temp_dir: &TempDir) -> Backend<Block>
trie_cache_maximum_size,
state_pruning: Some(PruningMode::ArchiveAll),
source: DatabaseSource::ParityDb { path },
blocks_pruning: BlocksPruning::All,
blocks_pruning: BlocksPruning::AllWithNonFinalized,
};

Backend::new(settings, 100).expect("Creates backend")
Expand Down
26 changes: 16 additions & 10 deletions client/db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,10 +320,12 @@ pub struct DatabaseSettings {
}

/// Block pruning settings.
#[derive(Debug, Clone, Copy)]
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum BlocksPruning {
/// Keep full block history.
All,
/// Keep full block history, including Non-Finalized blocks.
hzy1919 marked this conversation as resolved.
Show resolved Hide resolved
AllWithNonFinalized,
hzy1919 marked this conversation as resolved.
Show resolved Hide resolved
/// Keep full finalized block history.
AllFinalized,
hzy1919 marked this conversation as resolved.
Show resolved Hide resolved
/// Keep N recent finalized blocks.
Some(u32),
}
Expand Down Expand Up @@ -1707,12 +1709,16 @@ impl<Block: BlockT> Backend<Block> {
finalized: NumberFor<Block>,
displaced: &FinalizationOutcome<Block::Hash, NumberFor<Block>>,
) -> ClientResult<()> {
if let BlocksPruning::Some(blocks_pruning) = self.blocks_pruning {
// Always keep the last finalized block
let keep = std::cmp::max(blocks_pruning, 1);
if finalized >= keep.into() {
let number = finalized.saturating_sub(keep.into());
self.prune_block(transaction, BlockId::<Block>::number(number))?;
if BlocksPruning::AllWithNonFinalized == self.blocks_pruning {
hzy1919 marked this conversation as resolved.
Show resolved Hide resolved
// Do nothing.
} else {
if let BlocksPruning::Some(blocks_pruning) = self.blocks_pruning {
// Always keep the last finalized block
let keep = std::cmp::max(blocks_pruning, 1);
if finalized >= keep.into() {
let number = finalized.saturating_sub(keep.into());
self.prune_block(transaction, BlockId::<Block>::number(number))?;
}
}

// Also discard all blocks from displaced branches
Expand Down Expand Up @@ -2506,7 +2512,7 @@ pub(crate) mod tests {
trie_cache_maximum_size: Some(16 * 1024 * 1024),
state_pruning: Some(PruningMode::blocks_pruning(1)),
source: DatabaseSource::Custom { db: backing, require_create_flag: false },
blocks_pruning: BlocksPruning::All,
blocks_pruning: BlocksPruning::AllFinalized,
},
0,
)
Expand Down
4 changes: 2 additions & 2 deletions client/service/test/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1199,7 +1199,7 @@ fn doesnt_import_blocks_that_revert_finality() {
DatabaseSettings {
trie_cache_maximum_size: Some(1 << 20),
state_pruning: Some(PruningMode::ArchiveAll),
blocks_pruning: BlocksPruning::All,
blocks_pruning: BlocksPruning::AllWithNonFinalized,
source: DatabaseSource::RocksDb { path: tmp.path().into(), cache_size: 1024 },
},
u64::MAX,
Expand Down Expand Up @@ -1425,7 +1425,7 @@ fn returns_status_for_pruned_blocks() {
DatabaseSettings {
trie_cache_maximum_size: Some(1 << 20),
state_pruning: Some(PruningMode::blocks_pruning(1)),
blocks_pruning: BlocksPruning::All,
blocks_pruning: BlocksPruning::AllFinalized,
source: DatabaseSource::RocksDb { path: tmp.path().into(), cache_size: 1024 },
},
u64::MAX,
Expand Down
2 changes: 1 addition & 1 deletion client/service/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ fn node_config<
database: DatabaseSource::RocksDb { path: root.join("db"), cache_size: 128 },
trie_cache_maximum_size: Some(16 * 1024 * 1024),
state_pruning: Default::default(),
blocks_pruning: BlocksPruning::All,
blocks_pruning: BlocksPruning::AllFinalized,
chain_spec: Box::new((*spec).clone()),
wasm_method: sc_service::config::WasmExecutionMethod::Interpreted,
wasm_runtime_overrides: Default::default(),
Expand Down
12 changes: 6 additions & 6 deletions client/state-db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ impl PruningMode {

impl Default for PruningMode {
fn default() -> Self {
PruningMode::ArchiveCanonical
PruningMode::Constrained(Default::default())
}
}

Expand Down Expand Up @@ -924,12 +924,12 @@ mod tests {
#[test]
fn pruning_mode_compatibility() {
for (created, reopened, expected) in [
(None, None, Ok(PruningMode::ArchiveCanonical)),
(None, Some(PruningMode::blocks_pruning(256)), Err(())),
(None, Some(PruningMode::blocks_pruning(128)), Err(())),
(None, Some(PruningMode::blocks_pruning(512)), Err(())),
(None, None, Ok(PruningMode::blocks_pruning(256))),
(None, Some(PruningMode::blocks_pruning(256)), Ok(PruningMode::blocks_pruning(256))),
(None, Some(PruningMode::blocks_pruning(128)), Ok(PruningMode::blocks_pruning(128))),
(None, Some(PruningMode::blocks_pruning(512)), Ok(PruningMode::blocks_pruning(512))),
(None, Some(PruningMode::ArchiveAll), Err(())),
(None, Some(PruningMode::ArchiveCanonical), Ok(PruningMode::ArchiveCanonical)),
(None, Some(PruningMode::ArchiveCanonical), Err(())),
(Some(PruningMode::blocks_pruning(256)), None, Ok(PruningMode::blocks_pruning(256))),
(
Some(PruningMode::blocks_pruning(256)),
Expand Down