Re: [pbs-devel] [PATCH proxmox-backup] verify: handle manifest update errors as non-fatal

[Gabriel Goller <g.goller@proxmox.com>]

On 28.01.2025 12:47, Christian Ebner wrote:
>Since commit 8ea00f6e ("allow to abort verify jobs") errors
>propagated up to the verify jobs worker call side are interpreted as
>job aborts.
>
>The manifest update did not honor this, leading to the verify job
>being aborted with the misleading log entry:
>`verification failed - job aborted`
>
>Instead, handle the manifest update error non-fatal just like any
>other verification related error, log it including the error message
>and continue verification with the next item.
>
>Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>---
> src/backup/verify.rs | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
>diff --git a/src/backup/verify.rs b/src/backup/verify.rs
>index 840a37859..02478b165 100644
>--- a/src/backup/verify.rs
>+++ b/src/backup/verify.rs
>@@ -3,7 +3,7 @@ use std::sync::atomic::{AtomicUsize, Ordering};
> use std::sync::{Arc, Mutex};
> use std::time::Instant;
>
>-use anyhow::{bail, format_err, Error};
>+use anyhow::{bail, Error};
> use nix::dir::Dir;
> use tracing::{error, info, warn};
>
>@@ -399,12 +399,20 @@ pub fn verify_backup_dir_with_lock(
>         state: verify_result,
>         upid,
>     };
>-    let verify_state = serde_json::to_value(verify_state)?;
>-    backup_dir
>-        .update_manifest(|manifest| {
>+
>+    if let Err(err) = {
>+        let verify_state = serde_json::to_value(verify_state)?;
>+        backup_dir.update_manifest(|manifest| {
>             manifest.unprotected["verify_state"] = verify_state;
>         })
>-        .map_err(|err| format_err!("unable to update manifest blob - {}", err))?;
>+    } {
>+        info!(
>+            "verify {}:{} - manifest update error: {err}",
>+            verify_worker.datastore.name(),
>+            backup_dir.dir(),
>+        );

Is there any reason for not using tracing::error? This would be nice to
find in the syslog as well. Also using "{err:#}" would print the whole
error chain/context.

>+        return Ok(false);
>+    }
>
>     Ok(error_count == 0)
> }


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel