From ff816f2e9c21379e071965f2f85a17cf13ee63ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 26 Dec 2022 17:37:09 +0100 Subject: [PATCH] Make CLI state pruning optional again (#13017) * Make CLI state pruning optional again The state pruning setting is stored in the database when it is created. In later runs it is fine to drop the `--state-pruning` CLI argument as the setting is stored in the database. The state db will only return an error if the stored state pruning doesn't match the state pruning given via CLI. Recently we improved the state pruning CLI handling and accidentally made the state pruning value always present (as we set some default value for the clap). If we could find out if a user has passed a value or the default value was taken, we could keep the default value in the CLI interface, but clap isn't supporting this right now. So, we need to go back and make `state_pruning` an optional with the default written into the docs. It also adds a test to ensure that we don't break this behavior again. * More docs --- bin/node/cli/tests/common.rs | 6 ++- .../cli/tests/remember_state_pruning_works.rs | 38 +++++++++++++++++ client/cli/src/params/pruning_params.rs | 41 +++++++++++++++---- 3 files changed, 75 insertions(+), 10 deletions(-) create mode 100644 bin/node/cli/tests/remember_state_pruning_works.rs diff --git a/bin/node/cli/tests/common.rs b/bin/node/cli/tests/common.rs index 358c09779d59a..c8f9e2b3d3c82 100644 --- a/bin/node/cli/tests/common.rs +++ b/bin/node/cli/tests/common.rs @@ -161,6 +161,7 @@ pub fn find_ws_url_from_output(read: impl Read + Send) -> (String, String) { let line = line.expect("failed to obtain next line from stdout for WS address discovery"); data.push_str(&line); + data.push_str("\n"); // does the line contain our port (we expect this specific output from substrate). let sock_addr = match line.split_once("Running JSON-RPC WS server: addr=") { @@ -170,7 +171,10 @@ pub fn find_ws_url_from_output(read: impl Read + Send) -> (String, String) { Some(format!("ws://{}", sock_addr)) }) - .expect("We should get a WebSocket address"); + .unwrap_or_else(|| { + eprintln!("Observed node output:\n{}", data); + panic!("We should get a WebSocket address") + }); (ws_url, data) } diff --git a/bin/node/cli/tests/remember_state_pruning_works.rs b/bin/node/cli/tests/remember_state_pruning_works.rs new file mode 100644 index 0000000000000..5b8e34cc7a00d --- /dev/null +++ b/bin/node/cli/tests/remember_state_pruning_works.rs @@ -0,0 +1,38 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +use tempfile::tempdir; + +pub mod common; + +#[tokio::test] +#[cfg(unix)] +async fn remember_state_pruning_works() { + let base_path = tempdir().expect("could not create a temp dir"); + + // First run with `--state-pruning=archive`. + common::run_node_for_a_while( + base_path.path(), + &["--dev", "--state-pruning=archive", "--no-hardware-benchmarks"], + ) + .await; + + // Then run again without specifying the state pruning. + // This should load state pruning settings from the db. + common::run_node_for_a_while(base_path.path(), &["--dev", "--no-hardware-benchmarks"]).await; +} diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index 7e50f53d7169a..fd01ba67bab7d 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -28,21 +28,44 @@ pub struct PruningParams { /// This mode specifies when the block's state (ie, storage) /// should be pruned (ie, removed) from the database. /// + /// This setting can only be set on the first creation of the database. Every subsequent run + /// will load the pruning mode from the database and will error if the stored mode doesn't + /// match this CLI value. It is fine to drop this CLI flag for subsequent runs. + /// /// Possible values: - /// 'archive' Keep the state of all blocks. - /// 'archive-canonical' Keep only the state of finalized blocks. - /// number Keep the state of the last number of finalized blocks. - #[arg(alias = "pruning", long, value_name = "PRUNING_MODE", default_value = "256")] - pub state_pruning: DatabasePruningMode, + /// + /// - archive: + /// + /// Keep the state of all blocks. + /// + /// - 'archive-canonical' + /// + /// Keep only the state of finalized blocks. + /// + /// - number + /// + /// Keep the state of the last number of finalized blocks. + /// + /// [default: 256] + #[arg(alias = "pruning", long, value_name = "PRUNING_MODE")] + pub state_pruning: Option, /// Specify the blocks pruning mode. /// /// This mode specifies when the block's body (including justifications) /// should be pruned (ie, removed) from the database. /// /// Possible values: - /// 'archive' Keep all blocks. - /// 'archive-canonical' Keep only finalized blocks. - /// number Keep the last `number` of finalized blocks. + /// - 'archive' + /// + /// Keep all blocks. + /// + /// - 'archive-canonical' + /// + /// Keep only finalized blocks. + /// + /// - number + /// + /// Keep the last `number` of finalized blocks. #[arg( alias = "keep-blocks", long, @@ -55,7 +78,7 @@ pub struct PruningParams { impl PruningParams { /// Get the pruning value from the parameters pub fn state_pruning(&self) -> error::Result> { - Ok(Some(self.state_pruning.into())) + Ok(self.state_pruning.map(|v| v.into())) } /// Get the block pruning value from the parameters