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 679F71FF16B for ; Thu, 12 Dec 2024 08:52:54 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 05A4516169; Thu, 12 Dec 2024 08:53:00 +0100 (CET) From: Christian Ebner To: pbs-devel@lists.proxmox.com Date: Thu, 12 Dec 2024 08:52:04 +0100 Message-Id: <20241212075204.36931-1-c.ebner@proxmox.com> X-Mailer: git-send-email 2.39.5 MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.029 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox-backup] Revert "fix #5710: api: backup: stat known chunks on backup finish" 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" Commit da11d226 ("fix #5710: api: backup: stat known chunks on backup finish") introduced a seemingly cheap server side check to verify existence of known chunks in the chunk store by stating. This check however does not scale for large backup snapshots which might contain millions of known chunks, as reported in the community forum [0]. Revert the changes for now instead of making this opt-in/opt-out, a more general approach has to be thought out to mark backup snapshots which fail verification. Link to the report in the forum: [0] https://forum.proxmox.com/threads/158812/ Fixes: da11d226 ("fix #5710: api: backup: stat known chunks on backup finish") Signed-off-by: Christian Ebner --- src/api2/backup/environment.rs | 54 +++++----------------------------- src/api2/backup/mod.rs | 22 +------------- 2 files changed, 8 insertions(+), 68 deletions(-) diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs index 19624fae3..99d885e2e 100644 --- a/src/api2/backup/environment.rs +++ b/src/api2/backup/environment.rs @@ -1,4 +1,4 @@ -use anyhow::{bail, format_err, Context, Error}; +use anyhow::{bail, format_err, Error}; use nix::dir::Dir; use std::collections::HashMap; use std::sync::{Arc, Mutex}; @@ -72,14 +72,8 @@ struct FixedWriterState { incremental: bool, } -#[derive(Copy, Clone)] -struct KnownChunkInfo { - uploaded: bool, - length: u32, -} - -// key=digest, value=KnownChunkInfo -type KnownChunksMap = HashMap<[u8; 32], KnownChunkInfo>; +// key=digest, value=length +type KnownChunksMap = HashMap<[u8; 32], u32>; struct SharedBackupState { finished: bool, @@ -165,13 +159,7 @@ impl BackupEnvironment { state.ensure_unfinished()?; - state.known_chunks.insert( - digest, - KnownChunkInfo { - uploaded: false, - length, - }, - ); + state.known_chunks.insert(digest, length); Ok(()) } @@ -225,13 +213,7 @@ impl BackupEnvironment { } // register chunk - state.known_chunks.insert( - digest, - KnownChunkInfo { - uploaded: true, - length: size, - }, - ); + state.known_chunks.insert(digest, size); Ok(()) } @@ -266,13 +248,7 @@ impl BackupEnvironment { } // register chunk - state.known_chunks.insert( - digest, - KnownChunkInfo { - uploaded: true, - length: size, - }, - ); + state.known_chunks.insert(digest, size); Ok(()) } @@ -280,23 +256,7 @@ impl BackupEnvironment { pub fn lookup_chunk(&self, digest: &[u8; 32]) -> Option { let state = self.state.lock().unwrap(); - state - .known_chunks - .get(digest) - .map(|known_chunk_info| known_chunk_info.length) - } - - /// stat known chunks from previous backup, so excluding newly uploaded ones - pub fn stat_prev_known_chunks(&self) -> Result<(), Error> { - let state = self.state.lock().unwrap(); - for (digest, known_chunk_info) in &state.known_chunks { - if !known_chunk_info.uploaded { - self.datastore - .stat_chunk(digest) - .with_context(|| format!("stat failed on {}", hex::encode(digest)))?; - } - } - Ok(()) + state.known_chunks.get(digest).copied() } /// Store the writer with an unique ID diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs index 31334b59c..0373d135b 100644 --- a/src/api2/backup/mod.rs +++ b/src/api2/backup/mod.rs @@ -1,6 +1,6 @@ //! Backup protocol (HTTP2 upgrade) -use anyhow::{bail, format_err, Context, Error}; +use anyhow::{bail, format_err, Error}; use futures::*; use hex::FromHex; use hyper::header::{HeaderValue, CONNECTION, UPGRADE}; @@ -788,26 +788,6 @@ fn finish_backup( ) -> Result { let env: &BackupEnvironment = rpcenv.as_ref(); - if let Err(err) = env.stat_prev_known_chunks() { - env.debug(format!("stat registered chunks failed - {err:?}")); - - if let Some(last) = env.last_backup.as_ref() { - // No need to acquire snapshot lock, already locked when starting the backup - let verify_state = SnapshotVerifyState { - state: VerifyState::Failed, - upid: env.worker.upid().clone(), // backup writer UPID - }; - let verify_state = serde_json::to_value(verify_state)?; - last.backup_dir - .update_manifest(|manifest| { - manifest.unprotected["verify_state"] = verify_state; - }) - .with_context(|| "manifest update failed")?; - } - - bail!("stat known chunks failed - {err:?}"); - } - env.finish_backup()?; env.log("successfully finished backup"); -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel