Re: [PATCH datacenter-manager 04/15] task cache: rotate: align timestamp for new files to UTC midnight

["Lukas Wagner" <l.wagner@proxmox.com>]

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 
> 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 >=,
>> otherwise we would only rotate after two days instead of every day.
>> 
>> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>> ---
>>   server/src/remote_tasks/refresh_task.rs | 23 ++++++++++++++++++++++-
>>   server/src/remote_tasks/task_cache.rs   |  3 ++-
>>   2 files changed, 24 insertions(+), 2 deletions(-)
>> 
>> 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<bool, Error> {
>> -    tokio::task::spawn_blocking(move || cache.write()?.rotate(proxmox_time::epoch_i64())).await?
>> +    tokio::task::spawn_blocking(move || {
>> +        cache.write()?.rotate(align_timestamp(
>> +            proxmox_time::epoch_i64(),
>> +            super::ROTATE_AFTER as i64,
>> +        ))
>> +    })
>> +    .await?
>>   }
>>   
>>   /// 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_tasks/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 {
>>   
>>           match archive_files.first() {
>>               Some(bound) => {
>> -                if now > bound.starttime && now - bound.starttime > self.cache.rotate_after as i64 {
>> +                if now > bound.starttime && now - bound.starttime >= self.cache.rotate_after as i64
>> +                {
>>                       start_new_file(&mut archive_files)?;
>>                   }
>>               }