do_verification_job: fix "never-reverify" and refactor/comment

commit a4915dfc2b 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 <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht 2020-10-28 15:33:04 +01:00
parent c4a45ec744
commit b4b14dc16e
2 changed files with 17 additions and 14 deletions

View File

@ -442,7 +442,7 @@ pub fn verify_backup_group(
if filter(&info) == false { if filter(&info) == false {
task_log!( task_log!(
worker, worker,
"SKIPPED: verify {}:{} (already verified)", "SKIPPED: verify {}:{} (recently verified)",
datastore.name(), datastore.name(),
info.backup_dir, info.backup_dir,
); );

View File

@ -26,29 +26,32 @@ pub fn do_verification_job(
let datastore2 = datastore.clone(); let datastore2 = datastore.clone();
let outdated_after = verification_job.outdated_after.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| { let filter = move |backup_info: &BackupInfo| {
if !ignore_verified { if !ignore_verified_snapshots {
return true; return true;
} }
let manifest = match datastore2.load_manifest(&backup_info.backup_dir) { let manifest = match datastore2.load_manifest(&backup_info.backup_dir) {
Ok((manifest, _)) => manifest, 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 raw_verify_state = manifest.unprotected["verify_state"].clone();
let last_state = match serde_json::from_value::<SnapshotVerifyState>(raw_verify_state) { match serde_json::from_value::<SnapshotVerifyState>(raw_verify_state) {
Ok(last_state) => last_state, Err(_) => return true, // no last verification, always include
Err(_) => return true, 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 now = proxmox::tools::time::epoch_i64();
let days_since_last_verify = (now - last_state.upid.starttime) / 86400; let days_since_last_verify = (now - last_verify.upid.starttime) / 86400;
outdated_after days_since_last_verify > max_age
.map(|v| days_since_last_verify > v) }
.unwrap_or(true) }
}
}
}; };
let email = crate::server::lookup_user_email(userid); let email = crate::server::lookup_user_email(userid);