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

@@ -16587,7 +16587,6 @@ dependencies = [
  "clap 4.4.14",
  "fs4",
  "log",
- "sc-client-db",
  "sp-core",
  "thiserror",
  "tokio",

polkadot/cli/src/command.rs

@@ -256,11 +256,13 @@ where
 		)
 		.map(|full| full.task_manager)?;
 
-		sc_storage_monitor::StorageMonitorService::try_spawn(
+		if let Some(path) = database_source.path() {
-			cli.storage_monitor,
+			sc_storage_monitor::StorageMonitorService::try_spawn(
-			database_source,
+				cli.storage_monitor,
-			&task_manager.spawn_essential_handle(),
+				path.to_path_buf(),
-		)?;
+				&task_manager.spawn_essential_handle(),
+			)?;
+		}
 
 		Ok(task_manager)
 	})

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

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(
+	if let Some(database_path) = database_path {
-		cli.storage_monitor,
+		sc_storage_monitor::StorageMonitorService::try_spawn(
-		database_source,
+			cli.storage_monitor,
-		&task_manager.spawn_essential_handle(),
+			database_path,
-	)
+			&task_manager.spawn_essential_handle(),
-	.map_err(|e| ServiceError::Application(e.into()))?;
+		)
+		.map_err(|e| ServiceError::Application(e.into()))?;
+	}
 
 	Ok(task_manager)
 }

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"

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()) {
+		if parameters.threshold == 0 {
-			(0, _) => {
+			log::info!(
-				log::info!(
+				target: LOG_TARGET,
-					target: LOG_TARGET,
+				"StorageMonitorService: threshold `0` given, storage monitoring disabled",
-					"StorageMonitorService: threshold `0` given, storage monitoring disabled",
+			);
-				);
+		} else {
-			},
+			log::debug!(
-			(_, None) => {
+				target: LOG_TARGET,
-				log::warn!(
+				"Initializing StorageMonitorService for db path: {}",
-					target: LOG_TARGET,
+				path.display()
-					"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)?;
+			Self::check_free_space(&path, parameters.threshold)?;
 
-				let storage_monitor_service = StorageMonitorService {
+			let storage_monitor_service = StorageMonitorService {
-					path: path.to_path_buf(),
+				path,
-					threshold,
+				threshold: parameters.threshold,
-					polling_period: Duration::from_secs(parameters.polling_period.into()),
+				polling_period: Duration::from_secs(parameters.polling_period.into()),
-				};
+			};
 
-				spawner.spawn_essential(
+			spawner.spawn_essential(
-					"storage-monitor",
+				"storage-monitor",
-					None,
+				None,
-					Box::pin(storage_monitor_service.run()),
+				Box::pin(storage_monitor_service.run()),
-				);
+			);
-			},
+		}
-		})
+
+		Ok(())
 	}
 
 	/// Main monitoring loop, intended to be spawned as essential task. Quits if free space drop