From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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
<pbs-devel@lists.proxmox.com>, Hannes Laimer <h.laimer@proxmox.com>
References: <20210611075046.34449-1-h.laimer@proxmox.com>
<20210611075046.34449-2-h.laimer@proxmox.com>
From: Dominik Csapak <d.csapak@proxmox.com>
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
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=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<i64>,
+ 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 <h.laimer@proxmox.com>
> ---
> 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<i64>,
> +) -> Box<dyn std::marker::Sync + RefUnwindSafe + Fn(&BackupManifest) -> 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::<SnapshotVerifyState>(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::<SnapshotVerifyState>(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 {
>