From 34577eb334aa87dd883a6bfaf013663df278c193 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Thu, 5 Jan 2023 15:40:02 +0100 Subject: [PATCH 01/23] service: storage monitor added Storage monitor added. It uses `notify` create to get notifications about any changes to monitored path (which is database path). Notifications are consumed in essential task which terminates when available storage space drops below given threshold. Closes: #12399 --- Cargo.lock | 91 ++++++++++++++- bin/node/cli/benches/block_production.rs | 1 + bin/node/cli/benches/transaction_pool.rs | 1 + client/cli/src/config.rs | 11 ++ client/cli/src/params/database_params.rs | 15 ++- client/cli/src/runner.rs | 1 + client/service/Cargo.toml | 2 + client/service/src/builder.rs | 14 ++- client/service/src/config.rs | 3 + client/service/src/lib.rs | 1 + client/service/src/storage_monitor.rs | 143 +++++++++++++++++++++++ client/service/test/src/lib.rs | 1 + 12 files changed, 275 insertions(+), 9 deletions(-) create mode 100644 client/service/src/storage_monitor.rs diff --git a/Cargo.lock b/Cargo.lock index f82343e442a89..e1ac379997c6b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1179,7 +1179,7 @@ dependencies = [ "cfg-if", "crossbeam-utils", "lazy_static", - "memoffset", + "memoffset 0.6.4", "scopeguard", ] @@ -2826,6 +2826,26 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8e04e2fd2b8188ea827b32ef11de88377086d690286ab35747ef7f9bf3ccb590" +[[package]] +name = "inotify" +version = "0.9.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8069d3ec154eb856955c1c0fbffefbf5f3c40a104ec912d4797314c1801abff" +dependencies = [ + "bitflags", + "inotify-sys", + "libc", +] + +[[package]] +name = "inotify-sys" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e05c02b5e89bff3b946cedeca278abc628fe811e604f027c45a8aa3cf793d0eb" +dependencies = [ + "libc", +] + [[package]] name = "instant" version = "0.1.12" @@ -3161,6 +3181,26 @@ dependencies = [ "substrate-wasm-builder", ] +[[package]] +name = "kqueue" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2c8fc60ba15bf51257aa9807a48a61013db043fcf3a78cb0d916e8e396dcad98" +dependencies = [ + "kqueue-sys", + "libc", +] + +[[package]] +name = "kqueue-sys" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8367585489f01bc55dd27404dcf56b95e6da061a256a666ab23be9ba96a2e587" +dependencies = [ + "bitflags", + "libc", +] + [[package]] name = "kvdb" version = "0.13.0" @@ -3214,9 +3254,9 @@ checksum = "3576a87f2ba00f6f106fdfcd16db1d698d648a26ad8e0573cad8537c3c362d2a" [[package]] name = "libc" -version = "0.2.126" +version = "0.2.139" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "349d5a591cd28b49e1d1037471617a32ddcda5731b99419008085f72d5a53836" +checksum = "201de327520df007757c1f0adce6e827fe8562fbc28bfd9c15571c66ca1f5f79" [[package]] name = "libgit2-sys" @@ -3848,6 +3888,15 @@ dependencies = [ "autocfg", ] +[[package]] +name = "memoffset" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5de893c32cde5f383baa4c04c5d6dbdd735cfd4a794b0debdb2bb1b421da5ff4" +dependencies = [ + "autocfg", +] + [[package]] name = "memory-db" version = "0.31.0" @@ -4173,7 +4222,7 @@ dependencies = [ "cc", "cfg-if", "libc", - "memoffset", + "memoffset 0.6.4", ] [[package]] @@ -4187,6 +4236,20 @@ dependencies = [ "libc", ] +[[package]] +name = "nix" +version = "0.26.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46a58d1d356c6597d08cde02c2f09d785b09e28711837b1ed667dc652c08a694" +dependencies = [ + "bitflags", + "cfg-if", + "libc", + "memoffset 0.7.1", + "pin-utils", + "static_assertions", +] + [[package]] name = "node-bench" version = "0.9.0-dev" @@ -4551,6 +4614,22 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" +[[package]] +name = "notify" +version = "5.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed2c66da08abae1c024c01d635253e402341b4060a12e99b31c7594063bf490a" +dependencies = [ + "bitflags", + "filetime", + "inotify", + "kqueue", + "libc", + "mio", + "walkdir", + "winapi", +] + [[package]] name = "nu-ansi-term" version = "0.46.0" @@ -8182,6 +8261,8 @@ dependencies = [ "futures-timer", "jsonrpsee", "log", + "nix 0.26.1", + "notify", "parity-scale-codec", "parking_lot 0.12.1", "pin-project", @@ -11248,7 +11329,7 @@ dependencies = [ "log", "mach", "memfd", - "memoffset", + "memoffset 0.6.4", "paste", "rand 0.8.5", "rustix", diff --git a/bin/node/cli/benches/block_production.rs b/bin/node/cli/benches/block_production.rs index 4fcebb123d9e3..9576e7a61d59b 100644 --- a/bin/node/cli/benches/block_production.rs +++ b/bin/node/cli/benches/block_production.rs @@ -113,6 +113,7 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase { base_path: Some(base_path), informant_output_format: Default::default(), wasm_runtime_overrides: None, + available_storage_theshold: 1000, }; node_cli::service::new_full_base(config, false, |_, _| ()) diff --git a/bin/node/cli/benches/transaction_pool.rs b/bin/node/cli/benches/transaction_pool.rs index a8839642ddc26..de83ede2f9123 100644 --- a/bin/node/cli/benches/transaction_pool.rs +++ b/bin/node/cli/benches/transaction_pool.rs @@ -106,6 +106,7 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase { base_path: Some(base_path), informant_output_format: Default::default(), wasm_runtime_overrides: None, + available_storage_theshold: 1000, }; node_cli::service::new_full_base(config, false, |_, _| ()).expect("Creates node") diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index 77689708a231f..8789e6ce29308 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -191,6 +191,16 @@ pub trait CliConfiguration: Sized { .unwrap_or_else(|| Ok((None, KeystoreConfig::InMemory))) } + /// Get the database available storage space threshold. + /// + /// By default this is retrieved from `DatabaseParams` if it is available. + fn database_storage_threshold(&self) -> Result { + Ok(self + .database_params() + .map(|x| x.database_storage_threshold()) + .unwrap_or_default()) + } + /// Get the database cache size. /// /// By default this is retrieved from `DatabaseParams` if it is available. Otherwise its `None`. @@ -562,6 +572,7 @@ pub trait CliConfiguration: Sized { base_path: Some(base_path), informant_output_format: Default::default(), runtime_cache_size, + available_storage_theshold: self.database_storage_threshold()?, }) } diff --git a/client/cli/src/params/database_params.rs b/client/cli/src/params/database_params.rs index fdd3622580a6d..d4e69ce2699cf 100644 --- a/client/cli/src/params/database_params.rs +++ b/client/cli/src/params/database_params.rs @@ -19,7 +19,7 @@ use crate::arg_enums::Database; use clap::Args; -/// Parameters for block import. +/// Parameters for database #[derive(Debug, Clone, PartialEq, Args)] pub struct DatabaseParams { /// Select database backend to use. @@ -29,10 +29,16 @@ pub struct DatabaseParams { /// Limit the memory the database cache can use. #[arg(long = "db-cache", value_name = "MiB")] pub database_cache_size: Option, + + /// Required available space on database storage. If available space for DB storage drops below + /// the given threshold, node will be gracefully terminated. If `0` is given monitoring will be + /// disabled. + #[arg(long = "db-storage-threshold", value_name = "MB", default_value_t = 1000)] + pub database_storage_threshold: u64, } impl DatabaseParams { - /// Limit the memory the database cache can use. + /// Database backend pub fn database(&self) -> Option { self.database } @@ -41,4 +47,9 @@ impl DatabaseParams { pub fn database_cache_size(&self) -> Option { self.database_cache_size } + + /// Available storage space threshold + pub fn database_storage_threshold(&self) -> u64 { + self.database_storage_threshold + } } diff --git a/client/cli/src/runner.rs b/client/cli/src/runner.rs index 1a532b3bbc6fb..049e2b05a0f14 100644 --- a/client/cli/src/runner.rs +++ b/client/cli/src/runner.rs @@ -367,6 +367,7 @@ mod tests { base_path: None, informant_output_format: Default::default(), runtime_cache_size: 2, + available_storage_theshold: 1000, }, runtime, ) diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index 45b1c02620fb2..28befbf2127fa 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -80,6 +80,8 @@ tokio = { version = "1.22.0", features = ["time", "rt-multi-thread", "parking_lo tempfile = "3.1.0" directories = "4.0.1" static_init = "1.0.3" +nix = { version = "0.26.1", features = ["fs"] } +notify = { version = "5.0.0", default-features = false, feature=["macos_kqueue"] } [dev-dependencies] substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/runtime/client" } diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 1f94f96fae89e..7461d11d7dc8f 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -22,8 +22,10 @@ use crate::{ config::{Configuration, KeystoreConfig, PrometheusConfig}, error::Error, metrics::MetricsService, - start_rpc_servers, BuildGenesisBlock, GenesisBlockBuilder, RpcHandlers, SpawnTaskHandle, - TaskManager, TransactionPoolAdapter, + start_rpc_servers, + storage_monitor::StorageMonitorService, + BuildGenesisBlock, GenesisBlockBuilder, RpcHandlers, SpawnTaskHandle, TaskManager, + TransactionPoolAdapter, }; use futures::{channel::oneshot, future::ready, FutureExt, StreamExt}; use jsonrpsee::RpcModule; @@ -534,6 +536,14 @@ where metrics_service.run(client.clone(), transaction_pool.clone(), network.clone()), ); + if let Some(storage_monitor_service) = StorageMonitorService::new_for_config(&config)? { + task_manager.spawn_essential_handle().spawn( + "storage-monitor", + None, + storage_monitor_service.run(), + ) + } + let rpc_id_provider = config.rpc_id_provider.take(); // jsonrpsee RPC diff --git a/client/service/src/config.rs b/client/service/src/config.rs index efadb8433f63d..64e31cecf0413 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -149,6 +149,9 @@ pub struct Configuration { pub informant_output_format: sc_informant::OutputFormat, /// Maximum number of different runtime versions that can be cached. pub runtime_cache_size: u8, + /// Threshold (in megabytes) for available storage space associated with `base_path`. `0` means + /// no storage monitoring. + pub available_storage_theshold: u64, } /// Type for tasks spawned by the executor. diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs index 1529b822ade32..4c74edf287b2a 100644 --- a/client/service/src/lib.rs +++ b/client/service/src/lib.rs @@ -32,6 +32,7 @@ pub mod client; #[cfg(not(feature = "test-helpers"))] mod client; mod metrics; +mod storage_monitor; mod task_manager; use std::{collections::HashMap, net::SocketAddr}; diff --git a/client/service/src/storage_monitor.rs b/client/service/src/storage_monitor.rs new file mode 100644 index 0000000000000..65e2c61752745 --- /dev/null +++ b/client/service/src/storage_monitor.rs @@ -0,0 +1,143 @@ +// 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 crate::{config::Configuration, error::Error}; +use futures::StreamExt; +use nix::{errno::Errno, sys::statvfs::statvfs}; +use notify::{Config, Event, RecommendedWatcher, RecursiveMode, Watcher}; +use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver}; +use std::path::{Path, PathBuf}; + +const LOG_TARGET: &str = "storage-monitor"; + +pub struct StorageMonitorService { + /// watched path + path: PathBuf, + /// notify's events receiver + stream: TracingUnboundedReceiver>, + /// number of bytes that shall be free and available on the filesystem for watched path + threshold: u64, + /// keeps the ref for file system watcher + _watcher: RecommendedWatcher, +} + +impl StorageMonitorService { + /// creates new StorageMonitorService for given client config + pub fn new_for_config(config: &Configuration) -> Result, Error> { + Ok(match (config.available_storage_theshold, config.database.path()) { + (0, _) => { + log::info!( + target: LOG_TARGET, + "StorageMonitorService: threshold 0 given, storage monitoring disabled", + ); + None + }, + (_, None) => { + log::warn!( + target: LOG_TARGET, + "StorageMonitorService: no database path to observe", + ); + None + }, + (threshold, Some(path)) => { + let (sink, stream) = tracing_unbounded("mpsc_storage_monitor", 1024); + + let mut watcher = RecommendedWatcher::new( + move |res| { + sink.unbounded_send(res).unwrap(); + }, + Config::default(), + ) + .map_err(|e| Error::Other(format!("Could not create fs watcher {e}")))?; + + watcher + .watch(path.as_ref(), RecursiveMode::Recursive) + .map_err(|e| Error::Other(format!("Could not start fs watcher {e}")))?; + + log::debug!( + target: LOG_TARGET, + "Initializing StorageMonitorService for db path: {:?}", + path, + ); + + Self::check_free_space(&path, threshold)?; + + Some(StorageMonitorService { + path: path.to_path_buf(), + stream, + threshold, + _watcher: watcher, + }) + }, + }) + } + + /// main monitoring loop, intended to be spawn as essential task + pub async fn run(mut self) { + while let Some(watch_event) = self.stream.next().await { + match watch_event { + Ok(_) => + if Self::check_free_space(&self.path, self.threshold).is_err() { + break + }, + Err(e) => { + println!("watch error: {:?}", e); + }, + } + } + } + + // returns free space in MB + fn free_space(path: &Path) -> Result { + let fs_stats = statvfs(path); + fs_stats.map(|stats| stats.blocks_available() * stats.block_size() / 1_000_000) + } + + // checks if the free space for given `path` is below given `threshold`. + // If not error is returned. + // System errors are silently ignored. + fn check_free_space(path: &Path, threshold: u64) -> Result<(), Error> { + match StorageMonitorService::free_space(path.as_ref()) { + Ok(available_space) => { + log::trace!( + target: LOG_TARGET, + "free:{:?} , threshold:{:?}", + available_space, + threshold, + ); + + if available_space < threshold { + let msg = format!( + "Available space {:?}MB for path {:?} dropped below threshold:{:?}MB , terminating...", + available_space, + path, + threshold, + ); + log::error!(target: LOG_TARGET, "{}", msg); + Err(Error::Other(msg)) + } else { + Ok(()) + } + }, + Err(e) => { + log::warn!(target: LOG_TARGET, "could not read available space: {:?}", e); + Ok(()) + }, + } + } +} diff --git a/client/service/test/src/lib.rs b/client/service/test/src/lib.rs index 5f75e3521e235..afd14afb3c578 100644 --- a/client/service/test/src/lib.rs +++ b/client/service/test/src/lib.rs @@ -265,6 +265,7 @@ fn node_config< base_path: Some(BasePath::new(root)), informant_output_format: Default::default(), runtime_cache_size: 2, + available_storage_theshold: 1000, } } From 37bbba421d6f77fd2ae92279e1dc9afe25cd60c1 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Fri, 6 Jan 2023 11:44:27 +0100 Subject: [PATCH 02/23] Cargo.lock updated --- Cargo.lock | 83 +++++++++++++++++++++++++++++++++++++++ client/service/Cargo.toml | 2 +- 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index a3dc04a39718f..31785d8223b7a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2465,6 +2465,15 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2022715d62ab30faffd124d40b76f4134a550a87792276512b18d63272333394" +[[package]] +name = "fsevent-sys" +version = "4.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76ee7a02da4d231650c7cea31349b889be2f45ddb3ef3032d2ec8185f6313fd2" +dependencies = [ + "libc", +] + [[package]] name = "funty" version = "2.0.0" @@ -3132,6 +3141,26 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8e04e2fd2b8188ea827b32ef11de88377086d690286ab35747ef7f9bf3ccb590" +[[package]] +name = "inotify" +version = "0.9.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8069d3ec154eb856955c1c0fbffefbf5f3c40a104ec912d4797314c1801abff" +dependencies = [ + "bitflags", + "inotify-sys", + "libc", +] + +[[package]] +name = "inotify-sys" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e05c02b5e89bff3b946cedeca278abc628fe811e604f027c45a8aa3cf793d0eb" +dependencies = [ + "libc", +] + [[package]] name = "instant" version = "0.1.12" @@ -3511,6 +3540,26 @@ dependencies = [ "substrate-wasm-builder", ] +[[package]] +name = "kqueue" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2c8fc60ba15bf51257aa9807a48a61013db043fcf3a78cb0d916e8e396dcad98" +dependencies = [ + "kqueue-sys", + "libc", +] + +[[package]] +name = "kqueue-sys" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8367585489f01bc55dd27404dcf56b95e6da061a256a666ab23be9ba96a2e587" +dependencies = [ + "bitflags", + "libc", +] + [[package]] name = "kvdb" version = "0.13.0" @@ -4633,6 +4682,20 @@ dependencies = [ "memoffset 0.6.5", ] +[[package]] +name = "nix" +version = "0.26.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46a58d1d356c6597d08cde02c2f09d785b09e28711837b1ed667dc652c08a694" +dependencies = [ + "bitflags", + "cfg-if", + "libc", + "memoffset 0.7.1", + "pin-utils", + "static_assertions", +] + [[package]] name = "node-bench" version = "0.9.0-dev" @@ -4996,6 +5059,24 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" +[[package]] +name = "notify" +version = "5.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed2c66da08abae1c024c01d635253e402341b4060a12e99b31c7594063bf490a" +dependencies = [ + "bitflags", + "crossbeam-channel", + "filetime", + "fsevent-sys", + "inotify", + "kqueue", + "libc", + "mio", + "walkdir", + "winapi", +] + [[package]] name = "nu-ansi-term" version = "0.46.0" @@ -8837,6 +8918,8 @@ dependencies = [ "futures-timer", "jsonrpsee", "log", + "nix 0.26.1", + "notify", "parity-scale-codec", "parking_lot 0.12.1", "pin-project", diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index 28befbf2127fa..d70b0c0051ee7 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -81,7 +81,7 @@ tempfile = "3.1.0" directories = "4.0.1" static_init = "1.0.3" nix = { version = "0.26.1", features = ["fs"] } -notify = { version = "5.0.0", default-features = false, feature=["macos_kqueue"] } +notify = "5.0.0" [dev-dependencies] substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/runtime/client" } From ba0f28f015afb2c81468573f6e957c992ea7be51 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 9 Jan 2023 09:03:20 +0100 Subject: [PATCH 03/23] misspell --- bin/node/cli/benches/block_production.rs | 2 +- bin/node/cli/benches/transaction_pool.rs | 2 +- client/cli/src/config.rs | 2 +- client/cli/src/runner.rs | 2 +- client/service/src/config.rs | 2 +- client/service/src/storage_monitor.rs | 4 ++-- client/service/test/src/lib.rs | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/bin/node/cli/benches/block_production.rs b/bin/node/cli/benches/block_production.rs index 9576e7a61d59b..5bded2e5562c5 100644 --- a/bin/node/cli/benches/block_production.rs +++ b/bin/node/cli/benches/block_production.rs @@ -113,7 +113,7 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase { base_path: Some(base_path), informant_output_format: Default::default(), wasm_runtime_overrides: None, - available_storage_theshold: 1000, + available_storage_threshold: 1000, }; node_cli::service::new_full_base(config, false, |_, _| ()) diff --git a/bin/node/cli/benches/transaction_pool.rs b/bin/node/cli/benches/transaction_pool.rs index de83ede2f9123..d9fc22cfbf466 100644 --- a/bin/node/cli/benches/transaction_pool.rs +++ b/bin/node/cli/benches/transaction_pool.rs @@ -106,7 +106,7 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase { base_path: Some(base_path), informant_output_format: Default::default(), wasm_runtime_overrides: None, - available_storage_theshold: 1000, + available_storage_threshold: 1000, }; node_cli::service::new_full_base(config, false, |_, _| ()).expect("Creates node") diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index 8789e6ce29308..b6fd097ce5f5b 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -572,7 +572,7 @@ pub trait CliConfiguration: Sized { base_path: Some(base_path), informant_output_format: Default::default(), runtime_cache_size, - available_storage_theshold: self.database_storage_threshold()?, + available_storage_threshold: self.database_storage_threshold()?, }) } diff --git a/client/cli/src/runner.rs b/client/cli/src/runner.rs index 7ded35654589d..b7b44f21ee485 100644 --- a/client/cli/src/runner.rs +++ b/client/cli/src/runner.rs @@ -367,7 +367,7 @@ mod tests { base_path: None, informant_output_format: Default::default(), runtime_cache_size: 2, - available_storage_theshold: 1000, + available_storage_threshold: 1000, }, runtime, ) diff --git a/client/service/src/config.rs b/client/service/src/config.rs index 64e31cecf0413..afae951240bed 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -151,7 +151,7 @@ pub struct Configuration { pub runtime_cache_size: u8, /// Threshold (in megabytes) for available storage space associated with `base_path`. `0` means /// no storage monitoring. - pub available_storage_theshold: u64, + pub available_storage_threshold: u64, } /// Type for tasks spawned by the executor. diff --git a/client/service/src/storage_monitor.rs b/client/service/src/storage_monitor.rs index 65e2c61752745..efba642cc90ff 100644 --- a/client/service/src/storage_monitor.rs +++ b/client/service/src/storage_monitor.rs @@ -39,7 +39,7 @@ pub struct StorageMonitorService { impl StorageMonitorService { /// creates new StorageMonitorService for given client config pub fn new_for_config(config: &Configuration) -> Result, Error> { - Ok(match (config.available_storage_theshold, config.database.path()) { + Ok(match (config.available_storage_threshold, config.database.path()) { (0, _) => { log::info!( target: LOG_TARGET, @@ -96,7 +96,7 @@ impl StorageMonitorService { break }, Err(e) => { - println!("watch error: {:?}", e); + log::error!(target: LOG_TARGET, "watch error: {:?}", e); }, } } diff --git a/client/service/test/src/lib.rs b/client/service/test/src/lib.rs index afd14afb3c578..4e24c8855dc1e 100644 --- a/client/service/test/src/lib.rs +++ b/client/service/test/src/lib.rs @@ -265,7 +265,7 @@ fn node_config< base_path: Some(BasePath::new(root)), informant_output_format: Default::default(), runtime_cache_size: 2, - available_storage_theshold: 1000, + available_storage_threshold: 1000, } } From e36068584c6f365c11d995c0935e5727e888b86d Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 9 Jan 2023 12:45:32 +0100 Subject: [PATCH 04/23] fs events throttling added --- client/service/src/storage_monitor.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/client/service/src/storage_monitor.rs b/client/service/src/storage_monitor.rs index efba642cc90ff..c29e53e70ee58 100644 --- a/client/service/src/storage_monitor.rs +++ b/client/service/src/storage_monitor.rs @@ -21,9 +21,13 @@ use futures::StreamExt; use nix::{errno::Errno, sys::statvfs::statvfs}; use notify::{Config, Event, RecommendedWatcher, RecursiveMode, Watcher}; use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver}; -use std::path::{Path, PathBuf}; +use std::{ + path::{Path, PathBuf}, + time::{Duration, Instant}, +}; const LOG_TARGET: &str = "storage-monitor"; +const THROTTLE_PERIOD: std::time::Duration = Duration::from_secs(2); pub struct StorageMonitorService { /// watched path @@ -32,6 +36,8 @@ pub struct StorageMonitorService { stream: TracingUnboundedReceiver>, /// number of bytes that shall be free and available on the filesystem for watched path threshold: u64, + /// timestamp of the most recent check + recent_check: Instant, /// keeps the ref for file system watcher _watcher: RecommendedWatcher, } @@ -81,6 +87,7 @@ impl StorageMonitorService { path: path.to_path_buf(), stream, threshold, + recent_check: Instant::now(), _watcher: watcher, }) }, @@ -92,8 +99,11 @@ impl StorageMonitorService { while let Some(watch_event) = self.stream.next().await { match watch_event { Ok(_) => - if Self::check_free_space(&self.path, self.threshold).is_err() { - break + if self.recent_check.elapsed() >= THROTTLE_PERIOD { + self.recent_check = Instant::now(); + if Self::check_free_space(&self.path, self.threshold).is_err() { + break + } }, Err(e) => { log::error!(target: LOG_TARGET, "watch error: {:?}", e); @@ -108,8 +118,8 @@ impl StorageMonitorService { fs_stats.map(|stats| stats.blocks_available() * stats.block_size() / 1_000_000) } - // checks if the free space for given `path` is below given `threshold`. - // If not error is returned. + // Checks if the amount of free space for given `path` is below given `threshold`. + // If it dropped below, error is returned. // System errors are silently ignored. fn check_free_space(path: &Path, threshold: u64) -> Result<(), Error> { match StorageMonitorService::free_space(path.as_ref()) { From fd5679f24691629079162bfcd0ac65833881522f Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 9 Jan 2023 12:57:05 +0100 Subject: [PATCH 05/23] minor updates --- client/service/src/storage_monitor.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/client/service/src/storage_monitor.rs b/client/service/src/storage_monitor.rs index c29e53e70ee58..4d1d3eec5dbae 100644 --- a/client/service/src/storage_monitor.rs +++ b/client/service/src/storage_monitor.rs @@ -29,6 +29,7 @@ use std::{ const LOG_TARGET: &str = "storage-monitor"; const THROTTLE_PERIOD: std::time::Duration = Duration::from_secs(2); +/// Storage monitor service: checks the available space for the filesystem for fiven path. pub struct StorageMonitorService { /// watched path path: PathBuf, @@ -43,7 +44,7 @@ pub struct StorageMonitorService { } impl StorageMonitorService { - /// creates new StorageMonitorService for given client config + /// Creates new StorageMonitorService for given client config pub fn new_for_config(config: &Configuration) -> Result, Error> { Ok(match (config.available_storage_threshold, config.database.path()) { (0, _) => { @@ -94,7 +95,8 @@ impl StorageMonitorService { }) } - /// main monitoring loop, intended to be spawn as essential task + /// Main monitoring loop, intended to be spawned as essential task. Quits if free space drop + /// below threshold. pub async fn run(mut self) { while let Some(watch_event) = self.stream.next().await { match watch_event { @@ -112,15 +114,15 @@ impl StorageMonitorService { } } - // returns free space in MB + /// Returns free space in MB, or error if statvfs failed. fn free_space(path: &Path) -> Result { let fs_stats = statvfs(path); fs_stats.map(|stats| stats.blocks_available() * stats.block_size() / 1_000_000) } - // Checks if the amount of free space for given `path` is below given `threshold`. - // If it dropped below, error is returned. - // System errors are silently ignored. + /// Checks if the amount of free space for given `path` is below given `threshold`. + /// If it dropped below, error is returned. + /// System errors are silently ignored. fn check_free_space(path: &Path, threshold: u64) -> Result<(), Error> { match StorageMonitorService::free_space(path.as_ref()) { Ok(available_space) => { From 217234fd1da2ce64e4eee09593716adb89b5e9c9 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 9 Jan 2023 13:17:38 +0100 Subject: [PATCH 06/23] filter out non mutating events --- client/service/src/storage_monitor.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/client/service/src/storage_monitor.rs b/client/service/src/storage_monitor.rs index 4d1d3eec5dbae..1adc39410d555 100644 --- a/client/service/src/storage_monitor.rs +++ b/client/service/src/storage_monitor.rs @@ -100,6 +100,9 @@ impl StorageMonitorService { pub async fn run(mut self) { while let Some(watch_event) = self.stream.next().await { match watch_event { + Ok( Event { kind: notify::EventKind::Access(_), .. } ) => { + //skip non mutating events + }, Ok(_) => if self.recent_check.elapsed() >= THROTTLE_PERIOD { self.recent_check = Instant::now(); From 8b3310b93ffa0e2c56bf8749fe3712e6869be873 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 9 Jan 2023 13:42:23 +0100 Subject: [PATCH 07/23] misspell --- client/service/src/storage_monitor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/service/src/storage_monitor.rs b/client/service/src/storage_monitor.rs index 1adc39410d555..898e5ebfea6f3 100644 --- a/client/service/src/storage_monitor.rs +++ b/client/service/src/storage_monitor.rs @@ -35,7 +35,7 @@ pub struct StorageMonitorService { path: PathBuf, /// notify's events receiver stream: TracingUnboundedReceiver>, - /// number of bytes that shall be free and available on the filesystem for watched path + /// number of megabytes that shall be free and available on the filesystem for watched path threshold: u64, /// timestamp of the most recent check recent_check: Instant, From 931339c5679822c896f6250119af73c7c18d1900 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Tue, 10 Jan 2023 11:20:24 +0000 Subject: [PATCH 08/23] ".git/.scripts/commands/fmt/fmt.sh" --- client/service/src/storage_monitor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/service/src/storage_monitor.rs b/client/service/src/storage_monitor.rs index 898e5ebfea6f3..23fc12950177c 100644 --- a/client/service/src/storage_monitor.rs +++ b/client/service/src/storage_monitor.rs @@ -100,7 +100,7 @@ impl StorageMonitorService { pub async fn run(mut self) { while let Some(watch_event) = self.stream.next().await { match watch_event { - Ok( Event { kind: notify::EventKind::Access(_), .. } ) => { + Ok(Event { kind: notify::EventKind::Access(_), .. }) => { //skip non mutating events }, Ok(_) => From c20015957ed552453e76c4266f1d2bd4b40647ed Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Thu, 12 Jan 2023 15:36:00 +0100 Subject: [PATCH 09/23] Update client/service/src/storage_monitor.rs Co-authored-by: Anton --- client/service/src/storage_monitor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/service/src/storage_monitor.rs b/client/service/src/storage_monitor.rs index 23fc12950177c..36c625f7b6465 100644 --- a/client/service/src/storage_monitor.rs +++ b/client/service/src/storage_monitor.rs @@ -123,7 +123,7 @@ impl StorageMonitorService { fs_stats.map(|stats| stats.blocks_available() * stats.block_size() / 1_000_000) } - /// Checks if the amount of free space for given `path` is below given `threshold`. + /// Checks if the amount of free space for given `path` is above given `threshold`. /// If it dropped below, error is returned. /// System errors are silently ignored. fn check_free_space(path: &Path, threshold: u64) -> Result<(), Error> { From f5501f22b2e1bce41eb14f15c76f74cc15873bf1 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Fri, 13 Jan 2023 19:27:45 +0100 Subject: [PATCH 10/23] storage-monitor crate added --- client/storage-monitor/Cargo.toml | 19 ++++++++ .../src/lib.rs} | 44 ++++++++++++++----- 2 files changed, 52 insertions(+), 11 deletions(-) create mode 100644 client/storage-monitor/Cargo.toml rename client/{service/src/storage_monitor.rs => storage-monitor/src/lib.rs} (77%) diff --git a/client/storage-monitor/Cargo.toml b/client/storage-monitor/Cargo.toml new file mode 100644 index 0000000000000..0bccf9e5b7b97 --- /dev/null +++ b/client/storage-monitor/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "storage-monitor" +version = "0.1.0" +authors = ["Parity Technologies "] +edition = "2021" +license = "GPL-3.0-or-later WITH Classpath-exception-2.0" +repository = "https://github.com/paritytech/substrate" +description = "BEEFY Client gadget for substrate" +homepage = "https://substrate.io" + +[dependencies] +clap = { version = "4.0.9", features = ["derive", "string"] } +futures = "0.3.21" +log = "0.4.17" +nix = { version = "0.26.1", features = ["fs"] } +notify = "5.0.0" +sc-client-db = { version = "0.10.0-dev", default-features = false, path = "../db" } +sc-utils = { version = "4.0.0-dev", path = "../utils" } +sp-core = { version = "7.0.0", path = "../../primitives/core" } diff --git a/client/service/src/storage_monitor.rs b/client/storage-monitor/src/lib.rs similarity index 77% rename from client/service/src/storage_monitor.rs rename to client/storage-monitor/src/lib.rs index 36c625f7b6465..05ecfd8022945 100644 --- a/client/service/src/storage_monitor.rs +++ b/client/storage-monitor/src/lib.rs @@ -16,11 +16,13 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::{config::Configuration, error::Error}; +use clap::Args; use futures::StreamExt; use nix::{errno::Errno, sys::statvfs::statvfs}; use notify::{Config, Event, RecommendedWatcher, RecursiveMode, Watcher}; +use sc_client_db::DatabaseSource; use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver}; +use sp_core::traits::SpawnEssentialNamed; use std::{ path::{Path, PathBuf}, time::{Duration, Instant}, @@ -29,6 +31,22 @@ use std::{ const LOG_TARGET: &str = "storage-monitor"; const THROTTLE_PERIOD: std::time::Duration = Duration::from_secs(2); +type Error = String; + +/// Parameters used to create the storage monitor. +#[derive(Default, Debug, Clone, Args)] +pub struct StorageMonitorParams { + /// Required available space on database storage. If available space for DB storage drops below + /// the given threshold, node will be gracefully terminated. If `0` is given monitoring will be + /// disabled. + #[arg(long = "db-storage-threshold", value_name = "MB", default_value_t = 1000)] + pub threshold: u64, + + /// How often available space is polled. + #[arg(long = "db-storage-polling-period", value_name = "SECONDS", default_value_t = 5, value_parser = clap::value_parser!(u32).range(1..))] + pub polling_period: u32, +} + /// Storage monitor service: checks the available space for the filesystem for fiven path. pub struct StorageMonitorService { /// watched path @@ -45,21 +63,19 @@ pub struct StorageMonitorService { impl StorageMonitorService { /// Creates new StorageMonitorService for given client config - pub fn new_for_config(config: &Configuration) -> Result, Error> { - Ok(match (config.available_storage_threshold, config.database.path()) { + pub fn try_spawn(parameters: StorageMonitorParams, database: DatabaseSource, spawner: &impl SpawnEssentialNamed) -> Result<(), Error> { + Ok(match (parameters.threshold, database.path()) { (0, _) => { log::info!( target: LOG_TARGET, "StorageMonitorService: threshold 0 given, storage monitoring disabled", ); - None }, (_, None) => { log::warn!( target: LOG_TARGET, "StorageMonitorService: no database path to observe", ); - None }, (threshold, Some(path)) => { let (sink, stream) = tracing_unbounded("mpsc_storage_monitor", 1024); @@ -70,11 +86,11 @@ impl StorageMonitorService { }, Config::default(), ) - .map_err(|e| Error::Other(format!("Could not create fs watcher {e}")))?; + .map_err(|e| format!("Could not create fs watcher {e}"))?; watcher .watch(path.as_ref(), RecursiveMode::Recursive) - .map_err(|e| Error::Other(format!("Could not start fs watcher {e}")))?; + .map_err(|e| format!("Could not start fs watcher {e}"))?; log::debug!( target: LOG_TARGET, @@ -84,20 +100,26 @@ impl StorageMonitorService { Self::check_free_space(&path, threshold)?; - Some(StorageMonitorService { + let storage_monitor_service = StorageMonitorService { path: path.to_path_buf(), stream, threshold, recent_check: Instant::now(), _watcher: watcher, - }) + }; + + spawner.spawn_essential( + "storage-monitor", + None, + Box::pin(storage_monitor_service.run()), + ); }, }) } /// Main monitoring loop, intended to be spawned as essential task. Quits if free space drop /// below threshold. - pub async fn run(mut self) { + async fn run(mut self) { while let Some(watch_event) = self.stream.next().await { match watch_event { Ok(Event { kind: notify::EventKind::Access(_), .. }) => { @@ -144,7 +166,7 @@ impl StorageMonitorService { threshold, ); log::error!(target: LOG_TARGET, "{}", msg); - Err(Error::Other(msg)) + Err(msg) } else { Ok(()) } From 45171dccb78df43a508d49c0ffaa12c46a19618e Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Fri, 13 Jan 2023 19:30:45 +0100 Subject: [PATCH 11/23] cleanup: configuration + service builder --- bin/node/cli/benches/block_production.rs | 1 - bin/node/cli/benches/transaction_pool.rs | 1 - client/cli/src/config.rs | 11 ----------- client/cli/src/params/database_params.rs | 11 ----------- client/cli/src/runner.rs | 1 - client/service/Cargo.toml | 2 -- client/service/src/builder.rs | 9 --------- client/service/src/config.rs | 3 --- client/service/src/lib.rs | 1 - client/service/test/src/lib.rs | 1 - 10 files changed, 41 deletions(-) diff --git a/bin/node/cli/benches/block_production.rs b/bin/node/cli/benches/block_production.rs index 5bded2e5562c5..4fcebb123d9e3 100644 --- a/bin/node/cli/benches/block_production.rs +++ b/bin/node/cli/benches/block_production.rs @@ -113,7 +113,6 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase { base_path: Some(base_path), informant_output_format: Default::default(), wasm_runtime_overrides: None, - available_storage_threshold: 1000, }; node_cli::service::new_full_base(config, false, |_, _| ()) diff --git a/bin/node/cli/benches/transaction_pool.rs b/bin/node/cli/benches/transaction_pool.rs index d9fc22cfbf466..a8839642ddc26 100644 --- a/bin/node/cli/benches/transaction_pool.rs +++ b/bin/node/cli/benches/transaction_pool.rs @@ -106,7 +106,6 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase { base_path: Some(base_path), informant_output_format: Default::default(), wasm_runtime_overrides: None, - available_storage_threshold: 1000, }; node_cli::service::new_full_base(config, false, |_, _| ()).expect("Creates node") diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index b6fd097ce5f5b..77689708a231f 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -191,16 +191,6 @@ pub trait CliConfiguration: Sized { .unwrap_or_else(|| Ok((None, KeystoreConfig::InMemory))) } - /// Get the database available storage space threshold. - /// - /// By default this is retrieved from `DatabaseParams` if it is available. - fn database_storage_threshold(&self) -> Result { - Ok(self - .database_params() - .map(|x| x.database_storage_threshold()) - .unwrap_or_default()) - } - /// Get the database cache size. /// /// By default this is retrieved from `DatabaseParams` if it is available. Otherwise its `None`. @@ -572,7 +562,6 @@ pub trait CliConfiguration: Sized { base_path: Some(base_path), informant_output_format: Default::default(), runtime_cache_size, - available_storage_threshold: self.database_storage_threshold()?, }) } diff --git a/client/cli/src/params/database_params.rs b/client/cli/src/params/database_params.rs index d4e69ce2699cf..06a154fd60867 100644 --- a/client/cli/src/params/database_params.rs +++ b/client/cli/src/params/database_params.rs @@ -29,12 +29,6 @@ pub struct DatabaseParams { /// Limit the memory the database cache can use. #[arg(long = "db-cache", value_name = "MiB")] pub database_cache_size: Option, - - /// Required available space on database storage. If available space for DB storage drops below - /// the given threshold, node will be gracefully terminated. If `0` is given monitoring will be - /// disabled. - #[arg(long = "db-storage-threshold", value_name = "MB", default_value_t = 1000)] - pub database_storage_threshold: u64, } impl DatabaseParams { @@ -47,9 +41,4 @@ impl DatabaseParams { pub fn database_cache_size(&self) -> Option { self.database_cache_size } - - /// Available storage space threshold - pub fn database_storage_threshold(&self) -> u64 { - self.database_storage_threshold - } } diff --git a/client/cli/src/runner.rs b/client/cli/src/runner.rs index b7b44f21ee485..c7a6f1f3c0f99 100644 --- a/client/cli/src/runner.rs +++ b/client/cli/src/runner.rs @@ -367,7 +367,6 @@ mod tests { base_path: None, informant_output_format: Default::default(), runtime_cache_size: 2, - available_storage_threshold: 1000, }, runtime, ) diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index d70b0c0051ee7..45b1c02620fb2 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -80,8 +80,6 @@ tokio = { version = "1.22.0", features = ["time", "rt-multi-thread", "parking_lo tempfile = "3.1.0" directories = "4.0.1" static_init = "1.0.3" -nix = { version = "0.26.1", features = ["fs"] } -notify = "5.0.0" [dev-dependencies] substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/runtime/client" } diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 7461d11d7dc8f..afcdc803cbd42 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -23,7 +23,6 @@ use crate::{ error::Error, metrics::MetricsService, start_rpc_servers, - storage_monitor::StorageMonitorService, BuildGenesisBlock, GenesisBlockBuilder, RpcHandlers, SpawnTaskHandle, TaskManager, TransactionPoolAdapter, }; @@ -536,14 +535,6 @@ where metrics_service.run(client.clone(), transaction_pool.clone(), network.clone()), ); - if let Some(storage_monitor_service) = StorageMonitorService::new_for_config(&config)? { - task_manager.spawn_essential_handle().spawn( - "storage-monitor", - None, - storage_monitor_service.run(), - ) - } - let rpc_id_provider = config.rpc_id_provider.take(); // jsonrpsee RPC diff --git a/client/service/src/config.rs b/client/service/src/config.rs index afae951240bed..efadb8433f63d 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -149,9 +149,6 @@ pub struct Configuration { pub informant_output_format: sc_informant::OutputFormat, /// Maximum number of different runtime versions that can be cached. pub runtime_cache_size: u8, - /// Threshold (in megabytes) for available storage space associated with `base_path`. `0` means - /// no storage monitoring. - pub available_storage_threshold: u64, } /// Type for tasks spawned by the executor. diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs index 4c74edf287b2a..1529b822ade32 100644 --- a/client/service/src/lib.rs +++ b/client/service/src/lib.rs @@ -32,7 +32,6 @@ pub mod client; #[cfg(not(feature = "test-helpers"))] mod client; mod metrics; -mod storage_monitor; mod task_manager; use std::{collections::HashMap, net::SocketAddr}; diff --git a/client/service/test/src/lib.rs b/client/service/test/src/lib.rs index 4e24c8855dc1e..5f75e3521e235 100644 --- a/client/service/test/src/lib.rs +++ b/client/service/test/src/lib.rs @@ -265,7 +265,6 @@ fn node_config< base_path: Some(BasePath::new(root)), informant_output_format: Default::default(), runtime_cache_size: 2, - available_storage_threshold: 1000, } } From f346394085cac55718732f0f2164b9e1a07781d8 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Fri, 13 Jan 2023 19:39:53 +0100 Subject: [PATCH 12/23] storage_monitor in custom service (wip) --- Cargo.lock | 17 +++++++++++++++-- Cargo.toml | 1 + bin/node/cli/Cargo.toml | 2 ++ bin/node/cli/src/cli.rs | 4 ++++ bin/node/cli/src/command.rs | 9 ++++++++- 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 31785d8223b7a..8963628985655 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4808,6 +4808,7 @@ dependencies = [ "sp-tracing", "sp-transaction-pool", "sp-transaction-storage-proof", + "storage-monitor", "substrate-build-script-utils", "substrate-frame-cli", "substrate-rpc-client", @@ -8918,8 +8919,6 @@ dependencies = [ "futures-timer", "jsonrpsee", "log", - "nix 0.26.1", - "notify", "parity-scale-codec", "parking_lot 0.12.1", "pin-project", @@ -10522,6 +10521,20 @@ dependencies = [ "rand 0.8.5", ] +[[package]] +name = "storage-monitor" +version = "0.1.0" +dependencies = [ + "clap 4.0.32", + "futures", + "log", + "nix 0.26.1", + "notify", + "sc-client-db", + "sc-utils", + "sp-core", +] + [[package]] name = "strsim" version = "0.10.0" diff --git a/Cargo.toml b/Cargo.toml index 8f55d8e527ecd..f7eedadd1e8d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,6 +64,7 @@ members = [ "client/service", "client/service/test", "client/state-db", + "client/storage-monitor", "client/sysinfo", "client/sync-state-rpc", "client/telemetry", diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index 6b50115fd9a00..a470454862de1 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -81,6 +81,7 @@ sc-executor = { version = "0.10.0-dev", path = "../../../client/executor" } sc-authority-discovery = { version = "0.10.0-dev", path = "../../../client/authority-discovery" } sc-sync-state-rpc = { version = "0.10.0-dev", path = "../../../client/sync-state-rpc" } sc-sysinfo = { version = "6.0.0-dev", path = "../../../client/sysinfo" } +storage-monitor = { version = "0.1.0", path = "../../../client/storage-monitor" } # frame dependencies frame-system = { version = "4.0.0-dev", path = "../../../frame/system" } @@ -138,6 +139,7 @@ substrate-frame-cli = { version = "4.0.0-dev", optional = true, path = "../../.. try-runtime-cli = { version = "0.10.0-dev", optional = true, path = "../../../utils/frame/try-runtime/cli" } sc-cli = { version = "0.10.0-dev", path = "../../../client/cli", optional = true } pallet-balances = { version = "4.0.0-dev", path = "../../../frame/balances" } +storage-monitor = { version = "0.1.0", path = "../../../client/storage-monitor" } [features] default = ["cli"] diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index bb7f8a4c60aa9..43e491f4099d9 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -36,6 +36,10 @@ pub struct Cli { /// telemetry, if telemetry is enabled. #[arg(long)] pub no_hardware_benchmarks: bool, + + #[allow(missing_docs)] + #[clap(flatten)] + pub storage_monitor: storage_monitor::StorageMonitorParams, } /// Possible subcommands of the main binary. diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index fd464bbc914a5..b55643649415f 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -85,10 +85,17 @@ pub fn run() -> Result<()> { match &cli.subcommand { None => { + //this part is clumsy PoC, still need some work (maybe pass cli to service::new_full ?) + let sm = cli.storage_monitor.clone(); let runner = cli.create_runner(&cli.run)?; runner.run_node_until_exit(|config| async move { + let db = config.database.clone(); + let tm = service::new_full(config, cli.no_hardware_benchmarks) - .map_err(sc_cli::Error::Service) + .map_err(sc_cli::Error::Service); + + storage_monitor::StorageMonitorService::try_spawn(sm, db, &tm.as_ref().unwrap().spawn_essential_handle())?; + tm }) }, Some(Subcommand::Inspect(cmd)) => { From eb49400c5d92cb7340bf2a6c2dfebc9e96e88a81 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Fri, 13 Jan 2023 20:02:14 +0100 Subject: [PATCH 13/23] copy-paste bad desc fixed --- client/storage-monitor/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/storage-monitor/Cargo.toml b/client/storage-monitor/Cargo.toml index 0bccf9e5b7b97..8fb3f2990e307 100644 --- a/client/storage-monitor/Cargo.toml +++ b/client/storage-monitor/Cargo.toml @@ -5,7 +5,7 @@ authors = ["Parity Technologies "] edition = "2021" license = "GPL-3.0-or-later WITH Classpath-exception-2.0" repository = "https://github.com/paritytech/substrate" -description = "BEEFY Client gadget for substrate" +description = "Storage monitor service for substrate" homepage = "https://substrate.io" [dependencies] From 86197668dcc5efd8c003fa1042f1e25d810da1ef Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 16 Jan 2023 10:46:51 +0100 Subject: [PATCH 14/23] notify removed --- Cargo.lock | 69 +------------------------------ client/storage-monitor/Cargo.toml | 2 +- client/storage-monitor/src/lib.rs | 61 +++++++-------------------- 3 files changed, 17 insertions(+), 115 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8963628985655..c15d9150eb0e7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2465,15 +2465,6 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2022715d62ab30faffd124d40b76f4134a550a87792276512b18d63272333394" -[[package]] -name = "fsevent-sys" -version = "4.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "76ee7a02da4d231650c7cea31349b889be2f45ddb3ef3032d2ec8185f6313fd2" -dependencies = [ - "libc", -] - [[package]] name = "funty" version = "2.0.0" @@ -3141,26 +3132,6 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8e04e2fd2b8188ea827b32ef11de88377086d690286ab35747ef7f9bf3ccb590" -[[package]] -name = "inotify" -version = "0.9.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f8069d3ec154eb856955c1c0fbffefbf5f3c40a104ec912d4797314c1801abff" -dependencies = [ - "bitflags", - "inotify-sys", - "libc", -] - -[[package]] -name = "inotify-sys" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e05c02b5e89bff3b946cedeca278abc628fe811e604f027c45a8aa3cf793d0eb" -dependencies = [ - "libc", -] - [[package]] name = "instant" version = "0.1.12" @@ -3540,26 +3511,6 @@ dependencies = [ "substrate-wasm-builder", ] -[[package]] -name = "kqueue" -version = "1.0.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2c8fc60ba15bf51257aa9807a48a61013db043fcf3a78cb0d916e8e396dcad98" -dependencies = [ - "kqueue-sys", - "libc", -] - -[[package]] -name = "kqueue-sys" -version = "1.0.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8367585489f01bc55dd27404dcf56b95e6da061a256a666ab23be9ba96a2e587" -dependencies = [ - "bitflags", - "libc", -] - [[package]] name = "kvdb" version = "0.13.0" @@ -5060,24 +5011,6 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" -[[package]] -name = "notify" -version = "5.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed2c66da08abae1c024c01d635253e402341b4060a12e99b31c7594063bf490a" -dependencies = [ - "bitflags", - "crossbeam-channel", - "filetime", - "fsevent-sys", - "inotify", - "kqueue", - "libc", - "mio", - "walkdir", - "winapi", -] - [[package]] name = "nu-ansi-term" version = "0.46.0" @@ -10529,10 +10462,10 @@ dependencies = [ "futures", "log", "nix 0.26.1", - "notify", "sc-client-db", "sc-utils", "sp-core", + "tokio", ] [[package]] diff --git a/client/storage-monitor/Cargo.toml b/client/storage-monitor/Cargo.toml index 8fb3f2990e307..9bfa17e5de22e 100644 --- a/client/storage-monitor/Cargo.toml +++ b/client/storage-monitor/Cargo.toml @@ -13,7 +13,7 @@ clap = { version = "4.0.9", features = ["derive", "string"] } futures = "0.3.21" log = "0.4.17" nix = { version = "0.26.1", features = ["fs"] } -notify = "5.0.0" sc-client-db = { version = "0.10.0-dev", default-features = false, path = "../db" } sc-utils = { version = "4.0.0-dev", path = "../utils" } sp-core = { version = "7.0.0", path = "../../primitives/core" } +tokio = "1.22.0" diff --git a/client/storage-monitor/src/lib.rs b/client/storage-monitor/src/lib.rs index 05ecfd8022945..06efcf89464dc 100644 --- a/client/storage-monitor/src/lib.rs +++ b/client/storage-monitor/src/lib.rs @@ -17,19 +17,15 @@ // along with this program. If not, see . use clap::Args; -use futures::StreamExt; use nix::{errno::Errno, sys::statvfs::statvfs}; -use notify::{Config, Event, RecommendedWatcher, RecursiveMode, Watcher}; use sc_client_db::DatabaseSource; -use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver}; use sp_core::traits::SpawnEssentialNamed; use std::{ path::{Path, PathBuf}, - time::{Duration, Instant}, + time::Duration, }; const LOG_TARGET: &str = "storage-monitor"; -const THROTTLE_PERIOD: std::time::Duration = Duration::from_secs(2); type Error = String; @@ -51,19 +47,19 @@ pub struct StorageMonitorParams { pub struct StorageMonitorService { /// watched path path: PathBuf, - /// notify's events receiver - stream: TracingUnboundedReceiver>, /// number of megabytes that shall be free and available on the filesystem for watched path threshold: u64, - /// timestamp of the most recent check - recent_check: Instant, - /// keeps the ref for file system watcher - _watcher: RecommendedWatcher, + /// storage space polling period (seconds) + pub polling_period: u32, } impl StorageMonitorService { /// Creates new StorageMonitorService for given client config - pub fn try_spawn(parameters: StorageMonitorParams, database: DatabaseSource, spawner: &impl SpawnEssentialNamed) -> Result<(), Error> { + pub fn try_spawn( + parameters: StorageMonitorParams, + database: DatabaseSource, + spawner: &impl SpawnEssentialNamed, + ) -> Result<(), Error> { Ok(match (parameters.threshold, database.path()) { (0, _) => { log::info!( @@ -78,20 +74,6 @@ impl StorageMonitorService { ); }, (threshold, Some(path)) => { - let (sink, stream) = tracing_unbounded("mpsc_storage_monitor", 1024); - - let mut watcher = RecommendedWatcher::new( - move |res| { - sink.unbounded_send(res).unwrap(); - }, - Config::default(), - ) - .map_err(|e| format!("Could not create fs watcher {e}"))?; - - watcher - .watch(path.as_ref(), RecursiveMode::Recursive) - .map_err(|e| format!("Could not start fs watcher {e}"))?; - log::debug!( target: LOG_TARGET, "Initializing StorageMonitorService for db path: {:?}", @@ -102,10 +84,8 @@ impl StorageMonitorService { let storage_monitor_service = StorageMonitorService { path: path.to_path_buf(), - stream, threshold, - recent_check: Instant::now(), - _watcher: watcher, + polling_period: parameters.polling_period, }; spawner.spawn_essential( @@ -119,23 +99,12 @@ impl StorageMonitorService { /// Main monitoring loop, intended to be spawned as essential task. Quits if free space drop /// below threshold. - async fn run(mut self) { - while let Some(watch_event) = self.stream.next().await { - match watch_event { - Ok(Event { kind: notify::EventKind::Access(_), .. }) => { - //skip non mutating events - }, - Ok(_) => - if self.recent_check.elapsed() >= THROTTLE_PERIOD { - self.recent_check = Instant::now(); - if Self::check_free_space(&self.path, self.threshold).is_err() { - break - } - }, - Err(e) => { - log::error!(target: LOG_TARGET, "watch error: {:?}", e); - }, - } + async fn run(self) { + loop { + tokio::time::sleep(Duration::from_secs(self.polling_period.into())).await; + if Self::check_free_space(&self.path, self.threshold).is_err() { + break + }; } } From a0cd44d2daa31d0043c435bd4ddba1c7a351ad6d Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 16 Jan 2023 12:07:14 +0100 Subject: [PATCH 15/23] storage_monitor added to node --- bin/node/cli/src/command.rs | 9 +-------- bin/node/cli/src/service.rs | 18 ++++++++++++------ client/service/src/builder.rs | 5 ++--- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index b55643649415f..9b59cfd26380a 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -86,16 +86,9 @@ pub fn run() -> Result<()> { match &cli.subcommand { None => { //this part is clumsy PoC, still need some work (maybe pass cli to service::new_full ?) - let sm = cli.storage_monitor.clone(); let runner = cli.create_runner(&cli.run)?; runner.run_node_until_exit(|config| async move { - let db = config.database.clone(); - let tm = - service::new_full(config, cli.no_hardware_benchmarks) - .map_err(sc_cli::Error::Service); - - storage_monitor::StorageMonitorService::try_spawn(sm, db, &tm.as_ref().unwrap().spawn_essential_handle())?; - tm + service::new_full(config, cli).map_err(sc_cli::Error::Service) }) }, Some(Subcommand::Inspect(cmd)) => { diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index cdee61af3f500..db71fe2694351 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -20,6 +20,7 @@ //! Service implementation. Specialized wrapper over substrate service. +use crate::Cli; use codec::Encode; use frame_system_rpc_runtime_api::AccountNonceApi; use futures::prelude::*; @@ -552,12 +553,17 @@ pub fn new_full_base( } /// Builds a new service for a full client. -pub fn new_full( - config: Configuration, - disable_hardware_benchmarks: bool, -) -> Result { - new_full_base(config, disable_hardware_benchmarks, |_, _| ()) - .map(|NewFullBase { task_manager, .. }| task_manager) +pub fn new_full(config: Configuration, cli: Cli) -> Result { + let database_source = config.database.clone(); + let task_manager = new_full_base(config, cli.no_hardware_benchmarks, |_, _| ()) + .map(|NewFullBase { task_manager, .. }| task_manager); + + storage_monitor::StorageMonitorService::try_spawn( + cli.storage_monitor, + database_source, + &task_manager.as_ref().unwrap().spawn_essential_handle(), + )?; + task_manager } #[cfg(test)] diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index afcdc803cbd42..1f94f96fae89e 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -22,9 +22,8 @@ use crate::{ config::{Configuration, KeystoreConfig, PrometheusConfig}, error::Error, metrics::MetricsService, - start_rpc_servers, - BuildGenesisBlock, GenesisBlockBuilder, RpcHandlers, SpawnTaskHandle, TaskManager, - TransactionPoolAdapter, + start_rpc_servers, BuildGenesisBlock, GenesisBlockBuilder, RpcHandlers, SpawnTaskHandle, + TaskManager, TransactionPoolAdapter, }; use futures::{channel::oneshot, future::ready, FutureExt, StreamExt}; use jsonrpsee::RpcModule; From a79af9a60f76fbc0f0afbc6b05d8b82fa2bac5e7 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 16 Jan 2023 19:24:47 +0100 Subject: [PATCH 16/23] fix for clippy --- client/storage-monitor/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/storage-monitor/src/lib.rs b/client/storage-monitor/src/lib.rs index 06efcf89464dc..011543c4d4b12 100644 --- a/client/storage-monitor/src/lib.rs +++ b/client/storage-monitor/src/lib.rs @@ -118,7 +118,7 @@ impl StorageMonitorService { /// If it dropped below, error is returned. /// System errors are silently ignored. fn check_free_space(path: &Path, threshold: u64) -> Result<(), Error> { - match StorageMonitorService::free_space(path.as_ref()) { + match StorageMonitorService::free_space(path) { Ok(available_space) => { log::trace!( target: LOG_TARGET, From 00e7264743e57b9b4e5d93b05b40831fb02afab7 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 16 Jan 2023 20:14:47 +0100 Subject: [PATCH 17/23] publish = false --- client/storage-monitor/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/client/storage-monitor/Cargo.toml b/client/storage-monitor/Cargo.toml index 9bfa17e5de22e..be62935cb5243 100644 --- a/client/storage-monitor/Cargo.toml +++ b/client/storage-monitor/Cargo.toml @@ -7,6 +7,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" repository = "https://github.com/paritytech/substrate" description = "Storage monitor service for substrate" homepage = "https://substrate.io" +publish = false [dependencies] clap = { version = "4.0.9", features = ["derive", "string"] } From 6c3eb1c857acf57e8c581c35eba0c10fa1ecd0fc Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Tue, 17 Jan 2023 10:09:53 +0100 Subject: [PATCH 18/23] Update bin/node/cli/src/command.rs Co-authored-by: Dmitry Markin --- bin/node/cli/src/command.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index 9b59cfd26380a..2ed8a2c754018 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -85,7 +85,6 @@ pub fn run() -> Result<()> { match &cli.subcommand { None => { - //this part is clumsy PoC, still need some work (maybe pass cli to service::new_full ?) let runner = cli.create_runner(&cli.run)?; runner.run_node_until_exit(|config| async move { service::new_full(config, cli).map_err(sc_cli::Error::Service) From 58dcf55b931901814cec8dcc54fe6f80e67d77b3 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 23 Jan 2023 06:38:17 +0100 Subject: [PATCH 19/23] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- bin/node/cli/src/service.rs | 7 ++++--- client/storage-monitor/Cargo.toml | 2 +- client/storage-monitor/src/lib.rs | 21 ++++++--------------- 3 files changed, 11 insertions(+), 19 deletions(-) diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index db71fe2694351..4287a4e7bef5d 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -556,14 +556,15 @@ pub fn new_full_base( pub fn new_full(config: Configuration, cli: Cli) -> Result { let database_source = config.database.clone(); let task_manager = new_full_base(config, cli.no_hardware_benchmarks, |_, _| ()) - .map(|NewFullBase { task_manager, .. }| task_manager); + .map(|NewFullBase { task_manager, .. }| task_manager)?; storage_monitor::StorageMonitorService::try_spawn( cli.storage_monitor, database_source, - &task_manager.as_ref().unwrap().spawn_essential_handle(), + &task_manager.spawn_essential_handle(), )?; - task_manager + + Ok(task_manager) } #[cfg(test)] diff --git a/client/storage-monitor/Cargo.toml b/client/storage-monitor/Cargo.toml index be62935cb5243..fcaf2eb77b745 100644 --- a/client/storage-monitor/Cargo.toml +++ b/client/storage-monitor/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "storage-monitor" +name = "sc-storage-monitor" version = "0.1.0" authors = ["Parity Technologies "] edition = "2021" diff --git a/client/storage-monitor/src/lib.rs b/client/storage-monitor/src/lib.rs index 011543c4d4b12..7eb973d2480f0 100644 --- a/client/storage-monitor/src/lib.rs +++ b/client/storage-monitor/src/lib.rs @@ -47,10 +47,10 @@ pub struct StorageMonitorParams { pub struct StorageMonitorService { /// watched path path: PathBuf, - /// number of megabytes that shall be free and available on the filesystem for watched path + /// number of megabytes that shall be free on the filesystem for watched path threshold: u64, /// storage space polling period (seconds) - pub polling_period: u32, + polling_period: u32, } impl StorageMonitorService { @@ -64,7 +64,7 @@ impl StorageMonitorService { (0, _) => { log::info!( target: LOG_TARGET, - "StorageMonitorService: threshold 0 given, storage monitoring disabled", + "StorageMonitorService: threshold `0` given, storage monitoring disabled", ); }, (_, None) => { @@ -110,8 +110,7 @@ impl StorageMonitorService { /// Returns free space in MB, or error if statvfs failed. fn free_space(path: &Path) -> Result { - let fs_stats = statvfs(path); - fs_stats.map(|stats| stats.blocks_available() * stats.block_size() / 1_000_000) + statvfs(path).map(|stats| stats.blocks_available() * stats.block_size() / 1_000_000) } /// Checks if the amount of free space for given `path` is above given `threshold`. @@ -122,19 +121,11 @@ impl StorageMonitorService { Ok(available_space) => { log::trace!( target: LOG_TARGET, - "free:{:?} , threshold:{:?}", - available_space, - threshold, + "free: {available_space} , threshold: {threshold}", ); if available_space < threshold { - let msg = format!( - "Available space {:?}MB for path {:?} dropped below threshold:{:?}MB , terminating...", - available_space, - path, - threshold, - ); - log::error!(target: LOG_TARGET, "{}", msg); + log::error!(target: LOG_TARGET, "Available space {available_space}MB for path `{}` dropped below threshold: {threshold}MB , terminating...", path.display()); Err(msg) } else { Ok(()) From 95b576bccfc12e0b9bf0da71cfcb1b44f9ecc20a Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 23 Jan 2023 15:49:14 +0100 Subject: [PATCH 20/23] crate name: storage-monitor -> sc-storage-monitor --- Cargo.lock | 32 +++++++++++++++++--------------- bin/node/cli/Cargo.toml | 4 ++-- bin/node/cli/src/cli.rs | 2 +- bin/node/cli/src/service.rs | 4 ++-- client/service/Cargo.toml | 1 + 5 files changed, 23 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5551ea9956507..ed4b62edbf29e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4734,6 +4734,7 @@ dependencies = [ "sc-rpc", "sc-service", "sc-service-test", + "sc-storage-monitor", "sc-sync-state-rpc", "sc-sysinfo", "sc-telemetry", @@ -4759,7 +4760,6 @@ dependencies = [ "sp-tracing", "sp-transaction-pool", "sp-transaction-storage-proof", - "storage-monitor", "substrate-build-script-utils", "substrate-frame-cli", "substrate-rpc-client", @@ -8875,6 +8875,7 @@ dependencies = [ "sc-rpc", "sc-rpc-server", "sc-rpc-spec-v2", + "sc-storage-monitor", "sc-sysinfo", "sc-telemetry", "sc-tracing", @@ -8953,6 +8954,21 @@ dependencies = [ "sp-core", ] +[[package]] +name = "sc-storage-monitor" +version = "0.1.0" +dependencies = [ + "clap 4.0.32", + "futures", + "log", + "nix 0.26.1", + "sc-client-db", + "sc-utils", + "sp-core", + "thiserror", + "tokio", +] + [[package]] name = "sc-sync-state-rpc" version = "0.10.0-dev" @@ -10455,20 +10471,6 @@ dependencies = [ "rand 0.8.5", ] -[[package]] -name = "storage-monitor" -version = "0.1.0" -dependencies = [ - "clap 4.0.32", - "futures", - "log", - "nix 0.26.1", - "sc-client-db", - "sc-utils", - "sp-core", - "tokio", -] - [[package]] name = "strsim" version = "0.10.0" diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index a470454862de1..df81e481ca0aa 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -81,7 +81,7 @@ sc-executor = { version = "0.10.0-dev", path = "../../../client/executor" } sc-authority-discovery = { version = "0.10.0-dev", path = "../../../client/authority-discovery" } sc-sync-state-rpc = { version = "0.10.0-dev", path = "../../../client/sync-state-rpc" } sc-sysinfo = { version = "6.0.0-dev", path = "../../../client/sysinfo" } -storage-monitor = { version = "0.1.0", path = "../../../client/storage-monitor" } +sc-storage-monitor = { version = "0.1.0", path = "../../../client/storage-monitor" } # frame dependencies frame-system = { version = "4.0.0-dev", path = "../../../frame/system" } @@ -139,7 +139,7 @@ substrate-frame-cli = { version = "4.0.0-dev", optional = true, path = "../../.. try-runtime-cli = { version = "0.10.0-dev", optional = true, path = "../../../utils/frame/try-runtime/cli" } sc-cli = { version = "0.10.0-dev", path = "../../../client/cli", optional = true } pallet-balances = { version = "4.0.0-dev", path = "../../../frame/balances" } -storage-monitor = { version = "0.1.0", path = "../../../client/storage-monitor" } +sc-storage-monitor = { version = "0.1.0", path = "../../../client/storage-monitor" } [features] default = ["cli"] diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 43e491f4099d9..7bea336c8e41a 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -39,7 +39,7 @@ pub struct Cli { #[allow(missing_docs)] #[clap(flatten)] - pub storage_monitor: storage_monitor::StorageMonitorParams, + pub storage_monitor: sc_storage_monitor::StorageMonitorParams, } /// Possible subcommands of the main binary. diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 4287a4e7bef5d..f53cdad90f1e4 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -558,12 +558,12 @@ pub fn new_full(config: Configuration, cli: Cli) -> Result Date: Mon, 23 Jan 2023 15:52:01 +0100 Subject: [PATCH 21/23] error handling improved --- client/service/src/error.rs | 3 +++ client/storage-monitor/Cargo.toml | 1 + client/storage-monitor/src/lib.rs | 34 +++++++++++++++++++++++-------- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/client/service/src/error.rs b/client/service/src/error.rs index 001a83922d776..ec2951193964c 100644 --- a/client/service/src/error.rs +++ b/client/service/src/error.rs @@ -48,6 +48,9 @@ pub enum Error { #[error(transparent)] Telemetry(#[from] sc_telemetry::Error), + #[error(transparent)] + Storage(#[from] sc_storage_monitor::Error), + #[error("Best chain selection strategy (SelectChain) is not provided.")] SelectChainRequired, diff --git a/client/storage-monitor/Cargo.toml b/client/storage-monitor/Cargo.toml index fcaf2eb77b745..5cb3f61f0beb9 100644 --- a/client/storage-monitor/Cargo.toml +++ b/client/storage-monitor/Cargo.toml @@ -18,3 +18,4 @@ sc-client-db = { version = "0.10.0-dev", default-features = false, path = "../db sc-utils = { version = "4.0.0-dev", path = "../utils" } sp-core = { version = "7.0.0", path = "../../primitives/core" } tokio = "1.22.0" +thiserror = "1.0.30" diff --git a/client/storage-monitor/src/lib.rs b/client/storage-monitor/src/lib.rs index 7eb973d2480f0..62ab3454a1fec 100644 --- a/client/storage-monitor/src/lib.rs +++ b/client/storage-monitor/src/lib.rs @@ -27,7 +27,17 @@ use std::{ const LOG_TARGET: &str = "storage-monitor"; -type Error = String; +#[allow(missing_docs)] +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("IO Error")] + IOError(#[from] Errno), + #[error("Out of storage space: available {0}MB, required {1}MB")] + StorageOutOfSpace(u64, u64), +} + +#[allow(missing_docs)] +pub type Result = std::result::Result; /// Parameters used to create the storage monitor. #[derive(Default, Debug, Clone, Args)] @@ -59,7 +69,7 @@ impl StorageMonitorService { parameters: StorageMonitorParams, database: DatabaseSource, spawner: &impl SpawnEssentialNamed, - ) -> Result<(), Error> { + ) -> Result<()> { Ok(match (parameters.threshold, database.path()) { (0, _) => { log::info!( @@ -109,31 +119,37 @@ impl StorageMonitorService { } /// Returns free space in MB, or error if statvfs failed. - fn free_space(path: &Path) -> Result { - statvfs(path).map(|stats| stats.blocks_available() * stats.block_size() / 1_000_000) + fn free_space(path: &Path) -> Result { + statvfs(path) + .map(|stats| stats.blocks_available() * stats.block_size() / 1_000_000) + .map_err(Error::from) } /// Checks if the amount of free space for given `path` is above given `threshold`. /// If it dropped below, error is returned. /// System errors are silently ignored. - fn check_free_space(path: &Path, threshold: u64) -> Result<(), Error> { + fn check_free_space(path: &Path, threshold: u64) -> Result<()> { match StorageMonitorService::free_space(path) { Ok(available_space) => { log::trace!( target: LOG_TARGET, - "free: {available_space} , threshold: {threshold}", + "free: {available_space} , threshold: {threshold}.", ); if available_space < threshold { log::error!(target: LOG_TARGET, "Available space {available_space}MB for path `{}` dropped below threshold: {threshold}MB , terminating...", path.display()); - Err(msg) + println!( + "----> {:?}", + Error::StorageOutOfSpace(available_space, threshold).to_string() + ); + Err(Error::StorageOutOfSpace(available_space, threshold)) } else { Ok(()) } }, Err(e) => { - log::warn!(target: LOG_TARGET, "could not read available space: {:?}", e); - Ok(()) + log::error!(target: LOG_TARGET, "Could not read available space: {:?}.", e); + Err(e) }, } } From de35ddad9e1d9fe2c7cefc977d47c71b286cc8dc Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 23 Jan 2023 20:46:58 +0100 Subject: [PATCH 22/23] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- client/storage-monitor/src/lib.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/client/storage-monitor/src/lib.rs b/client/storage-monitor/src/lib.rs index 62ab3454a1fec..39bd15675b350 100644 --- a/client/storage-monitor/src/lib.rs +++ b/client/storage-monitor/src/lib.rs @@ -27,7 +27,7 @@ use std::{ const LOG_TARGET: &str = "storage-monitor"; -#[allow(missing_docs)] +/// Error type used in this crate. #[derive(Debug, thiserror::Error)] pub enum Error { #[error("IO Error")] @@ -36,9 +36,6 @@ pub enum Error { StorageOutOfSpace(u64, u64), } -#[allow(missing_docs)] -pub type Result = std::result::Result; - /// Parameters used to create the storage monitor. #[derive(Default, Debug, Clone, Args)] pub struct StorageMonitorParams { @@ -69,7 +66,7 @@ impl StorageMonitorService { parameters: StorageMonitorParams, database: DatabaseSource, spawner: &impl SpawnEssentialNamed, - ) -> Result<()> { + ) -> Result<(), Error> { Ok(match (parameters.threshold, database.path()) { (0, _) => { log::info!( @@ -119,7 +116,7 @@ impl StorageMonitorService { } /// Returns free space in MB, or error if statvfs failed. - fn free_space(path: &Path) -> Result { + fn free_space(path: &Path) -> Result { statvfs(path) .map(|stats| stats.blocks_available() * stats.block_size() / 1_000_000) .map_err(Error::from) @@ -128,7 +125,7 @@ impl StorageMonitorService { /// Checks if the amount of free space for given `path` is above given `threshold`. /// If it dropped below, error is returned. /// System errors are silently ignored. - fn check_free_space(path: &Path, threshold: u64) -> Result<()> { + fn check_free_space(path: &Path, threshold: u64) -> Result<(), Error> { match StorageMonitorService::free_space(path) { Ok(available_space) => { log::trace!( @@ -138,10 +135,6 @@ impl StorageMonitorService { if available_space < threshold { log::error!(target: LOG_TARGET, "Available space {available_space}MB for path `{}` dropped below threshold: {threshold}MB , terminating...", path.display()); - println!( - "----> {:?}", - Error::StorageOutOfSpace(available_space, threshold).to_string() - ); Err(Error::StorageOutOfSpace(available_space, threshold)) } else { Ok(()) From 4e32f3de133d408701d1aa33c18e35d40896afe7 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Tue, 24 Jan 2023 11:55:59 +0100 Subject: [PATCH 23/23] publish=false removed --- client/storage-monitor/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/client/storage-monitor/Cargo.toml b/client/storage-monitor/Cargo.toml index 5cb3f61f0beb9..2ba24f9e2e718 100644 --- a/client/storage-monitor/Cargo.toml +++ b/client/storage-monitor/Cargo.toml @@ -7,7 +7,6 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" repository = "https://github.com/paritytech/substrate" description = "Storage monitor service for substrate" homepage = "https://substrate.io" -publish = false [dependencies] clap = { version = "4.0.9", features = ["derive", "string"] }