From af2e30e383b48302441f063929d4e69959db9830 Mon Sep 17 00:00:00 2001
From: Nazar Mokrynskyi <nazar@mokrynskyi.com>
Date: Wed, 10 Jan 2024 13:56:44 +0200
Subject: [PATCH] Improve storage monitor API (#2899)
This removes the need to unnecessarily provide a very specific data
structure `DatabaseSource` and removes huge `sc-client-db` dependency
from storage monitor. It is now possible to use storage monitor with any
path.
P.S. I still strongly dislike that it pulls `clap` dependency for such a
small feature, but many other crates do as well, so nothing special
here.
---
Cargo.lock | 1 -
polkadot/cli/src/command.rs | 12 ++--
prdoc/pr_2899.prdoc | 10 ++++
substrate/bin/node/cli/src/service.rs | 18 +++---
substrate/client/storage-monitor/Cargo.toml | 3 +-
substrate/client/storage-monitor/src/lib.rs | 61 +++++++++------------
6 files changed, 55 insertions(+), 50 deletions(-)
create mode 100644 prdoc/pr_2899.prdoc
diff --git a/Cargo.lock b/Cargo.lock
index 3ad73feb5d5..e00c173228f 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -16587,7 +16587,6 @@ dependencies = [
"clap 4.4.14",
"fs4",
"log",
- "sc-client-db",
"sp-core",
"thiserror",
"tokio",
diff --git a/polkadot/cli/src/command.rs b/polkadot/cli/src/command.rs
index 9290ec584e4..1e25f6533f0 100644
--- a/polkadot/cli/src/command.rs
+++ b/polkadot/cli/src/command.rs
@@ -256,11 +256,13 @@ where
)
.map(|full| full.task_manager)?;
- sc_storage_monitor::StorageMonitorService::try_spawn(
- cli.storage_monitor,
- database_source,
- &task_manager.spawn_essential_handle(),
- )?;
+ if let Some(path) = database_source.path() {
+ sc_storage_monitor::StorageMonitorService::try_spawn(
+ cli.storage_monitor,
+ path.to_path_buf(),
+ &task_manager.spawn_essential_handle(),
+ )?;
+ }
Ok(task_manager)
})
diff --git a/prdoc/pr_2899.prdoc b/prdoc/pr_2899.prdoc
new file mode 100644
index 00000000000..0c7afc0ad08
--- /dev/null
+++ b/prdoc/pr_2899.prdoc
@@ -0,0 +1,10 @@
+title: Improve storage monitor API
+
+doc:
+ - audience: Node Dev
+ description: |
+ This removes the need to unnecessarily provide a very specific data structure DatabaseSource and removes huge
+ sc-client-db dependency from storage monitor. It is now possible to use storage monitor with any path.
+
+crates:
+ - name: sc-storage-monitor
diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs
index 5e6e794f732..67d21ee69ed 100644
--- a/substrate/bin/node/cli/src/service.rs
+++ b/substrate/bin/node/cli/src/service.rs
@@ -38,7 +38,7 @@ use sc_transaction_pool_api::OffchainTransactionPoolFactory;
use sp_api::ProvideRuntimeApi;
use sp_core::crypto::Pair;
use sp_runtime::{generic, traits::Block as BlockT, SaturatedConversion};
-use std::sync::Arc;
+use std::{path::Path, sync::Arc};
/// Host functions required for kitchensink runtime and Substrate node.
#[cfg(not(feature = "runtime-benchmarks"))]
@@ -769,16 +769,18 @@ pub fn new_full_base(
/// Builds a new service for a full client.
pub fn new_full(config: Configuration, cli: Cli) -> Result<TaskManager, ServiceError> {
let mixnet_config = cli.mixnet_params.config(config.role.is_authority());
- let database_source = config.database.clone();
+ let database_path = config.database.path().map(Path::to_path_buf);
let task_manager = new_full_base(config, mixnet_config, cli.no_hardware_benchmarks, |_, _| ())
.map(|NewFullBase { task_manager, .. }| task_manager)?;
- sc_storage_monitor::StorageMonitorService::try_spawn(
- cli.storage_monitor,
- database_source,
- &task_manager.spawn_essential_handle(),
- )
- .map_err(|e| ServiceError::Application(e.into()))?;
+ if let Some(database_path) = database_path {
+ sc_storage_monitor::StorageMonitorService::try_spawn(
+ cli.storage_monitor,
+ database_path,
+ &task_manager.spawn_essential_handle(),
+ )
+ .map_err(|e| ServiceError::Application(e.into()))?;
+ }
Ok(task_manager)
}
diff --git a/substrate/client/storage-monitor/Cargo.toml b/substrate/client/storage-monitor/Cargo.toml
index 2b46ccbcdc1..7185c62e276 100644
--- a/substrate/client/storage-monitor/Cargo.toml
+++ b/substrate/client/storage-monitor/Cargo.toml
@@ -15,7 +15,6 @@ workspace = true
clap = { version = "4.4.14", features = ["derive", "string"] }
log = "0.4.17"
fs4 = "0.7.0"
-sc-client-db = { path = "../db", default-features = false }
sp-core = { path = "../../primitives/core" }
-tokio = "1.22.0"
+tokio = { version = "1.22.0", features = ["time"] }
thiserror = "1.0.48"
diff --git a/substrate/client/storage-monitor/src/lib.rs b/substrate/client/storage-monitor/src/lib.rs
index b88b66d2d60..28d9063e5e4 100644
--- a/substrate/client/storage-monitor/src/lib.rs
+++ b/substrate/client/storage-monitor/src/lib.rs
@@ -17,7 +17,6 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.
use clap::Args;
-use sc_client_db::DatabaseSource;
use sp_core::traits::SpawnEssentialNamed;
use std::{
io,
@@ -70,43 +69,37 @@ impl StorageMonitorService {
/// Creates new StorageMonitorService for given client config
pub fn try_spawn(
parameters: StorageMonitorParams,
- database: DatabaseSource,
+ path: PathBuf,
spawner: &impl SpawnEssentialNamed,
) -> Result<()> {
- Ok(match (parameters.threshold, database.path()) {
- (0, _) => {
- log::info!(
- target: LOG_TARGET,
- "StorageMonitorService: threshold `0` given, storage monitoring disabled",
- );
- },
- (_, None) => {
- log::warn!(
- target: LOG_TARGET,
- "StorageMonitorService: no database path to observe",
- );
- },
- (threshold, Some(path)) => {
- log::debug!(
- target: LOG_TARGET,
- "Initializing StorageMonitorService for db path: {path:?}",
- );
-
- Self::check_free_space(&path, threshold)?;
+ if parameters.threshold == 0 {
+ log::info!(
+ target: LOG_TARGET,
+ "StorageMonitorService: threshold `0` given, storage monitoring disabled",
+ );
+ } else {
+ log::debug!(
+ target: LOG_TARGET,
+ "Initializing StorageMonitorService for db path: {}",
+ path.display()
+ );
+
+ Self::check_free_space(&path, parameters.threshold)?;
+
+ let storage_monitor_service = StorageMonitorService {
+ path,
+ threshold: parameters.threshold,
+ polling_period: Duration::from_secs(parameters.polling_period.into()),
+ };
- let storage_monitor_service = StorageMonitorService {
- path: path.to_path_buf(),
- threshold,
- polling_period: Duration::from_secs(parameters.polling_period.into()),
- };
+ spawner.spawn_essential(
+ "storage-monitor",
+ None,
+ Box::pin(storage_monitor_service.run()),
+ );
+ }
- spawner.spawn_essential(
- "storage-monitor",
- None,
- Box::pin(storage_monitor_service.run()),
- );
- },
- })
+ Ok(())
}
/// Main monitoring loop, intended to be spawned as essential task. Quits if free space drop
--
GitLab