From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 64A261FF0E7 for ; Fri, 10 Jul 2026 14:03:11 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 11B8A2142F; Fri, 10 Jul 2026 14:03:11 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 10 Jul 2026 14:02:38 +0200 Message-Id: Subject: Re: [PATCH datacenter-manager 04/15] task cache: rotate: align timestamp for new files to UTC midnight From: "Lukas Wagner" To: "Dominik Csapak" , "Lukas Wagner" , X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260702092258.174740-1-l.wagner@proxmox.com> <20260702092258.174740-5-l.wagner@proxmox.com> <655cfb23-835d-456d-a2f8-5bc4ae0f7c07@proxmox.com> In-Reply-To: <655cfb23-835d-456d-a2f8-5bc4ae0f7c07@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1783684947537 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.000 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: GQZZNCDV7KHYTGO7YZVN2VTGPCTTSA7Z X-Message-ID-Hash: GQZZNCDV7KHYTGO7YZVN2VTGPCTTSA7Z X-MailFrom: l.wagner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Fri Jul 10, 2026 at 1:36 PM CEST, Dominik Csapak wrote: > wouldn't it make more sense to put the alignment inside rotate? so=20 > potential future callers can't miss this? That would of course also be possible, but semantically I think it fits better in the caller. It does not really make a big practical difference for us here, but if the task_cache module was reused somewhere else, a different callers might have different expectations when it comes to alignment (example, maybe prefer midnight in the local timezone, as a trivial, maybe slightly synthetic example). Not saying that putting the alignment into 'rotate/init' would be wrong, just explaining my thinking here. > > also the init cache function does not align the timestamp > so it's aligned only after rotation, is that intended? Just an oversight, actually. I'll add the alignment to the init call as well. Thanks! > > On 7/2/26 11:23 AM, Lukas Wagner wrote: >> This also requires to change the rotation check to an inclusive >=3D, >> otherwise we would only rotate after two days instead of every day. >>=20 >> Signed-off-by: Lukas Wagner >> --- >> server/src/remote_tasks/refresh_task.rs | 23 ++++++++++++++++++++++- >> server/src/remote_tasks/task_cache.rs | 3 ++- >> 2 files changed, 24 insertions(+), 2 deletions(-) >>=20 >> diff --git a/server/src/remote_tasks/refresh_task.rs b/server/src/remote= _tasks/refresh_task.rs >> index 68ef5a25..55d5694e 100644 >> --- a/server/src/remote_tasks/refresh_task.rs >> +++ b/server/src/remote_tasks/refresh_task.rs >> @@ -360,7 +360,13 @@ fn get_remotes_with_finished_tasks( >> /// >> /// Returns Ok(true) the cache's files were rotated. >> async fn rotate_cache(cache: TaskCache) -> Result { >> - tokio::task::spawn_blocking(move || cache.write()?.rotate(proxmox_t= ime::epoch_i64())).await? >> + tokio::task::spawn_blocking(move || { >> + cache.write()?.rotate(align_timestamp( >> + proxmox_time::epoch_i64(), >> + super::ROTATE_AFTER as i64, >> + )) >> + }) >> + .await? >> } >> =20 >> /// Apply the task cache journal. >> @@ -541,3 +547,18 @@ async fn update_task_cache( >> }) >> .await? >> } >> + >> +/// Align a UNIX timestamp to the specified interval. >> +fn align_timestamp(now: i64, interval: i64) -> i64 { >> + now - (now.rem_euclid(interval)) >> +} >> + >> +#[cfg(test)] >> +mod tests { >> + use crate::remote_tasks::refresh_task::align_timestamp; >> + >> + #[test] >> + fn test_align_timestamp() { >> + assert_eq!(align_timestamp(1782898009, 24 * 3600), 1782864000); >> + } >> +} >> diff --git a/server/src/remote_tasks/task_cache.rs b/server/src/remote_t= asks/task_cache.rs >> index b5fc3b21..55aa8b70 100644 >> --- a/server/src/remote_tasks/task_cache.rs >> +++ b/server/src/remote_tasks/task_cache.rs >> @@ -285,7 +285,8 @@ impl WritableTaskCache { >> =20 >> match archive_files.first() { >> Some(bound) =3D> { >> - if now > bound.starttime && now - bound.starttime > sel= f.cache.rotate_after as i64 { >> + if now > bound.starttime && now - bound.starttime >=3D = self.cache.rotate_after as i64 >> + { >> start_new_file(&mut archive_files)?; >> } >> }