Skip to content

Commit

Permalink
Improve storage monitor API (paritytech#2899)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nazar-pc authored Jan 10, 2024
1 parent 9dce234 commit 047f6fe
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 44 deletions.
18 changes: 10 additions & 8 deletions substrate/bin/node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down Expand Up @@ -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)
}
Expand Down
3 changes: 1 addition & 2 deletions substrate/client/storage-monitor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
61 changes: 27 additions & 34 deletions substrate/client/storage-monitor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 047f6fe

Please sign in to comment.