From 5eaab7864ab95d680ed6de6f5a5d28daa37f4be0 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Thu, 7 Apr 2022 12:57:03 +0200 Subject: [PATCH] rest server: log rotation: fix off-by-one for max_days The entries in a file go from oldest end-time in the first time to newest end-time in the last line. So, just because the first line is older than the cut-off time, the remaining one doesn't necessarily have to be old enough too. What we can know for sure that older than the current checked rotations of the task archive are definitively up for deletion. Another possibility would be to check the last line, but as scanning backwards is more expensive/complex to do while only being an actual improvement in a very specific edge case (it's more likely to have a mixed time-cutoff vs. task-log-file boundary than that those are aligned) Signed-off-by: Thomas Lamprecht --- proxmox-rest-server/src/worker_task.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/proxmox-rest-server/src/worker_task.rs b/proxmox-rest-server/src/worker_task.rs index fd6ad86a..7b959b45 100644 --- a/proxmox-rest-server/src/worker_task.rs +++ b/proxmox-rest-server/src/worker_task.rs @@ -238,6 +238,11 @@ pub fn rotate_task_log_archive( let mut delete = false; let file_names = logrotate.file_names(); let mut files = logrotate.files(); + // task archives have task-logs sorted by endtime, with the oldest at the start of the + // file. So, peak into every archive and see if the first listed tasks' endtime would be + // cut off. If that's the case we know that the next (older) task archive rotation surely + // falls behind the cut-off line. We cannot say the same for the current archive without + // checking its last (newest) line, but that's more complex, expensive and rather unlikely. for file_name in file_names { if !delete { // we only have to check if we did not start deleting already @@ -262,8 +267,7 @@ pub fn rotate_task_log_archive( } } } - } - if delete { + } else { if let Err(err) = std::fs::remove_file(&file_name) { log::error!("could not remove {:?}: {}", file_name, err); }