[pbs-devel] [PATCH proxmox-backup 3/7] chunk store: add and use method to remove chunks

[Christian Ebner]

Reworks the removing of cached chunks during phase 2 of garbage
collection for datastores backed by s3.

Move the actual chunk removal logic to be a method of the chunk store
and require the mutex guard to be passe as shared reference,
signaling that the caller locked the store as required to avoid races
with chunk insert.

Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
---
 pbs-datastore/src/chunk_store.rs               | 15 ++++++++++++++-
 pbs-datastore/src/datastore.rs                 |  2 +-
 pbs-datastore/src/local_datastore_lru_cache.rs |  7 +++----
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
index 0725ca3a7..010785fbc 100644
--- a/pbs-datastore/src/chunk_store.rs
+++ b/pbs-datastore/src/chunk_store.rs
@@ -1,7 +1,7 @@
 use std::os::unix::fs::MetadataExt;
 use std::os::unix::io::AsRawFd;
 use std::path::{Path, PathBuf};
-use std::sync::{Arc, Mutex};
+use std::sync::{Arc, Mutex, MutexGuard};
 use std::time::Duration;
 
 use anyhow::{bail, format_err, Context, Error};
@@ -254,6 +254,19 @@ impl ChunkStore {
         Ok(true)
     }
 
+    /// Remove a chunk from the chunk store
+    ///
+    /// Used to remove chunks from the local datastore cache. Caller must signal to hold the chunk
+    /// store mutex lock.
+    pub fn remove_chunk(
+        &self,
+        digest: &[u8; 32],
+        _guard: &MutexGuard<'_, ()>,
+    ) -> Result<(), Error> {
+        let (path, _digest) = self.chunk_path(digest);
+        std::fs::remove_file(path).map_err(Error::from)
+    }
+
     pub fn get_chunk_iterator(
         &self,
     ) -> Result<
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index e36af68fc..4f55eb9db 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1686,7 +1686,7 @@ impl DataStore {
                     ) {
                         if let Some(cache) = self.cache() {
                             // ignore errors, phase 3 will retry cleanup anyways
-                            let _ = cache.remove(&digest);
+                            let _ = cache.remove(&digest, &lock);
                         }
                         delete_list.push(content.key);
                     }
diff --git a/pbs-datastore/src/local_datastore_lru_cache.rs b/pbs-datastore/src/local_datastore_lru_cache.rs
index c0edd3619..1d2e87cb9 100644
--- a/pbs-datastore/src/local_datastore_lru_cache.rs
+++ b/pbs-datastore/src/local_datastore_lru_cache.rs
@@ -2,7 +2,7 @@
 //! a network layer (e.g. via the S3 backend).
 
 use std::future::Future;
-use std::sync::Arc;
+use std::sync::{Arc, MutexGuard};
 
 use anyhow::{bail, Error};
 use http_body_util::BodyExt;
@@ -87,10 +87,9 @@ impl LocalDatastoreLruCache {
     /// Remove a chunk from the local datastore cache.
     ///
     /// Fails if the chunk cannot be deleted successfully.
-    pub fn remove(&self, digest: &[u8; 32]) -> Result<(), Error> {
+    pub fn remove(&self, digest: &[u8; 32], guard: &MutexGuard<'_, ()>) -> Result<(), Error> {
         self.cache.remove(*digest);
-        let (path, _digest_str) = self.store.chunk_path(digest);
-        std::fs::remove_file(path).map_err(Error::from)
+        self.store.remove_chunk(digest, guard)
     }
 
     /// Access the locally cached chunk or fetch it from the S3 object store via the provided
-- 
2.47.3