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 75ADA1FF17A for ; Tue, 11 Nov 2025 12:45:22 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4C9617B88; Tue, 11 Nov 2025 12:46:08 +0100 (CET) Date: Tue, 11 Nov 2025 12:46:01 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20250917134131.444585-1-c.ebner@proxmox.com> In-Reply-To: <20250917134131.444585-1-c.ebner@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.17.0 (https://github.com/astroidmail/astroid) Message-Id: <1762861538.w8u0ks6jw1.astroid@yuna.none> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1762861540963 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.046 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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] superseded: [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" a variant of this will be included in the next s3-race-fix patch series version ;) On September 17, 2025 3:41 pm, Christian Ebner wrote: > 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 > > > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel