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 025641FF15C for ; Wed, 5 Mar 2025 10:42:16 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id F20E9110DE; Wed, 5 Mar 2025 10:42:11 +0100 (CET) Date: Wed, 05 Mar 2025 10:41:35 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20250304183546.461918-1-c.ebner@proxmox.com> <20250304183546.461918-8-c.ebner@proxmox.com> In-Reply-To: <20250304183546.461918-8-c.ebner@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1741164555.od3qzrnpme.astroid@yuna.none> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.045 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: Re: [pbs-devel] [PATCH v3 proxmox-backup 7/9] datastore: conditionally use custom GC atime cutoff if set 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 March 4, 2025 7:35 pm, Christian Ebner wrote: > Use the user configured atime cutoff over the default 24h 5m > margin if explicitly set, but only if the atime safety check is > enabled and succeeded. If the atime safety check is not enabled, > fallback to use the current default. > > Signed-off-by: Christian Ebner > --- > changes since version 2: > - Adapt datastore tuning option names > - Fallback to default on relatime behaviour > > pbs-datastore/src/chunk_store.rs | 9 ++++++++- > pbs-datastore/src/datastore.rs | 28 +++++++++++++++++++++++++--- > 2 files changed, 33 insertions(+), 4 deletions(-) > > diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs > index e529dcc9c..f591412a2 100644 > --- a/pbs-datastore/src/chunk_store.rs > +++ b/pbs-datastore/src/chunk_store.rs > @@ -371,6 +371,7 @@ impl ChunkStore { > &self, > oldest_writer: i64, > phase1_start_time: i64, > + atime_cutoff: Option, we could just pass in min_atime instead of phase1_start_time? > status: &mut GarbageCollectionStatus, > worker: &dyn WorkerTaskContext, > ) -> Result<(), Error> { > @@ -380,7 +381,13 @@ impl ChunkStore { > use nix::sys::stat::fstatat; > use nix::unistd::{unlinkat, UnlinkatFlags}; > > - let mut min_atime = phase1_start_time - 3600 * 24; // at least 24h (see mount option relatime) > + let mut min_atime = if let Some(atime_cutoff) = atime_cutoff { > + // account for the default 5 min safety gap subtracted below > + phase1_start_time + 300 - atime_cutoff as i64 * 60 > + } else { > + // at least 24h (see mount option relatime) > + phase1_start_time - 3600 * 24 > + }; since this should move below > > if oldest_writer < min_atime { > min_atime = oldest_writer; > diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs > index ef932b47b..4f07cfc60 100644 > --- a/pbs-datastore/src/datastore.rs > +++ b/pbs-datastore/src/datastore.rs > @@ -1174,11 +1174,32 @@ impl DataStore { > DatastoreTuning::API_SCHEMA > .parse_property_string(gc_store_config.tuning.as_deref().unwrap_or(""))?, > )?; > - if tuning.gc_atime_safety_check.unwrap_or(true) { > - self.inner.chunk_store.atime_safety_check()?; > - info!("Filesystem atime safety check successful."); > + let gc_atime_safety_check = tuning.gc_atime_safety_check.unwrap_or(true); > + let atime_updated = if gc_atime_safety_check { > + if self.inner.chunk_store.atime_safety_check()? { > + info!("Filesystem atime safety check successful."); > + true > + } else { > + info!("Filesystem atime safety check successful with relatime behaviour."); > + false > + } > } else { > info!("Filesystem atime safety check disabled by datastore tuning options."); > + false > + }; > + let mut atime_cutoff = None; > + if let Some(cutoff) = tuning.gc_atime_cutoff { > + if atime_updated { > + info!("Using GC atime cutoff {cutoff}m."); > + atime_cutoff = Some(cutoff); > + } else { > + warn!( > + "Ignoring GC atime cutoff of {cutoff}m, because atime check is \ > + disabled or relatime behaviour detected." > + ); > + } > + } else { > + info!("Using default GC atime cutoff of 24h 5m"); > } since both the check and the decision is made here, and there is no reason to split it between the two modules.. > > info!("Start GC phase1 (mark used chunks)"); > @@ -1189,6 +1210,7 @@ impl DataStore { > self.inner.chunk_store.sweep_unused_chunks( > oldest_writer, > phase1_start_time, > + atime_cutoff, > &mut gc_status, > worker, > )?; > -- > 2.39.5 > > > > _______________________________________________ > 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