[pbs-devel] [PATCH proxmox-backup] GC: use time pre phase1 to calculate min_atime in phase2

[pbs-devel] [PATCH proxmox-backup] GC: use time pre phase1 to calculate min_atime in phase2

[Aaron Lauterer]

Used chunks are marked in phase1 of the garbage collection process by
using the atime property. Each used chunk gets touched so that the atime
gets updated (if older than 24h, see relatime).

Should there ever be a situation in which the phase1 in the GC run needs
a very long time to finish, it could happen that the grace period
calculated in phase2 is not long enough and thus the marking of the
chunks (atime) becomes invalid. This would result in the removal of
needed chunks.

Even though the likelyhood of this happening is very low, using the
timestamp from right before phase1 is started, to calculate the grace
period in phase2 should avoid this situation.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 src/backup/chunk_store.rs | 5 ++---
 src/backup/datastore.rs   | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/backup/chunk_store.rs b/src/backup/chunk_store.rs
index d57befdd..8c35ba0d 100644
--- a/src/backup/chunk_store.rs
+++ b/src/backup/chunk_store.rs
@@ -291,14 +291,13 @@ impl ChunkStore {
     pub fn sweep_unused_chunks(
         &self,
         oldest_writer: i64,
+        phase1_start_time: i64,
         status: &mut GarbageCollectionStatus,
         worker: &WorkerTask,
     ) -> Result<(), Error> {
         use nix::sys::stat::fstatat;
 
-        let now = unsafe { libc::time(std::ptr::null_mut()) };
-
-        let mut min_atime = now - 3600*24; // at least 24h (see mount option relatime)
+        let mut min_atime = phase1_start_time - 3600*24; // at least 24h (see mount option relatime)
 
         if oldest_writer < min_atime {
             min_atime = oldest_writer;
diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
index 5d9cb335..0966d7e9 100644
--- a/src/backup/datastore.rs
+++ b/src/backup/datastore.rs
@@ -426,7 +426,7 @@ impl DataStore {
             self.mark_used_chunks(&mut gc_status, &worker)?;
 
             worker.log("Start GC phase2 (sweep unused chunks)");
-            self.chunk_store.sweep_unused_chunks(oldest_writer, &mut gc_status, &worker)?;
+            self.chunk_store.sweep_unused_chunks(oldest_writer, now, &mut gc_status, &worker)?;
 
             worker.log(&format!("Removed bytes: {}", gc_status.removed_bytes));
             worker.log(&format!("Removed chunks: {}", gc_status.removed_chunks));
-- 
2.20.1

[pbs-devel] applied: [PATCH proxmox-backup] GC: use time pre phase1 to calculate min_atime in phase2

[Dietmar Maurer]

good catch - thanks!