[pbs-devel] [PATCH proxmox-backup v10 40/46] datastore: conditionally upload atime marker chunk to s3 backend

[Christian Ebner <c.ebner@proxmox.com>]

Since commit b18eab64 ("fix #5982: garbage collection: check atime
updates are honored"), the 4 MiB fixed sized, unencypted and
compressed chunk containing all zeros is inserted at datastore
creation if the atime safety check is enabled.

If the datastore is backed by an S3 object store, chunk uploads are
avoided by checking the presence of the chunks in the local cache
store. Therefore, the all zero chunk will however not be uploaded
since already inserted locally.

Fix this by conditionally uploading the chunk before performing the
atime update check for datastores backed by S3.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 9:
- no changes

 pbs-datastore/src/chunk_store.rs | 25 ++++++++++++++++++++++---
 pbs-datastore/src/datastore.rs   | 20 ++++++++++----------
 src/api2/config/datastore.rs     |  5 ++++-
 3 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
index 95f00e8d5..3c59612bb 100644
--- a/pbs-datastore/src/chunk_store.rs
+++ b/pbs-datastore/src/chunk_store.rs
@@ -9,6 +9,7 @@ use tracing::{info, warn};
 
 use pbs_api_types::{DatastoreFSyncLevel, GarbageCollectionStatus};
 use proxmox_io::ReadExt;
+use proxmox_s3_client::S3Client;
 use proxmox_sys::fs::{create_dir, create_path, file_type_from_file_stat, CreateOptions};
 use proxmox_sys::process_locker::{
     ProcessLockExclusiveGuard, ProcessLockSharedGuard, ProcessLocker,
@@ -454,11 +455,29 @@ impl ChunkStore {
     /// Uses a 4 MiB fixed size, compressed but unencrypted chunk to test. The chunk is inserted in
     /// the chunk store if not yet present.
     /// Returns with error if the check could not be performed.
-    pub fn check_fs_atime_updates(&self, retry_on_file_changed: bool) -> Result<(), Error> {
+    pub fn check_fs_atime_updates(
+        &self,
+        retry_on_file_changed: bool,
+        s3_client: Option<Arc<S3Client>>,
+    ) -> Result<(), Error> {
         let (zero_chunk, digest) = DataChunkBuilder::build_zero_chunk(None, 4096 * 1024, true)?;
-        let (pre_existing, _) = self.insert_chunk(&zero_chunk, &digest)?;
         let (path, _digest) = self.chunk_path(&digest);
 
+        if let Some(ref s3_client) = s3_client {
+            if let Err(err) = std::fs::metadata(&path) {
+                if err.kind() == std::io::ErrorKind::NotFound {
+                    let object_key = crate::s3::object_key_from_digest(&digest)?;
+                    proxmox_async::runtime::block_on(s3_client.upload_no_replace_with_retry(
+                        object_key,
+                        zero_chunk.raw_data().to_vec().into(),
+                    ))
+                    .context("failed to upload chunk to s3 backend")?;
+                }
+            }
+        }
+
+        let (pre_existing, _) = self.insert_chunk(&zero_chunk, &digest)?;
+
         // Take into account timestamp update granularity in the kernel
         // Blocking the thread is fine here since this runs in a worker.
         std::thread::sleep(Duration::from_secs(1));
@@ -478,7 +497,7 @@ impl ChunkStore {
         // two metadata calls, try to check once again on changed file
         if metadata_before.ino() != metadata_now.ino() {
             if retry_on_file_changed {
-                return self.check_fs_atime_updates(false);
+                return self.check_fs_atime_updates(false, s3_client);
             }
             bail!("chunk {path:?} changed twice during access time safety check, cannot proceed.");
         }
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index e6dfa8ee7..4526bf2b8 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1543,10 +1543,19 @@ impl DataStore {
                     .parse_property_string(gc_store_config.tuning.as_deref().unwrap_or(""))?,
             )?;
 
+            let s3_client = match self.backend()? {
+                DatastoreBackend::Filesystem => None,
+                DatastoreBackend::S3(s3_client) => {
+                    proxmox_async::runtime::block_on(s3_client.head_bucket())
+                        .context("failed to reach bucket")?;
+                    Some(s3_client)
+                }
+            };
+
             if tuning.gc_atime_safety_check.unwrap_or(true) {
                 self.inner
                     .chunk_store
-                    .check_fs_atime_updates(true)
+                    .check_fs_atime_updates(true, s3_client.clone())
                     .context("atime safety check failed")?;
                 info!("Access time update check successful, proceeding with GC.");
             } else {
@@ -1585,15 +1594,6 @@ impl DataStore {
                 1024 * 1024
             };
 
-            let s3_client = match self.backend()? {
-                DatastoreBackend::Filesystem => None,
-                DatastoreBackend::S3(s3_client) => {
-                    proxmox_async::runtime::block_on(s3_client.head_bucket())
-                        .context("failed to reach bucket")?;
-                    Some(s3_client)
-                }
-            };
-
             info!("Start GC phase1 (mark used chunks)");
 
             self.mark_used_chunks(
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 7a4a39074..9b87e01f2 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -1,4 +1,5 @@
 use std::path::{Path, PathBuf};
+use std::sync::Arc;
 
 use ::serde::{Deserialize, Serialize};
 use anyhow::{bail, format_err, Context, Error};
@@ -118,6 +119,7 @@ pub(crate) fn do_create_datastore(
             .parse_property_string(datastore.tuning.as_deref().unwrap_or(""))?,
     )?;
 
+    let mut backend_s3_client = None;
     if let Some(ref backend_config) = datastore.backend {
         let backend_config: DatastoreBackendConfig = backend_config.parse()?;
         match backend_config.ty.unwrap_or_default() {
@@ -151,6 +153,7 @@ pub(crate) fn do_create_datastore(
                 // Fine to block since this runs in worker task
                 proxmox_async::runtime::block_on(s3_client.head_bucket())
                     .context("failed to access bucket")?;
+                backend_s3_client = Some(Arc::new(s3_client));
             }
         }
     }
@@ -194,7 +197,7 @@ pub(crate) fn do_create_datastore(
 
     if tuning.gc_atime_safety_check.unwrap_or(true) {
         chunk_store
-            .check_fs_atime_updates(true)
+            .check_fs_atime_updates(true, backend_s3_client)
             .context("access time safety check failed")?;
         info!("Access time update check successful.");
     } else {
-- 
2.47.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel