]> git.proxmox.com Git - proxmox-backup.git/commitdiff
log rotate: do NOT compress first rotation
authorThomas Lamprecht <t.lamprecht@proxmox.com>
Tue, 20 Oct 2020 08:26:28 +0000 (10:26 +0200)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Tue, 20 Oct 2020 09:09:17 +0000 (11:09 +0200)
The first rotation is normally the one still opened by one or more
processes for writing, so it must NOT be replaced, removed, ..., as
this then makes the remaining logging, until those processes are
noticed that they should reopen the logfile due to rotation, goes
into nirvana, which is far from ideal for a log.

Only rotating (renaming) is OK for this active file, as this does not
invalidates the file and keeps open FDs intact.

So start compressing with the second rotation, which should be clear
to use, as all writers must have been told to reopen the log during
the last rotation, reopen is a fast operation and normally triggered
at least day ago (at least if one did not dropped the state file
manually), so we are fine to archive that one for real.
If we plan to allow faster rotation the whole rotation+reopen should
be locked, so that we can guarantee that all writers switched over,
but this is unlikely to be needed.

Again, this is was logrotate sanely does by default since forever.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
src/tools/logrotate.rs

index 28e3b7bb69138f0bf081d18d2e6f8b2d72b2f985..54fe4bfe06dd3ed1fa13fcf6460adb5b6bd82078 100644 (file)
@@ -100,17 +100,16 @@ impl LogRotate {
         filenames.push(PathBuf::from(next_filename));
         let count = filenames.len();
 
-        // rotate all but the first, that we maybe have to compress
-        for i in (1..count-1).rev() {
+        for i in (0..count-1).rev() {
             rename(&filenames[i], &filenames[i+1])?;
         }
 
         if self.compress {
-            if filenames[0].extension().unwrap_or(std::ffi::OsStr::new("")) != "zst" {
-                Self::compress(&filenames[0], &options)?;
+            for i in 2..count-1 {
+                if filenames[i].extension().unwrap_or(std::ffi::OsStr::new("")) != "zst" {
+                    Self::compress(&filenames[i], &options)?;
+                }
             }
-        } else {
-            rename(&filenames[0], &filenames[1])?;
         }
 
         if let Some(max_files) = max_files {