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 14/21] GC: touch bad chunk files independent of backend type
Date: Fri, 14 Nov 2025 14:18:54 +0100	[thread overview]
Message-ID: <20251114131901.441650-15-c.ebner@proxmox.com> (raw)
In-Reply-To: <20251114131901.441650-1-c.ebner@proxmox.com>

The bad chunks marker files should be kept around also for s3 backed
datastores so GC does not clean them up unless a good chunk was
re-uploaded and bad chunk renaming detects them and does not lead
to conflicting renames.

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

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 083a83f7d..d71106ea8 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1322,41 +1322,37 @@ impl DataStore {
                 }
             }
 
-            match s3_client {
-                None => {
-                    // Filesystem backend
-                    if !self.inner.chunk_store.cond_touch_chunk(digest, false)? {
-                        let hex = hex::encode(digest);
-                        warn!(
-                            "warning: unable to access non-existent chunk {hex}, required by {file_name:?}"
-                        );
-
-                        // touch any corresponding .bad files to keep them around, meaning if a chunk is
-                        // rewritten correctly they will be removed automatically, as well as if no index
-                        // file requires the chunk anymore (won't get to this loop then)
-                        for i in 0..=9 {
-                            let bad_ext = format!("{i}.bad");
-                            let mut bad_path = PathBuf::new();
-                            bad_path.push(self.chunk_path(digest).0);
-                            bad_path.set_extension(bad_ext);
-                            self.inner.chunk_store.cond_touch_path(&bad_path, false)?;
-                        }
-                    }
+            if !self.inner.chunk_store.cond_touch_chunk(digest, false)? {
+                let hex = hex::encode(digest);
+                warn!(
+                    "warning: unable to access non-existent chunk {hex}, required by {file_name:?}"
+                );
+
+                // touch any corresponding .bad files to keep them around, meaning if a chunk is
+                // rewritten correctly they will be removed automatically, as well as if no index
+                // file requires the chunk anymore (won't get to this loop then)
+                for i in 0..=9 {
+                    let bad_ext = format!("{i}.bad");
+                    let mut bad_path = PathBuf::new();
+                    bad_path.push(self.chunk_path(digest).0);
+                    bad_path.set_extension(bad_ext);
+                    self.inner.chunk_store.cond_touch_path(&bad_path, false)?;
                 }
-                Some(ref _s3_client) => {
-                    // Update atime on local cache marker files.
-                    if !self.inner.chunk_store.cond_touch_chunk(digest, false)? {
-                        let (chunk_path, _digest) = self.chunk_path(digest);
-                        // 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))
-                            })?;
-                    }
+            }
+
+            if let Some(ref _s3_client) = s3_client {
+                // Update atime on local cache marker files.
+                if !self.inner.chunk_store.cond_touch_chunk(digest, false)? {
+                    let (chunk_path, _digest) = self.chunk_path(digest);
+                    // 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))
+                        })?;
                 }
             }
         }
-- 
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 ` Christian Ebner [this message]
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 ` [pbs-devel] [PATCH proxmox-backup v6 20/21] GC: fix: don't drop bad extension for S3 object to chunk path helper Christian Ebner
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-15-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