[pbs-devel] [PATCH proxmox-backup v2 02/12] 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 | 90 +++++++++++++++++++++-----------
 pbs-datastore/src/datastore.rs   | 41 ++++++---------
 2 files changed, 76 insertions(+), 55 deletions(-)

diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
index 3c59612bb..dc247886a 100644
--- a/pbs-datastore/src/chunk_store.rs
+++ b/pbs-datastore/src/chunk_store.rs
@@ -408,37 +408,28 @@ impl ChunkStore {
 
                 chunk_count += 1;
 
-                if stat.st_atime < min_atime {
-                    //let age = now - stat.st_atime;
-                    //println!("UNLINK {}  {:?}", age/(3600*24), filename);
-                    if let Err(err) = unlinkat(Some(dirfd), filename, UnlinkatFlags::NoRemoveDir) {
-                        if bad {
-                            status.still_bad += 1;
+                Self::check_atime_and_update_gc_status(
+                    stat.st_atime,
+                    min_atime,
+                    oldest_writer,
+                    stat.st_size as u64,
+                    bad,
+                    status,
+                    |status| {
+                        if let Err(err) =
+                            unlinkat(Some(dirfd), filename, UnlinkatFlags::NoRemoveDir)
+                        {
+                            if bad {
+                                status.still_bad += 1;
+                            }
+                            bail!(
+                                "unlinking chunk {filename:?} failed on store '{}' - {err}",
+                                self.name,
+                            );
                         }
-                        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;
-                }
+                        Ok(())
+                    },
+                )?;
             }
             drop(lock);
         }
@@ -446,6 +437,45 @@ impl ChunkStore {
         Ok(())
     }
 
+    /// Check within what range the provided chunks atime falls and update the garbage collection
+    /// status accordingly.
+    ///
+    /// If the chunk should be removed, the [`remove_callback`] is executed.
+    pub(super) fn check_atime_and_update_gc_status<
+        T: FnOnce(&mut GarbageCollectionStatus) -> Result<(), Error>,
+    >(
+        atime: i64,
+        min_atime: i64,
+        oldest_writer: i64,
+        size: u64,
+        bad: bool,
+        gc_status: &mut GarbageCollectionStatus,
+        remove_callback: T,
+    ) -> Result<(), Error> {
+        if atime < min_atime {
+            remove_callback(gc_status)?;
+            if bad {
+                gc_status.removed_bad += 1;
+            } else {
+                gc_status.removed_chunks += 1;
+            }
+            gc_status.removed_bytes += size;
+        } 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;
+        }
+        Ok(())
+    }
+
     /// 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 c37df94b7..7ef16c31e 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1676,31 +1676,22 @@ impl DataStore {
                         .extension()
                         .is_some_and(|ext| ext == "bad");
 
-                    if atime < min_atime {
-                        if let Some(cache) = self.cache() {
-                            // ignore errors, phase 3 will retry cleanup anyways
-                            let _ = cache.remove(&digest);
-                        }
-                        delete_list.push(content.key);
-                        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;
-                    }
+                    ChunkStore::check_atime_and_update_gc_status(
+                        atime,
+                        min_atime,
+                        oldest_writer,
+                        content.size,
+                        bad,
+                        &mut gc_status,
+                        |_status| {
+                            if let Some(cache) = self.cache() {
+                                // ignore errors, phase 3 will retry cleanup anyways
+                                let _ = cache.remove(&digest);
+                            }
+                            delete_list.push(content.key);
+                            Ok(())
+                        },
+                    )?;
 
                     chunk_count += 1;
                 }
-- 
2.47.3