]> git.proxmox.com Git - proxmox.git/commitdiff
rest server: log rotation: fix off-by-one for max_days
authorThomas Lamprecht <t.lamprecht@proxmox.com>
Thu, 7 Apr 2022 10:57:03 +0000 (12:57 +0200)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Thu, 7 Apr 2022 10:58:32 +0000 (12:58 +0200)
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 <t.lamprecht@proxmox.com>
proxmox-rest-server/src/worker_task.rs

index fd6ad86a6d35d26dbba8d36bde0f5c4a32b36c94..7b959b4542a940cf5e3a84ffaa84bcd420e72281 100644 (file)
@@ -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);
                 }