public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] applied: [PATCH backup 3/4] log rotate: do NOT compress first rotation
Date: Tue, 20 Oct 2020 11:10:43 +0200	[thread overview]
Message-ID: <20201020091044.6971-3-t.lamprecht@proxmox.com> (raw)
In-Reply-To: <20201020091044.6971-1-t.lamprecht@proxmox.com>

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 | 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





  parent reply	other threads:[~2020-10-20  9:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-20  9:10 [pbs-devel] applied: [PATCH backup 1/4] log rotate: do NOT overwrite file with possible writers Thomas Lamprecht
2020-10-20  9:10 ` [pbs-devel] applied: [PATCH backup 2/4] log rotate: factor out compression in private function Thomas Lamprecht
2020-10-20  9:10 ` Thomas Lamprecht [this message]
2020-10-20  9:10 ` [pbs-devel] applied: [PATCH backup 4/4] log rotate: move basic rotation logic into module for reuse Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201020091044.6971-3-t.lamprecht@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal