From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 66D3A1FF17C for ; Wed, 17 Sep 2025 15:41:59 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1A782559F; Wed, 17 Sep 2025 15:42:14 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Date: Wed, 17 Sep 2025 15:41:31 +0200 Message-ID: <20250917134131.444585-1-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1758116493307 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.043 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox-backup] gc: only create locally missing chunk markers if present on S3 store X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" This fixes a logic issue with the local cache marker files for all indexed chunks being created during phase 1 of the garbage collection, independent on whether the chunks might have been moved to be bad by a verification job. Therefore, check if the chunk exists in the S3 object store before creating the marker, if no bad chunk marker is present in the local store cache (this can also happen after reusing a fresh local store cache after a new datastore setup). Make sure to also keep the bad chunk marker files in the local store cache. This is achieved by marking these unconditionally if the chunk marker file is not found. Signed-off-by: Christian Ebner --- Encountered while investigating an unrelated issue. pbs-datastore/src/datastore.rs | 73 +++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 32 deletions(-) diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs index 7cf020fc0..4562b0fca 100644 --- a/pbs-datastore/src/datastore.rs +++ b/pbs-datastore/src/datastore.rs @@ -1308,40 +1308,49 @@ 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!("{}.bad", i); - 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); + let mut is_bad_chunk = false; + + // 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!("{}.bad", i); + let mut bad_path = PathBuf::new(); + bad_path.push(self.chunk_path(digest).0); + bad_path.set_extension(bad_ext); + if self.inner.chunk_store.cond_touch_path(&bad_path, false)? { + is_bad_chunk = true; } } - 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)) - })?; + + match s3_client { + None => warn!( + "warning: unable to access non-existent chunk {hex}, required by {file_name:?}" + ), + Some(ref s3_client) => { + if !is_bad_chunk { + // Might be a new empty cache with no markers present yet, so fallback to + // check the s3 object store in that case. + let object_key = crate::s3::object_key_from_digest(digest)?; + let head_object_response = + proxmox_async::runtime::block_on(s3_client.head_object(object_key)) + .context("failed to check chunk presence in s3 object store")?; + if head_object_response.is_some() { + info!("Create missing marker for existing chunk {hex}"); + 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}") + })?; + } + } } } } -- 2.47.3 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel