Merge pull request 'Some improvements to Garage internals' (#451) from internals-rework into main

Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/451

4 files changed, 55 insertions, 64 deletions

diff --git a/src/garage/admin.rs b/src/garage/admin.rs
index 1ca3698a..c669b5e6 100644
--- a/src/garage/admin.rs
+++ b/src/garage/admin.rs
@@ -5,6 +5,7 @@ use std::sync::Arc;
 use async_trait::async_trait;
 use serde::{Deserialize, Serialize};
 
+use garage_util::background::BackgroundRunner;
 use garage_util::crdt::*;
 use garage_util::data::*;
 use garage_util::error::Error as GarageError;
@@ -74,13 +75,18 @@ impl Rpc for AdminRpc {
 
 pub struct AdminRpcHandler {
 	garage: Arc<Garage>,
+	background: Arc<BackgroundRunner>,
 	endpoint: Arc<Endpoint<AdminRpc, Self>>,
 }
 
 impl AdminRpcHandler {
-	pub fn new(garage: Arc<Garage>) -> Arc<Self> {
+	pub fn new(garage: Arc<Garage>, background: Arc<BackgroundRunner>) -> Arc<Self> {
 		let endpoint = garage.system.netapp.endpoint(ADMIN_RPC_PATH.into());
-		let admin = Arc::new(Self { garage, endpoint });
+		let admin = Arc::new(Self {
+			garage,
+			background,
+			endpoint,
+		});
 		admin.endpoint.set_handler(admin.clone());
 		admin
 	}
@@ -759,7 +765,7 @@ impl AdminRpcHandler {
 				)))
 			}
 		} else {
-			launch_online_repair(self.garage.clone(), opt).await;
+			launch_online_repair(&self.garage, &self.background, opt).await?;
 			Ok(AdminRpc::Ok(format!(
 				"Repair launched on {:?}",
 				self.garage.system.id
@@ -925,12 +931,11 @@ impl AdminRpcHandler {
 	async fn handle_worker_cmd(&self, cmd: &WorkerOperation) -> Result<AdminRpc, Error> {
 		match cmd {
 			WorkerOperation::List { opt } => {
-				let workers = self.garage.background.get_worker_info();
+				let workers = self.background.get_worker_info();
 				Ok(AdminRpc::WorkerList(workers, *opt))
 			}
 			WorkerOperation::Info { tid } => {
 				let info = self
-					.garage
 					.background
 					.get_worker_info()
 					.get(tid)
@@ -944,7 +949,7 @@ impl AdminRpcHandler {
 					self.garage
 						.block_manager
 						.send_scrub_command(scrub_command)
-						.await;
+						.await?;
 					Ok(AdminRpc::Ok("Scrub tranquility updated".into()))
 				}
 				WorkerSetCmd::ResyncWorkerCount { worker_count } => {
diff --git a/src/garage/repair/offline.rs b/src/garage/repair/offline.rs
index 7760a8bd..25193e4a 100644
--- a/src/garage/repair/offline.rs
+++ b/src/garage/repair/offline.rs
@@ -1,8 +1,5 @@
 use std::path::PathBuf;
 
-use tokio::sync::watch;
-
-use garage_util::background::*;
 use garage_util::config::*;
 use garage_util::error::*;
 
@@ -20,12 +17,8 @@ pub async fn offline_repair(config_file: PathBuf, opt: OfflineRepairOpt) -> Resu
 	info!("Loading configuration...");
 	let config = read_config(config_file)?;
 
-	info!("Initializing background runner...");
-	let (done_tx, done_rx) = watch::channel(false);
-	let (background, await_background_done) = BackgroundRunner::new(16, done_rx);
-
 	info!("Initializing Garage main data store...");
-	let garage = Garage::new(config.clone(), background)?;
+	let garage = Garage::new(config)?;
 
 	info!("Launching repair operation...");
 	match opt.what {
@@ -43,13 +36,7 @@ pub async fn offline_repair(config_file: PathBuf, opt: OfflineRepairOpt) -> Resu
 		}
 	}
 
-	info!("Repair operation finished, shutting down Garage internals...");
-	done_tx.send(true).unwrap();
-	drop(garage);
-
-	await_background_done.await?;
-
-	info!("Cleaning up...");
+	info!("Repair operation finished, shutting down...");
 
 	Ok(())
 }
diff --git a/src/garage/repair/online.rs b/src/garage/repair/online.rs
index 42221c2a..7120972c 100644
--- a/src/garage/repair/online.rs
+++ b/src/garage/repair/online.rs
@@ -15,35 +15,33 @@ use garage_util::error::Error;
 
 use crate::*;
 
-pub async fn launch_online_repair(garage: Arc<Garage>, opt: RepairOpt) {
+pub async fn launch_online_repair(
+	garage: &Arc<Garage>,
+	bg: &BackgroundRunner,
+	opt: RepairOpt,
+) -> Result<(), Error> {
 	match opt.what {
 		RepairWhat::Tables => {
 			info!("Launching a full sync of tables");
-			garage.bucket_table.syncer.add_full_sync();
-			garage.object_table.syncer.add_full_sync();
-			garage.version_table.syncer.add_full_sync();
-			garage.block_ref_table.syncer.add_full_sync();
-			garage.key_table.syncer.add_full_sync();
+			garage.bucket_table.syncer.add_full_sync()?;
+			garage.object_table.syncer.add_full_sync()?;
+			garage.version_table.syncer.add_full_sync()?;
+			garage.block_ref_table.syncer.add_full_sync()?;
+			garage.key_table.syncer.add_full_sync()?;
 		}
 		RepairWhat::Versions => {
 			info!("Repairing the versions table");
-			garage
-				.background
-				.spawn_worker(RepairVersionsWorker::new(garage.clone()));
+			bg.spawn_worker(RepairVersionsWorker::new(garage.clone()));
 		}
 		RepairWhat::BlockRefs => {
 			info!("Repairing the block refs table");
-			garage
-				.background
-				.spawn_worker(RepairBlockrefsWorker::new(garage.clone()));
+			bg.spawn_worker(RepairBlockrefsWorker::new(garage.clone()));
 		}
 		RepairWhat::Blocks => {
 			info!("Repairing the stored blocks");
-			garage
-				.background
-				.spawn_worker(garage_block::repair::RepairWorker::new(
-					garage.block_manager.clone(),
-				));
+			bg.spawn_worker(garage_block::repair::RepairWorker::new(
+				garage.block_manager.clone(),
+			));
 		}
 		RepairWhat::Scrub { cmd } => {
 			let cmd = match cmd {
@@ -56,9 +54,10 @@ pub async fn launch_online_repair(garage: Arc<Garage>, opt: RepairOpt) {
 				}
 			};
 			info!("Sending command to scrub worker: {:?}", cmd);
-			garage.block_manager.send_scrub_command(cmd).await;
+			garage.block_manager.send_scrub_command(cmd).await?;
 		}
 	}
+	Ok(())
 }
 
 // ----
@@ -93,19 +92,14 @@ impl Worker for RepairVersionsWorker {
 	}
 
 	async fn work(&mut self, _must_exit: &mut watch::Receiver<bool>) -> Result<WorkerState, Error> {
-		let item_bytes = match self.garage.version_table.data.store.get_gt(&self.pos)? {
-			Some((k, v)) => {
-				self.pos = k;
-				v
-			}
+		let (item_bytes, next_pos) = match self.garage.version_table.data.store.get_gt(&self.pos)? {
+			Some((k, v)) => (v, k),
 			None => {
 				info!("repair_versions: finished, done {}", self.counter);
 				return Ok(WorkerState::Done);
 			}
 		};
 
-		self.counter += 1;
-
 		let version = rmp_serde::decode::from_read_ref::<_, Version>(&item_bytes)?;
 		if !version.deleted.get() {
 			let object = self
@@ -134,10 +128,13 @@ impl Worker for RepairVersionsWorker {
 			}
 		}
 
+		self.counter += 1;
+		self.pos = next_pos;
+
 		Ok(WorkerState::Busy)
 	}
 
-	async fn wait_for_work(&mut self, _must_exit: &watch::Receiver<bool>) -> WorkerState {
+	async fn wait_for_work(&mut self) -> WorkerState {
 		unreachable!()
 	}
 }
@@ -174,18 +171,14 @@ impl Worker for RepairBlockrefsWorker {
 	}
 
 	async fn work(&mut self, _must_exit: &mut watch::Receiver<bool>) -> Result<WorkerState, Error> {
-		let item_bytes = match self.garage.block_ref_table.data.store.get_gt(&self.pos)? {
-			Some((k, v)) => {
-				self.pos = k;
-				v
-			}
-			None => {
-				info!("repair_block_ref: finished, done {}", self.counter);
-				return Ok(WorkerState::Done);
-			}
-		};
-
-		self.counter += 1;
+		let (item_bytes, next_pos) =
+			match self.garage.block_ref_table.data.store.get_gt(&self.pos)? {
+				Some((k, v)) => (v, k),
+				None => {
+					info!("repair_block_ref: finished, done {}", self.counter);
+					return Ok(WorkerState::Done);
+				}
+			};
 
 		let block_ref = rmp_serde::decode::from_read_ref::<_, BlockRef>(&item_bytes)?;
 		if !block_ref.deleted.get() {
@@ -212,10 +205,13 @@ impl Worker for RepairBlockrefsWorker {
 			}
 		}
 
+		self.counter += 1;
+		self.pos = next_pos;
+
 		Ok(WorkerState::Busy)
 	}
 
-	async fn wait_for_work(&mut self, _must_exit: &watch::Receiver<bool>) -> WorkerState {
+	async fn wait_for_work(&mut self) -> WorkerState {
 		unreachable!()
 	}
 }
diff --git a/src/garage/server.rs b/src/garage/server.rs
index d4099a97..16f1b625 100644
--- a/src/garage/server.rs
+++ b/src/garage/server.rs
@@ -35,12 +35,15 @@ pub async fn run_server(config_file: PathBuf) -> Result<(), Error> {
 	#[cfg(feature = "metrics")]
 	let metrics_exporter = opentelemetry_prometheus::exporter().init();
 
+	info!("Initializing Garage main data store...");
+	let garage = Garage::new(config.clone())?;
+
 	info!("Initializing background runner...");
 	let watch_cancel = watch_shutdown_signal();
-	let (background, await_background_done) = BackgroundRunner::new(16, watch_cancel.clone());
+	let (background, await_background_done) = BackgroundRunner::new(watch_cancel.clone());
 
-	info!("Initializing Garage main data store...");
-	let garage = Garage::new(config.clone(), background)?;
+	info!("Spawning Garage workers...");
+	garage.spawn_workers(&background);
 
 	if config.admin.trace_sink.is_some() {
 		info!("Initialize tracing...");
@@ -63,7 +66,7 @@ pub async fn run_server(config_file: PathBuf) -> Result<(), Error> {
 	let run_system = tokio::spawn(garage.system.clone().run(watch_cancel.clone()));
 
 	info!("Create admin RPC handler...");
-	AdminRpcHandler::new(garage.clone());
+	AdminRpcHandler::new(garage.clone(), background.clone());
 
 	// ---- Launch public-facing API servers ----