Re: [pbs-devel] [PATCH v2 proxmox-backup 1/5] backup: don't validate chunk existance if base was recently verified

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 30.09.20 16:15, Stefan Reiter wrote:
> If the base was successfully verified within the last 7 days, we assume
> that it is okay and all chunks exist, so we don't have to check.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> 
> v2:
> * use proxmox::tools::time
> 
>  src/api2/backup/environment.rs | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
> index d515bf30..a8c9ddb4 100644
> --- a/src/api2/backup/environment.rs
> +++ b/src/api2/backup/environment.rs
> @@ -5,7 +5,7 @@ use std::collections::HashMap;
>  use ::serde::{Serialize};
>  use serde_json::{json, Value};
>  
> -use proxmox::tools::digest_to_hex;
> +use proxmox::tools::{digest_to_hex, time};
>  use proxmox::tools::fs::{replace_file, CreateOptions};
>  use proxmox::api::{RpcEnvironment, RpcEnvironmentType};
>  
> @@ -457,6 +457,32 @@ impl BackupEnvironment {
>          Ok(())
>      }
>  
> +    fn last_backup_has_recent_verify(&self) -> Result<bool, Error> {
> +        match &self.last_backup {
> +            Some(last_backup) => {
> +                let last_dir = &last_backup.backup_dir;
> +                let (manifest, _) = self.datastore.load_manifest(last_dir)?;
> +                let verify = manifest.unprotected["verify_state"].clone();
> +                match serde_json::from_value::<Option<SnapshotVerifyState>>(verify) {
> +                    Ok(verify) => match verify {
> +                        Some(verify) => {
> +                            let mut cutoff = time::TmEditor::with_epoch(time::epoch_i64(), false)?;
> +                            cutoff.add_days(-7)?;
> +                            let cutoff = cutoff.into_epoch()?;

why not just use time::epoch_i64(), avoiding a conversion to tm just
to convert it back, this is no time critical check, if there's a leap
second or something like that.

IMO, below is just more simple and everyone still gets whats meant..

let cutoff = time::epoch_i64() - 7 * 24 * 60 * 60;
Ok(verify.state == VerifyState::Ok && verify.upid.starttime > cutoff)

(but please, if you agree, do not send a v3 with the full series, this can
stand as it's own patch - out of the series)