[pbs-devel] [PATCH proxmox-backup v5 18/19] GC: assure chunk exists on s3 store when creating missing chunk marker

[Christian Ebner]

Currently it is not assured the chunk is still present on the s3
object store before re-creating the chunk marker file. That will
however lead to the chunk not being re-inserted if re-uploaded.
Since checking the presence right away is expensive as it requires
additional api requests, mark the chunk as expected instead and
delay the existence check to phase 2 which must fetch the chunks
anyways.

Rely on the per-chunk file locks for consistency.

Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
---
 pbs-datastore/src/datastore.rs | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 549bc3b41..bf06d6fda 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1347,13 +1347,7 @@ impl DataStore {
                     if !self.inner.chunk_store.cond_touch_chunk(digest, false)? && !is_bad {
                         // Insert empty file as marker to tell GC phase2 that this is
                         // a chunk still in-use, so to keep in the S3 object store.
-                        std::fs::File::options()
-                            .write(true)
-                            .create_new(true)
-                            .open(&chunk_path)
-                            .with_context(|| {
-                                format!("failed to create marker for chunk {}", hex::encode(digest))
-                            })?;
+                        self.inner.chunk_store.mark_chunk_as_expected(digest)?;
                     }
                 } else {
                     let hex = hex::encode(digest);
@@ -1683,8 +1677,16 @@ impl DataStore {
                     let atime = match std::fs::metadata(&chunk_path) {
                         Ok(stat) => stat.accessed()?,
                         Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
-                            // File not found, delete by setting atime to unix epoch
-                            SystemTime::UNIX_EPOCH
+                            if self.inner.chunk_store.clear_chunk_expected_mark(&digest)? {
+                                unsafe {
+                                    // chunk store lock held
+                                    self.inner.chunk_store.replace_chunk_with_marker(&digest)?;
+                                }
+                                SystemTime::now()
+                            } else {
+                                // File not found, delete by setting atime to unix epoch
+                                SystemTime::UNIX_EPOCH
+                            }
                         }
                         Err(err) => return Err(err.into()),
                     };
@@ -1980,7 +1982,8 @@ impl DataStore {
         )
         .map_err(|err| format_err!("failed to upload chunk to s3 backend - {err:#}"))?;
         tracing::info!("Caching of chunk {}", hex::encode(digest));
-        self.cache_insert(&digest, &chunk)?;
+        self.cache_insert(digest, chunk)?;
+        self.inner.chunk_store.clear_chunk_expected_mark(digest)?;
         Ok((is_duplicate, chunk_size))
     }
 
-- 
2.47.3