[pbs-devel] [PATCH proxmox-backup 2/7] gc: chunk store: rework atime check and gc status into common helper

[Christian Ebner]

Use the shared code paths for both, filesystem and s3 backend to a
common helper to avoid code duplication and adapt callsites
accordingly.

Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
---
 pbs-datastore/src/chunk_store.rs | 69 ++++++++++++++++++++++----------
 pbs-datastore/src/datastore.rs   | 29 +++++---------
 2 files changed, 57 insertions(+), 41 deletions(-)

diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
index 3c59612bb..0725ca3a7 100644
--- a/pbs-datastore/src/chunk_store.rs
+++ b/pbs-datastore/src/chunk_store.rs
@@ -408,36 +408,27 @@ impl ChunkStore {
 
                 chunk_count += 1;
 
-                if stat.st_atime < min_atime {
-                    //let age = now - stat.st_atime;
-                    //println!("UNLINK {}  {:?}", age/(3600*24), filename);
+                if Self::check_atime_and_update_gc_status(
+                    stat.st_atime,
+                    min_atime,
+                    oldest_writer,
+                    stat.st_size as u64,
+                    bad,
+                    status,
+                ) {
                     if let Err(err) = unlinkat(Some(dirfd), filename, UnlinkatFlags::NoRemoveDir) {
                         if bad {
+                            status.removed_bad -= 1;
                             status.still_bad += 1;
+                        } else {
+                            status.removed_chunks += 1;
                         }
+                        status.removed_bytes -= stat.st_size as u64;
                         bail!(
                             "unlinking chunk {filename:?} failed on store '{}' - {err}",
                             self.name,
                         );
                     }
-                    if bad {
-                        status.removed_bad += 1;
-                    } else {
-                        status.removed_chunks += 1;
-                    }
-                    status.removed_bytes += stat.st_size as u64;
-                } else if stat.st_atime < oldest_writer {
-                    if bad {
-                        status.still_bad += 1;
-                    } else {
-                        status.pending_chunks += 1;
-                    }
-                    status.pending_bytes += stat.st_size as u64;
-                } else {
-                    if !bad {
-                        status.disk_chunks += 1;
-                    }
-                    status.disk_bytes += stat.st_size as u64;
                 }
             }
             drop(lock);
@@ -446,6 +437,42 @@ impl ChunkStore {
         Ok(())
     }
 
+    /// Check within what range the provided chunks atime falls and update the garbage collection
+    /// status accordingly.
+    ///
+    /// Returns true if the chunk file should be removed.
+    pub(super) fn check_atime_and_update_gc_status(
+        atime: i64,
+        min_atime: i64,
+        oldest_writer: i64,
+        size: u64,
+        bad: bool,
+        gc_status: &mut GarbageCollectionStatus,
+    ) -> bool {
+        if atime < min_atime {
+            if bad {
+                gc_status.removed_bad += 1;
+            } else {
+                gc_status.removed_chunks += 1;
+            }
+            gc_status.removed_bytes += size;
+            return true;
+        } else if atime < oldest_writer {
+            if bad {
+                gc_status.still_bad += 1;
+            } else {
+                gc_status.pending_chunks += 1;
+            }
+            gc_status.pending_bytes += size;
+        } else {
+            if !bad {
+                gc_status.disk_chunks += 1;
+            }
+            gc_status.disk_bytes += size;
+        }
+        false
+    }
+
     /// Check if atime updates are honored by the filesystem backing the chunk store.
     ///
     /// Checks if the atime is always updated by utimensat taking into consideration the Linux
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index c2a82b8b8..e36af68fc 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1676,30 +1676,19 @@ impl DataStore {
                         .extension()
                         .is_some_and(|ext| ext == "bad");
 
-                    if atime < min_atime {
+                    if ChunkStore::check_atime_and_update_gc_status(
+                        atime,
+                        min_atime,
+                        oldest_writer,
+                        content.size,
+                        bad,
+                        &mut gc_status,
+                    ) {
                         if let Some(cache) = self.cache() {
                             // ignore errors, phase 3 will retry cleanup anyways
                             let _ = cache.remove(&digest);
                         }
-                        delete_list.push(content.key.clone());
-                        if bad {
-                            gc_status.removed_bad += 1;
-                        } else {
-                            gc_status.removed_chunks += 1;
-                        }
-                        gc_status.removed_bytes += content.size;
-                    } else if atime < oldest_writer {
-                        if bad {
-                            gc_status.still_bad += 1;
-                        } else {
-                            gc_status.pending_chunks += 1;
-                        }
-                        gc_status.pending_bytes += content.size;
-                    } else {
-                        if !bad {
-                            gc_status.disk_chunks += 1;
-                        }
-                        gc_status.disk_bytes += content.size;
+                        delete_list.push(content.key);
                     }
 
                     chunk_count += 1;
-- 
2.47.3