From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 66B731FF164 for ; Fri, 22 Nov 2024 11:38:01 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id DBCBA101E9; Fri, 22 Nov 2024 11:38:09 +0100 (CET) MIME-Version: 1.0 In-Reply-To: <20241122093919.59777-1-g.goller@proxmox.com> References: <20241122093919.59777-1-g.goller@proxmox.com> From: Fabian =?utf-8?q?Gr=C3=BCnbichler?= To: Gabriel Goller , pbs-devel@lists.proxmox.com Date: Fri, 22 Nov 2024 11:37:30 +0100 Message-ID: <173227185008.2118190.9441547307434136940@yuna.proxmox.com> User-Agent: alot/0.10 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.049 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox-backup v5 0/4] fix #3786: resync corrupt chunks in sync-job X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" 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