[pbs-devel] applied: [PATCH backup 3/4] log rotate: do NOT compress first rotation

Thomas Lamprecht t.lamprecht at proxmox.com
Tue Oct 20 11:10:43 CEST 2020


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 at proxmox.com>
---
 src/tools/logrotate.rs | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/tools/logrotate.rs b/src/tools/logrotate.rs
index 28e3b7bb..54fe4bfe 100644
--- a/src/tools/logrotate.rs
+++ b/src/tools/logrotate.rs
@@ -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 {
-- 
2.27.0






More information about the pbs-devel mailing list