[pbs-devel] applied-series: [PATCH proxmox-backup v6 0/4] fix #3786: resync corrupt chunks in sync-job

[Fabian Grünbichler]

with some slight rebasing (context), and one follow-up patch:

Subject: [PATCH proxmox-backup] sync config: forbid setting resync_corrupt for

they don't support it (yet), so don't allow setting it in the backend either.

Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
---
 src/api2/config/sync.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index 7ff6cae02..afaa0d5e4 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -229,6 +229,10 @@ pub fn create_sync_job(
         bail!("source and target datastore can't be the same");
     }
 
+    if sync_direction == SyncDirection::Push && config.resync_corrupt.is_some() {
+        bail!("push jobs do not support resync-corrupt option");
+    }
+
     if let Some(max_depth) = config.max_depth {
         if let Some(ref ns) = config.ns {
             ns.check_max_depth(max_depth)?;
@@ -389,6 +393,10 @@ pub fn update_sync_job(
             http_bail!(NOT_FOUND, "job '{id}' does not exist.")
         };
 
+    if sync_direction == SyncDirection::Push && update.resync_corrupt.is_some() {
+        bail!("push jobs do not support resync-corrupt option");
+    }
+
     if let Some(delete) = delete {
         for delete_prop in delete {
             match delete_prop {
-- 
2.39.5

there's some more follow-up potential:
- if we successfully re-synced a corrupt snapshot, it would be great to
  set the verifystate accordingly, since we know the snapshot is good
  now?
- if we re-sync the last snapshot during regular sync, we reset the
  verifystate from the remote side.. should we maybe compare the
  manifests after dropping the verifystate from them? but the whole
  question of "how to handle verifystate and other metadata when
  syncing" is probably a bigger can worms anyway, so feel free to ignore
  that one for now ;)

On November 22, 2024 1:16 pm, Gabriel Goller wrote:
> Add an option `resync-corrupt` that resyncs corrupt snapshots when running
> sync-job. This option checks if the local snapshot failed the last
> verification and if it did, overwrites the local snapshot with the
> remote one.
> 
> This is quite useful, as we currently don't have an option to "fix" 
> broken chunks/snapshots in any way, even if a healthy version is on 
> another (e.g. offsite) instance.
> 
> Important things to note are also: this has a slight performance 
> penalty, as all the manifests have to be looked through, and a 
> verification job has to be run beforehand, otherwise we do not know 
> if the snapshot is healthy.
> 
> Note: This series was originally written by Shannon! I just picked it 
> up, rebased, and fixed the obvious comments on the last series.
> 
> Changelog v6 (thanks @Fabian):
>  - rebase
>  - only allow resync-chunks option on pull snapshots
>  - fix typo
> 
> Changelog v5 (thanks @Fabian):
>  - rebase
>  - don't remove parsing error in verify_state helper
>  - add error logs on failures
> 
> Changelog v4 (thanks @Fabian):
>  - make verify_state bubble up errors
>  - call verify_state helper everywhere we need the verify_state
>  - resync broken manifests (so resync when load_manifest fails)
> 
> Changelog v3 (thanks @Fabian):
>  - filter out snapshots earlier in the pull_group function
>  - move verify_state to BackupManifest and fixed invocations
>  - reverted verify_state Option -> Result state (It doesn't matter if we get an
>    error, we get that quite often f.e. in new backups)
>  - removed some unnecessary log lines
>  - removed some unnecessary imports and modifications
>  - rebase to current master
> 
> Changelog v2 (thanks @Thomas):
>  - order git trailers
>  - adjusted schema description to include broken indexes
>  - change verify_state to return a Result<_,_>
>  - print error if verify_state is not able to read the state
>  - update docs on pull_snapshot function
>  - simplify logic by combining flags
>  - move log line out of loop to only print once that we resync the snapshot
> 
> Changelog since RFC (Shannon's work):
>  - rename option from deep-sync to resync-corrupt
>  - rebase on latest master (and change implementation details, as a 
>      lot has changed around sync-jobs)
> 
> proxmox-backup:
> 
> Gabriel Goller (4):
>   snapshot: add helper function to retrieve verify_state
>   fix #3786: api: add resync-corrupt option to sync jobs
>   fix #3786: ui/cli: add resync-corrupt option on sync-jobs
>   fix #3786: docs: add resync-corrupt option to sync-job
> 
>  docs/managing-remotes.rst         |  6 +++
>  pbs-api-types/src/jobs.rs         | 10 +++++
>  pbs-datastore/src/backup_info.rs  |  9 +++-
>  pbs-datastore/src/manifest.rs     | 14 +++++-
>  src/api2/admin/datastore.rs       | 16 +++----
>  src/api2/backup/mod.rs            | 18 +++++---
>  src/api2/config/sync.rs           |  4 ++
>  src/api2/pull.rs                  |  9 +++-
>  src/backup/verify.rs              | 13 +++---
>  src/bin/proxmox-backup-manager.rs | 16 ++++++-
>  src/server/pull.rs                | 72 ++++++++++++++++++++++++-------
>  www/window/SyncJobEdit.js         | 14 ++++++
>  12 files changed, 158 insertions(+), 43 deletions(-)
> 
> 
> Summary over all repositories:
>   12 files changed, 158 insertions(+), 43 deletions(-)
> 
> -- 
> Generated by git-murpp 0.7.1
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
>