From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 3ED261FF13F for ; Wed, 14 Jan 2026 13:32:00 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 49A33118F6; Wed, 14 Jan 2026 13:31:59 +0100 (CET) From: Christian Ebner To: pbs-devel@lists.proxmox.com Date: Wed, 14 Jan 2026 13:31:37 +0100 Message-ID: <20260114123139.505214-5-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260114123139.505214-1-c.ebner@proxmox.com> References: <20260114123139.505214-1-c.ebner@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1768393869160 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.048 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 v3 4/6] datastore: move bad chunk touching logic to chunk store and lock it 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" Provide an dedicated method and encapsulate the bad chunk filename generation and access time updates. This is an implementation detail of the chunks store, the garbage collection should not be concerned about this. To assure exclusive access to the chunk store, acquire the chunk store mutex guard before performing access time updates. While there is currently no code path which would lead to races as the bad chunk renaming (which generates the bad chunks) is concerned about file existence only and the garbage collection cannot be executed concurrently, locking makes access time updates future prove. In preparation for making the bad filename generation a dedicated helper as well, all contained within the chunk store module. Signed-off-by: Christian Ebner --- changes since version 2: - lock chunk store before performing atime updates pbs-datastore/src/chunk_store.rs | 21 ++++++++++++++++++++- pbs-datastore/src/datastore.rs | 11 +---------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs index 5148d6c46..8bc74faf7 100644 --- a/pbs-datastore/src/chunk_store.rs +++ b/pbs-datastore/src/chunk_store.rs @@ -242,7 +242,7 @@ impl ChunkStore { self.cond_touch_path(&chunk_path, assert_exists) } - pub(super) fn cond_touch_path(&self, path: &Path, assert_exists: bool) -> Result { + fn cond_touch_path(&self, path: &Path, assert_exists: bool) -> Result { // unwrap: only `None` in unit tests assert!(self.locker.is_some()); @@ -276,6 +276,25 @@ impl ChunkStore { Ok(true) } + /// Update access timestamp on all bad chunks for given digest + /// + /// Gets exclusive access by acquiring the chunk store mutex guard. + pub(super) fn cond_touch_bad_chunks(&self, digest: &[u8; 32]) -> Result { + let _lock = self.mutex.lock(); + + let (chunk_path, _digest_str) = self.chunk_path(digest); + let mut is_bad = false; + for i in 0..=9 { + let bad_ext = format!("{i}.bad"); + let mut bad_path = chunk_path.clone(); + bad_path.set_extension(bad_ext); + if self.cond_touch_path(&bad_path, false)? { + is_bad = true; + } + } + Ok(is_bad) + } + fn get_chunk_store_iterator( &self, ) -> Result< diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs index c32f232d9..a84c6fe54 100644 --- a/pbs-datastore/src/datastore.rs +++ b/pbs-datastore/src/datastore.rs @@ -1355,19 +1355,10 @@ impl DataStore { } if !self.inner.chunk_store.cond_touch_chunk(digest, false)? { - let (chunk_path, _digest_str) = self.chunk_path(digest); // 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) - let mut is_bad = false; - for i in 0..=9 { - let bad_ext = format!("{i}.bad"); - let mut bad_path = chunk_path.clone(); - bad_path.set_extension(bad_ext); - if self.inner.chunk_store.cond_touch_path(&bad_path, false)? { - is_bad = true; - } - } + let is_bad = self.inner.chunk_store.cond_touch_bad_chunks(digest)?; if let Some(ref _s3_client) = s3_client { // Do not retry here, this is very unlikely to happen as chunk markers will -- 2.47.3 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel