public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup v6 20/21] GC: fix: don't drop bad extension for S3 object to chunk path helper
Date: Fri, 14 Nov 2025 14:19:00 +0100	[thread overview]
Message-ID: <20251114131901.441650-21-c.ebner@proxmox.com> (raw)
In-Reply-To: <20251114131901.441650-1-c.ebner@proxmox.com>

The current implementation does return a path and digest also for S3
object keys which are bad chunks, does however drop the extension.

Since this will cause issues for phase 2 of garbage collection on
S3 backends, include the extension and return a flag signaling if
this is a bad chunk or not.

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

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index c0dc0f75d..b91e90638 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1660,10 +1660,11 @@ impl DataStore {
             let mut delete_list = Vec::with_capacity(1000);
             loop {
                 for content in list_bucket_result.contents {
-                    let (chunk_path, digest) = match self.chunk_path_from_object_key(&content.key) {
-                        Some(path) => path,
-                        None => continue,
-                    };
+                    let (chunk_path, digest, bad) =
+                        match self.chunk_path_from_object_key(&content.key) {
+                            Some(path) => path,
+                            None => continue,
+                        };
 
                     let timeout = std::time::Duration::from_secs(0);
                     let _chunk_guard = match self.inner.chunk_store.lock_chunk(&digest, timeout) {
@@ -1692,11 +1693,6 @@ impl DataStore {
                     };
                     let atime = atime.duration_since(SystemTime::UNIX_EPOCH)?.as_secs() as i64;
 
-                    let bad = chunk_path
-                        .as_path()
-                        .extension()
-                        .is_some_and(|ext| ext == "bad");
-
                     unsafe {
                         self.inner.chunk_store.cond_sweep_chunk(
                             atime,
@@ -1852,14 +1848,23 @@ impl DataStore {
     }
 
     // Check and generate a chunk path from given object key
-    fn chunk_path_from_object_key(&self, object_key: &S3ObjectKey) -> Option<(PathBuf, [u8; 32])> {
+    fn chunk_path_from_object_key(
+        &self,
+        object_key: &S3ObjectKey,
+    ) -> Option<(PathBuf, [u8; 32], bool)> {
         // Check object is actually a chunk
+        let path = Path::new::<str>(object_key);
         // file_name() should always be Some, as objects will have a filename
-        let digest = Path::new::<str>(object_key).file_name()?;
+        let digest = path.file_name()?;
         let bytes = digest.as_bytes();
-        if bytes.len() != 64 && bytes.len() != 64 + ".0.bad".len() {
+        let bad_ext_len = ".0.bad".len();
+        let bad_chunk = if bytes.len() == 64 + bad_ext_len {
+            true
+        } else if bytes.len() == 64 {
+            false
+        } else {
             return None;
-        }
+        };
         if !bytes.iter().take(64).all(u8::is_ascii_hexdigit) {
             return None;
         }
@@ -1871,13 +1876,17 @@ impl DataStore {
         chunk_path.push(".chunks");
         chunk_path.push(hexdigit_prefix);
         chunk_path.push(digest);
+        if bad_chunk {
+            let extension = unsafe { digest_str.get_unchecked(64..64 + bad_ext_len) };
+            chunk_path.push(extension);
+        }
 
         let mut digest_bytes = [0u8; 32];
         let digest = digest.as_bytes();
         // safe to unwrap as already checked above
         hex::decode_to_slice(&digest[..64], &mut digest_bytes).unwrap();
 
-        Some((chunk_path, digest_bytes))
+        Some((chunk_path, digest_bytes, bad_chunk))
     }
 
     pub fn try_shared_chunk_store_lock(&self) -> Result<ProcessLockSharedGuard, Error> {
-- 
2.47.3



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


  parent reply	other threads:[~2025-11-14 13:19 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-14 13:18 [pbs-devel] [PATCH proxmox-backup v6 00/21] fix chunk upload/insert, rename corrupt chunks and GC race conditions for s3 backend Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 01/21] datastore: GC: drop overly verbose info message during s3 chunk sweep Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 02/21] chunk store: implement per-chunk file locking helper for s3 backend Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 03/21] datastore: acquire chunk store mutex lock when renaming corrupt chunk Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 04/21] datastore: get per-chunk file lock for chunk rename on s3 backend Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 05/21] fix #6961: datastore: verify: evict corrupt chunks from in-memory LRU cache Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 06/21] datastore: add locking to protect against races on chunk insert for s3 Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 07/21] GC: fix race with chunk upload/insert on s3 backends Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 08/21] chunk store: reduce exposure of clear_chunk() to crate only Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 09/21] chunk store: make chunk removal a helper method of the chunk store Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 10/21] store: split insert_chunk into wrapper + unsafe locked implementation Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 11/21] store: cache: move Mutex acquire to cache insertion Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 12/21] chunk store: rename cache-specific helpers Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 13/21] GC: cleanup chunk markers from cache in phase 3 on s3 backends Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 14/21] GC: touch bad chunk files independent of backend type Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 15/21] GC: guard missing marker file insertion for s3 backed stores Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 16/21] GC: s3: track if a chunk marker file is missing since a bad chunk Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 17/21] chunk store: add helpers marking missing local chunk markers as expected Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 18/21] GC: assure chunk exists on s3 store when creating missing chunk marker Christian Ebner
2025-11-14 13:18 ` [pbs-devel] [PATCH proxmox-backup v6 19/21] datastore: document s3 backend specific locking restrictions Christian Ebner
2025-11-14 13:19 ` Christian Ebner [this message]
2025-11-14 13:19 ` [pbs-devel] [PATCH proxmox-backup v6 21/21] GC: clean up bad chunks from the filesystem only Christian Ebner
2025-11-14 13:34 ` [pbs-devel] [PATCH proxmox-backup v6 00/21] fix chunk upload/insert, rename corrupt chunks and GC race conditions for s3 backend Fabian Grünbichler
2025-11-14 22:14 ` Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251114131901.441650-21-c.ebner@proxmox.com \
    --to=c.ebner@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal