From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with UTF8SMTPS id 7858C71B0C for ; Fri, 11 Jun 2021 15:18:40 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with UTF8SMTP id 6AE2A12E42 for ; Fri, 11 Jun 2021 15:18:40 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with UTF8SMTPS id 4802B12E33 for ; Fri, 11 Jun 2021 15:18:36 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with UTF8SMTP id 1905C42BA3; Fri, 11 Jun 2021 15:18:36 +0200 (CEST) Message-ID: <41f9b74f-7f30-469b-d563-c0bf80ec5ff4@proxmox.com> Date: Fri, 11 Jun 2021 15:18:35 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:90.0) Gecko/20100101 Thunderbird/90.0 Content-Language: en-US To: Proxmox Backup Server development discussion , Hannes Laimer References: <20210611075046.34449-1-h.laimer@proxmox.com> <20210611075046.34449-2-h.laimer@proxmox.com> From: Dominik Csapak In-Reply-To: <20210611075046.34449-2-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.937 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [verify.rs] Subject: Re: [pbs-devel] [PATCH v1 proxmox-backup 1/3] verify-job: move building of snapshot filter into function X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Jun 2021 13:18:40 -0000 while factoring that out is a good idea, there is no need to make a builder function that returns a closure, a much simpler diff is: --->8--- diff --git a/src/backup/verify.rs b/src/backup/verify.rs index a1b1e6dd..684fde67 100644 --- a/src/backup/verify.rs +++ b/src/backup/verify.rs @@ -575,3 +575,30 @@ pub fn verify_all_backups( Ok(errors) } + +/// Filter function for verification of snapshots +pub fn verify_filter( + ignore_verified_snapshots: bool, + outdated_after: Option, + manifest: &BackupManifest, +) -> bool { + .... old function + } +} diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs index 1dd8baa7..85245aca 100644 --- a/src/server/verify_job.rs +++ b/src/server/verify_job.rs @@ -7,8 +7,8 @@ use crate::{ config::verify::VerificationJobConfig, backup::{ DataStore, - BackupManifest, verify_all_backups, + verify_filter, }, task_log, }; let (email, notify) = crate::server::lookup_datastore_notify_settings(&verification_job.store); let job_id = format!("{}:{}", @@ -68,7 +46,12 @@ pub fn do_verification_job( } let verify_worker = crate::backup::VerifyWorker::new(worker.clone(), datastore); - let result = verify_all_backups(&verify_worker, worker.upid(), None, Some(&filter)); + let result = verify_all_backups( + &verify_worker, + worker.upid(), + None, + Some(&move |manifest| verify_filter(ignore_verified_snapshots, outdated_after, manifest)), + ); let job_result = match result { Ok(ref failed_dirs) if failed_dirs.is_empty() => Ok(()), Ok(ref failed_dirs) => { ---8<--- (i omitted the obvious parts) so simply have the filter function, and build a small closure when needed the only change this needs in 2/3 is that let filter = build....; must now be let filter = move |manifest| ....; On 6/11/21 09:50, Hannes Laimer wrote: > Signed-off-by: Hannes Laimer > --- > src/backup/verify.rs | 29 +++++++++++++++++++++++++++++ > src/server/verify_job.rs | 25 ++----------------------- > 2 files changed, 31 insertions(+), 23 deletions(-) > > diff --git a/src/backup/verify.rs b/src/backup/verify.rs > index a1b1e6dd..a0b8de58 100644 > --- a/src/backup/verify.rs > +++ b/src/backup/verify.rs > @@ -1,5 +1,6 @@ > use nix::dir::Dir; > use std::collections::HashSet; > +use std::panic::RefUnwindSafe; > use std::sync::atomic::{AtomicUsize, Ordering}; > use std::sync::{Arc, Mutex}; > use std::time::Instant; > @@ -575,3 +576,31 @@ pub fn verify_all_backups( > > Ok(errors) > } > + > +/// Builds a filter for the verification of snapshots > +pub fn build_verify_filter( > + ignore_verified_snapshots: bool, > + outdated_after: Option, > +) -> Box bool> { > + Box::new(move |manifest: &BackupManifest| { > + if !ignore_verified_snapshots { > + return true; > + } > + > + let raw_verify_state = manifest.unprotected["verify_state"].clone(); > + match serde_json::from_value::(raw_verify_state) { > + Err(_) => 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; > + > + days_since_last_verify > max_age > + } > + } > + } > + } > + }) > +} > \ No newline at end of file > diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs > index 1dd8baa7..47ac4da4 100644 > --- a/src/server/verify_job.rs > +++ b/src/server/verify_job.rs > @@ -7,7 +7,7 @@ use crate::{ > config::verify::VerificationJobConfig, > backup::{ > DataStore, > - BackupManifest, > + build_verify_filter, > verify_all_backups, > }, > task_log, > @@ -26,28 +26,6 @@ pub fn do_verification_job( > let outdated_after = verification_job.outdated_after; > let ignore_verified_snapshots = verification_job.ignore_verified.unwrap_or(true); > > - let filter = move |manifest: &BackupManifest| { > - if !ignore_verified_snapshots { > - return true; > - } > - > - let raw_verify_state = manifest.unprotected["verify_state"].clone(); > - match serde_json::from_value::(raw_verify_state) { > - Err(_) => 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; > - > - days_since_last_verify > max_age > - } > - } > - } > - } > - }; > - > let (email, notify) = crate::server::lookup_datastore_notify_settings(&verification_job.store); > > let job_id = format!("{}:{}", > @@ -67,6 +45,7 @@ pub fn do_verification_job( > task_log!(worker,"task triggered by schedule '{}'", event_str); > } > > + let filter = build_verify_filter(ignore_verified_snapshots, outdated_after); > let verify_worker = crate::backup::VerifyWorker::new(worker.clone(), datastore); > let result = verify_all_backups(&verify_worker, worker.upid(), None, Some(&filter)); > let job_result = match result { >