all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Gabriel Goller <g.goller@proxmox.com>, pbs-devel@lists.proxmox.com
Subject: Re: [pbs-devel] [PATCH proxmox-backup v5 0/4] fix #3786: resync corrupt chunks in sync-job
Date: Fri, 22 Nov 2024 11:37:30 +0100	[thread overview]
Message-ID: <173227185008.2118190.9441547307434136940@yuna.proxmox.com> (raw)
In-Reply-To: <20241122093919.59777-1-g.goller@proxmox.com>

w.r.t. the off-list discussion - I think resync-corrupt is okay as standalone
option, it matches with the others like remove_vanished.

but I noticed another thing that requires some more changes - we need to only
allow resync-corrupt for pull syncs, not for push ones (for now - it's not
impossible to implement it for push as well, but it requires some backend
changes and thoughts about the priv implications).

Quoting Gabriel Goller (2024-11-22 10:39:15)
> 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 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         | 11 +++++
>  12 files changed, 155 insertions(+), 43 deletions(-)
> 
> 
> Summary over all repositories:
>   12 files changed, 155 insertions(+), 43 deletions(-)
> 
> -- 
> Generated by git-murpp 0.7.1
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
>


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


  parent reply	other threads:[~2024-11-22 10:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-22  9:39 Gabriel Goller
2024-11-22  9:39 ` [pbs-devel] [PATCH proxmox-backup v5 1/4] snapshot: add helper function to retrieve verify_state Gabriel Goller
2024-11-22  9:39 ` [pbs-devel] [PATCH proxmox-backup v5 2/4] fix #3786: api: add resync-corrupt option to sync jobs Gabriel Goller
2024-11-22  9:39 ` [pbs-devel] [PATCH proxmox-backup v5 3/4] fix #3786: ui/cli: add resync-corrupt option on sync-jobs Gabriel Goller
2024-11-22  9:39 ` [pbs-devel] [PATCH proxmox-backup v5 4/4] fix #3786: docs: add resync-corrupt option to sync-job Gabriel Goller
2024-11-22 10:37 ` Fabian Grünbichler [this message]
2024-11-22 12:15 ` [pbs-devel] [PATCH proxmox-backup v5 0/4] fix #3786: resync corrupt chunks in sync-job Gabriel Goller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=173227185008.2118190.9441547307434136940@yuna.proxmox.com \
    --to=f.gruenbichler@proxmox.com \
    --cc=g.goller@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal