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 254B21FF17A for ; Tue, 25 Nov 2025 15:19:22 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 83F7F182FC; Tue, 25 Nov 2025 15:19:32 +0100 (CET) Date: Tue, 25 Nov 2025 15:18:55 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20251125140013.586369-1-c.ebner@proxmox.com> In-Reply-To: <20251125140013.586369-1-c.ebner@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.17.0 (https://github.com/astroidmail/astroid) Message-Id: <1764080186.d9oxoqi5dh.astroid@yuna.none> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1764080302715 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: Re: [pbs-devel] [PATCH proxmox-backup v2] GC: chunk store: fix chunk using markers cleanup 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" On November 25, 2025 3:00 pm, Christian Ebner wrote: > Since commit 9510ef1a ("GC: assure chunk exists on s3 store when > creating missing chunk marker") chunks which are referenced by > an index file but do not have a local marker file are marked by a > file with the `using` extension, so they are not cleaned up during > phase 2 if the chunk is still present on the backend. > > If the chunk is however not encountered, phase 3 will see the marker > and tries to clean it up, which currently however fails because > it is first tried to be cleaned up from the LRU cache, the filename > being converted to the chunk digest. > > Therefore, clean up any using marker file encountered during phase 3 > before any regular or bad chunk, independent from the atime. > > Fixes: https://forum.proxmox.com/threads/176567/post-819437 > Signed-off-by: Christian Ebner > --- > Changes since version 1 (thanks a lot for offlist discussion Thomas): > - Cleanup using marker chunks independent from atime cutoff > > pbs-datastore/src/chunk_store.rs | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs > index f53460664..7fe09b914 100644 > --- a/pbs-datastore/src/chunk_store.rs > +++ b/pbs-datastore/src/chunk_store.rs > @@ -25,6 +25,8 @@ use crate::file_formats::{ > }; > use crate::{DataBlob, LocalDatastoreLruCache}; > > +const USING_MARKER_FILENAME_EXT: &str = "using"; > + > /// File system based chunk store > pub struct ChunkStore { > name: String, // used for error reporting > @@ -426,6 +428,16 @@ impl ChunkStore { > drop(lock); > continue; > } > + if filename > + .to_bytes() > + .ends_with(USING_MARKER_FILENAME_EXT.as_bytes()) > + { > + unlinkat(Some(dirfd), filename, UnlinkatFlags::NoRemoveDir).map_err(|err| { > + format_err!("unlinking chunk using marker {filename:?} failed - {err}") > + })?; > + drop(lock); > + continue; > + } this looks okay as a stop-gap, but isn't the actual problem that .using and .0.bad have the same length, so we end up taking a codepath using a weird "bad but not bad" filename instead of skipping those markers in phase3? in get_chunk_iterator, we skip all files that are not 64 bytes or 64+len(.0.bad) bytes long, but then set the "bad" flag based on the extension.. and then in cond_sweep_chunk in sweep_unused_chunk, we convert non-bad chunk filenames to digests which then fails for the "using" filenames, because they are too long (per the error from the forum thread). > > chunk_count += 1; > > @@ -776,7 +788,7 @@ impl ChunkStore { > /// Helper to generate marker file path for expected chunks > fn chunk_expected_marker_path(&self, digest: &[u8; 32]) -> PathBuf { > let (mut path, _digest_str) = self.chunk_path(digest); > - path.set_extension("using"); > + path.set_extension(USING_MARKER_FILENAME_EXT); > path > } > > -- > 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