[ENG-794] Library deletion hangs (#1137)

Solving deadlock on library deletion
Also improving some minor stuff on thumbnails remover actor

core/src/api/libraries.rs

@@ -116,7 +116,9 @@ pub(crate) fn mount() -> AlphaRouter<Ctx> {
 		})
 		.procedure(
 			"delete",
-			R.mutation(|ctx, id: Uuid| async move { Ok(ctx.library_manager.delete(id).await?) }),
+			R.mutation(|ctx, id: Uuid| async move {
+				ctx.library_manager.delete(id).await.map_err(Into::into)
+			}),
 		)
 	// .yolo_merge("peer.guest.", peer_guest_router())
 	// .yolo_merge("peer.host.", peer_host_router())

core/src/library/manager.rs

@@ -315,15 +315,18 @@ impl LibraryManager {
 	}
 
 	pub async fn delete(&self, id: Uuid) -> Result<(), LibraryManagerError> {
-		let libraries = self.libraries.read().await;
+		let mut libraries_write_guard = self.libraries.write().await;
 
-		let library = libraries
+		// As we're holding a write lock here, we know that our index can't change before removal.
+		let library_idx = libraries_write_guard
 			.iter()
-			.find(|l| l.id == id)
+			.position(|l| l.id == id)
 			.ok_or(LibraryManagerError::LibraryNotFound)?;
 
-		let db_path = self.libraries_dir.join(format!("{}.db", library.id));
-		let sd_lib_path = self.libraries_dir.join(format!("{}.sdlibrary", library.id));
+		let library_id = libraries_write_guard[library_idx].id;
+
+		let db_path = self.libraries_dir.join(format!("{}.db", library_id));
+		let sd_lib_path = self.libraries_dir.join(format!("{}.sdlibrary", library_id));
 
 		try_join!(
 			async {
@@ -338,10 +341,14 @@ impl LibraryManager {
 			},
 		)?;
 
-		invalidate_query!(library, "library.list");
-
 		self.thumbnail_remover.remove_library(id).await;
-		self.libraries.write().await.retain(|l| l.id != id);
+
+		// We only remove here after files deletion
+		let library = libraries_write_guard.remove(library_idx);
+
+		info!("Removed Library <id='{library_id}'>");
+
+		invalidate_query!(library, "library.list");
 
 		Ok(())
 	}

core/src/object/thumbnail_remover.rs

@@ -19,13 +19,14 @@ use futures_concurrency::{future::TryJoin, stream::Merge};
 use thiserror::Error;
 use tokio::{
 	fs, io,
-	time::{interval, MissedTickBehavior},
+	time::{interval_at, Instant, MissedTickBehavior},
 };
 use tokio_stream::{wrappers::IntervalStream, StreamExt};
 use tokio_util::sync::{CancellationToken, DropGuard};
 use tracing::{debug, error, trace};
 use uuid::Uuid;
 
+const THIRTY_SECS: Duration = Duration::from_secs(30);
 const HALF_HOUR: Duration = Duration::from_secs(30 * 60);
 
 #[derive(Error, Debug)]
@@ -65,6 +66,7 @@ impl ThumbnailRemoverActorProxy {
 	}
 }
 
+#[derive(Debug)]
 enum DatabaseMessage {
 	Add(Uuid, Arc<PrismaClient>),
 	Remove(Uuid),
@@ -154,12 +156,13 @@ impl ThumbnailRemoverActor {
 		non_indexed_thumbnails_cas_ids_rx: chan::Receiver<String>,
 		cancel_token: CancellationToken,
 	) {
-		let mut check_interval = interval(HALF_HOUR);
+		let mut check_interval = interval_at(Instant::now() + THIRTY_SECS, HALF_HOUR);
 		check_interval.set_missed_tick_behavior(MissedTickBehavior::Skip);
 
 		let mut databases = HashMap::new();
 		let mut non_indexed_thumbnails_cas_ids = HashSet::new();
 
+		#[derive(Debug)]
 		enum StreamMessage {
 			Run,
 			ToDelete(Vec<String>),
@@ -196,8 +199,12 @@ impl ThumbnailRemoverActor {
 					}
 				}
 				StreamMessage::ToDelete(cas_ids) => {
-					if let Err(e) = Self::remove_by_cas_ids(&thumbnails_directory, cas_ids).await {
-						error!("Got an error when trying to remove thumbnails: {e:#?}");
+					if !cas_ids.is_empty() {
+						if let Err(e) =
+							Self::remove_by_cas_ids(&thumbnails_directory, cas_ids).await
+						{
+							error!("Got an error when trying to remove thumbnails: {e:#?}");
+						}
 					}
 				}
 
@@ -317,6 +324,10 @@ impl ThumbnailRemoverActor {
 			}
 
 			if thumbnails_paths_by_cas_id.is_empty() {
+				trace!(
+					"Removing empty thumbnails sharding directory: {}",
+					entry_path.display()
+				);
 				fs::remove_dir(&entry_path)
 					.await
 					.map_err(|e| FileIOError::from((entry_path, e)))?;
@@ -324,6 +335,8 @@ impl ThumbnailRemoverActor {
 				continue;
 			}
 
+			let thumbs_found = thumbnails_paths_by_cas_id.len();
+
 			let mut thumbs_in_db_futs = databases
 				.iter()
 				.map(|db| {
@@ -348,6 +361,8 @@ impl ThumbnailRemoverActor {
 			thumbnails_paths_by_cas_id
 				.retain(|cas_id, _| !non_indexed_thumbnails_cas_ids.contains(cas_id));
 
+			let thumbs_to_remove = thumbnails_paths_by_cas_id.len();
+
 			thumbnails_paths_by_cas_id
 				.into_values()
 				.map(|path| async move {
@@ -359,6 +374,18 @@ impl ThumbnailRemoverActor {
 				.collect::<Vec<_>>()
 				.try_join()
 				.await?;
+
+			if thumbs_to_remove == thumbs_found {
+				// if we removed all the thumnails we foumd, it means that the directory is empty
+				// and can be removed...
+				trace!(
+					"Removing empty thumbnails sharding directory: {}",
+					entry_path.display()
+				);
+				fs::remove_dir(&entry_path)
+					.await
+					.map_err(|e| FileIOError::from((entry_path, e)))?;
+			}
 		}
 
 		Ok(())