[pbs-devel] [PATCH proxmox-backup 0/2] let users control the amount of task logs saved

[pbs-devel] [PATCH proxmox-backup 0/2] let users control the amount of task logs saved

[Dominik Csapak]

by providing a 'task-log-max-days' property which controls how many
days (approximately) we save.

simpler way would be to configure the amount of archive files, but
that does not really translate into a nice user setting (that concept
is not really visible to users)

alternatively we could change the logrotation that it rotates by
date/time instead of size/filecount.

there is no gui yet, but should not be hard to do and one can do
 proxmox-backup-manager node update --task-log-max-days 365

Dominik Csapak (2):
  config/node: add 'task_log_max_days' config
  rest-server: add option to rotate task logs by 'max_days' instead of
    'max_files'

 proxmox-rest-server/src/worker_task.rs | 49 +++++++++++++++++++++++---
 src/api2/node/config.rs                |  4 +++
 src/bin/proxmox-backup-proxy.rs        |  6 ++++
 src/config/node.rs                     |  4 +++
 4 files changed, 59 insertions(+), 4 deletions(-)

-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 1/2] config/node: add 'task_log_max_days' config

[Dominik Csapak]

to be able to configure the maximum days to keep task logs

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/node/config.rs | 4 ++++
 src/config/node.rs      | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/src/api2/node/config.rs b/src/api2/node/config.rs
index 0a119354..b25a5c21 100644
--- a/src/api2/node/config.rs
+++ b/src/api2/node/config.rs
@@ -64,6 +64,8 @@ pub enum DeletableProperty {
     ciphers_tls_1_2,
     /// Delete the default-lang property.
     default_lang,
+    /// Delete the task-log-max-days property
+    task_log_max_days,
 }
 
 #[api(
@@ -124,6 +126,7 @@ pub fn update_node_config(
                 DeletableProperty::ciphers_tls_1_3 => { config.ciphers_tls_1_3 = None; },
                 DeletableProperty::ciphers_tls_1_2 => { config.ciphers_tls_1_2 = None; },
                 DeletableProperty::default_lang => { config.default_lang = None; },
+                DeletableProperty::task_log_max_days => { config.task_log_max_days = None; },
             }
         }
     }
@@ -139,6 +142,7 @@ pub fn update_node_config(
     if update.ciphers_tls_1_3.is_some() { config.ciphers_tls_1_3 = update.ciphers_tls_1_3; }
     if update.ciphers_tls_1_2.is_some() { config.ciphers_tls_1_2 = update.ciphers_tls_1_2; }
     if update.default_lang.is_some() { config.default_lang = update.default_lang; }
+    if update.task_log_max_days.is_some() { config.task_log_max_days = update.task_log_max_days; }
 
     crate::config::node::save_config(&config)?;
 
diff --git a/src/config/node.rs b/src/config/node.rs
index 0ba87450..54c8d8c3 100644
--- a/src/config/node.rs
+++ b/src/config/node.rs
@@ -210,6 +210,10 @@ pub struct NodeConfig {
     /// Default language used in the GUI
     #[serde(skip_serializing_if = "Option::is_none")]
     pub default_lang: Option<String>,
+
+    /// Maximum days to keep Task logs
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub task_log_max_days: Option<usize>,
 }
 
 impl NodeConfig {
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 2/2] rest-server: add option to rotate task logs by 'max_days' instead of 'max_files'

[Dominik Csapak]

and use it with the configurable: 'task_log_max_days' of the node config

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 proxmox-rest-server/src/worker_task.rs | 49 +++++++++++++++++++++++---
 src/bin/proxmox-backup-proxy.rs        |  6 ++++
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/proxmox-rest-server/src/worker_task.rs b/proxmox-rest-server/src/worker_task.rs
index 643e1872..fa3bc327 100644
--- a/proxmox-rest-server/src/worker_task.rs
+++ b/proxmox-rest-server/src/worker_task.rs
@@ -208,14 +208,16 @@ pub fn init_worker_tasks(basedir: PathBuf, file_opts: CreateOptions) -> Result<(
 }
 
 /// checks if the Task Archive is bigger that 'size_threshold' bytes, and
-/// rotates it if it is
+/// rotates it if it is, keeps only up to 'max_files'.
+/// If 'max_days' is given, 'max_files' is ignored, and all archive files
+/// will be deleted where there are only tasks that are older than the given days.
 pub fn rotate_task_log_archive(
     size_threshold: u64,
     compress: bool,
     max_files: Option<usize>,
+    max_days: Option<usize>,
     options: Option<CreateOptions>,
 ) -> Result<bool, Error> {
-
     let setup = worker_task_setup()?;
 
     let _lock = setup.lock_task_list_files(true)?;
@@ -223,11 +225,50 @@ pub fn rotate_task_log_archive(
     let mut logrotate = LogRotate::new(
         &setup.task_archive_fn,
         compress,
-        max_files,
+        if max_days.is_none() { max_files } else { None },
         options,
     )?;
 
-    logrotate.rotate(size_threshold)
+    let mut rotated = logrotate.rotate(size_threshold)?;
+
+    if let Some(max_days) = max_days {
+        let mut delete = false;
+        let file_names = logrotate.file_names();
+        let mut files = logrotate.files();
+        for file_name in file_names {
+            if !delete {
+                // we only have to check if we did not start deleting already
+
+                // this is ok because the task log files are locked, so no one
+                // else should modify these
+                let reader = match files.next() {
+                    Some(file) => BufReader::new(file),
+                    None => {
+                        bail!("unexpected error: files do not match file_names");
+                    }
+                };
+                if let Some(line) = reader.lines().next() {
+                    if let Ok((_, _, Some(state))) = parse_worker_status_line(&line?) {
+                        // we approximate here with the days, but should be close enough
+                        let cutoff_time =
+                            proxmox_time::epoch_i64() - (max_days * 24 * 60 * 60) as i64;
+                        if state.endtime() < cutoff_time {
+                            // we found the first file that has only older entries, start deleting
+                            delete = true;
+                            rotated = true;
+                        }
+                    }
+                }
+            }
+            if delete {
+                if let Err(err) = std::fs::remove_file(&file_name) {
+                    log::error!("could not remove {:?}: {}", file_name, err);
+                }
+            }
+        }
+    }
+
+    Ok(rotated)
 }
 
 /// removes all task logs that are older than the oldest task entry in the
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index a4f20af1..bf57469f 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -847,6 +847,11 @@ async fn schedule_task_log_rotate() {
                 let max_size = 512 * 1024 - 1; // an entry has ~ 100b, so > 5000 entries/file
                 let max_files = 20; // times twenty files gives > 100000 task entries
 
+                let max_days = proxmox_backup::config::node::config()
+                    .map(|(cfg, _)| cfg.task_log_max_days)
+                    .ok()
+                    .flatten();
+
                 let user = pbs_config::backup_user()?;
                 let options = proxmox_sys::fs::CreateOptions::new()
                     .owner(user.uid)
@@ -856,6 +861,7 @@ async fn schedule_task_log_rotate() {
                     max_size,
                     true,
                     Some(max_files),
+                    max_days,
                     Some(options.clone()),
                 )?;
 
-- 
2.30.2

Re: [pbs-devel] [PATCH proxmox-backup 2/2] rest-server: add option to rotate task logs by 'max_days' instead of 'max_files'

[Dietmar Maurer]

patch does not apply - please can you rebase?