From b4b14dc16e4469e1210c9177d7efe1a6c682b61f Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Wed, 28 Oct 2020 15:33:04 +0100 Subject: [PATCH] do_verification_job: fix "never-reverify" and refactor/comment commit a4915dfc2bc7bef03354f97f5bbce9fe2df4e0d6 made a wrong fix, as it did not observed that the last expressions was done under the invariant that we had a last verification result, because if none could be loaded we already returned true (include). It thus broke the case for "never re-verify", which is important when using multiple schedules, a more high frequent one for new, unverified snapshots, and a low frequency to re-verify older snapshots, e.g., monthly. Fix this case again, rework the code to avoid this easy to oversee invariant. Use a nested match to better express the implication of each setting, and add some comments. Signed-off-by: Thomas Lamprecht --- src/backup/verify.rs | 2 +- src/server/verify_job.rs | 29 ++++++++++++++++------------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/backup/verify.rs b/src/backup/verify.rs index 8178ef8e..151a92e7 100644 --- a/src/backup/verify.rs +++ b/src/backup/verify.rs @@ -442,7 +442,7 @@ pub fn verify_backup_group( if filter(&info) == false { task_log!( worker, - "SKIPPED: verify {}:{} (already verified)", + "SKIPPED: verify {}:{} (recently verified)", datastore.name(), info.backup_dir, ); diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs index 71220f65..a96371df 100644 --- a/src/server/verify_job.rs +++ b/src/server/verify_job.rs @@ -26,29 +26,32 @@ pub fn do_verification_job( let datastore2 = datastore.clone(); let outdated_after = verification_job.outdated_after.clone(); - let ignore_verified = verification_job.ignore_verified.unwrap_or(true); + let ignore_verified_snapshots = verification_job.ignore_verified.unwrap_or(true); let filter = move |backup_info: &BackupInfo| { - if !ignore_verified { + if !ignore_verified_snapshots { return true; } let manifest = match datastore2.load_manifest(&backup_info.backup_dir) { Ok((manifest, _)) => manifest, - Err(_) => return true, + Err(_) => return true, // include, so task picks this up as error }; let raw_verify_state = manifest.unprotected["verify_state"].clone(); - let last_state = match serde_json::from_value::(raw_verify_state) { - Ok(last_state) => last_state, - Err(_) => return true, - }; + match serde_json::from_value::(raw_verify_state) { + Err(_) => return true, // no last verification, always include + Ok(last_verify) => { + match outdated_after { + None => false, // never re-verify if ignored and no max age + Some(max_age) => { + let now = proxmox::tools::time::epoch_i64(); + let days_since_last_verify = (now - last_verify.upid.starttime) / 86400; - let now = proxmox::tools::time::epoch_i64(); - let days_since_last_verify = (now - last_state.upid.starttime) / 86400; - - outdated_after - .map(|v| days_since_last_verify > v) - .unwrap_or(true) + days_since_last_verify > max_age + } + } + } + } }; let email = crate::server::lookup_user_email(userid);