From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 24591B86B0 for ; Fri, 8 Mar 2024 14:02:09 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 042D6DC69 for ; Fri, 8 Mar 2024 14:02:09 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 8 Mar 2024 14:02:05 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 23EDB488CE for ; Fri, 8 Mar 2024 14:02:05 +0100 (CET) From: Christian Ebner To: pbs-devel@lists.proxmox.com Date: Fri, 8 Mar 2024 14:01:48 +0100 Message-Id: <20240308130150.310352-2-c.ebner@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240308130150.310352-1-c.ebner@proxmox.com> References: <20240308130150.310352-1-c.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.040 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 T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pbs-devel] [PATCH proxmox-backup 1/3] datastore: group: return basic stats on backup group destroy 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: , X-List-Received-Date: Fri, 08 Mar 2024 13:02:09 -0000 No functional change intended: In preparation for including the removed vanished groups and snapshots statistics in a sync jobs task log output. Instead of returning a boolean value showing whether all of the snapshots of the group have been removed, return an instance of `BackupGroupDeleteStats`, containing the count of deleted and protected snapshots, the latter not having been removed from the group. The `removed_all` method is introduced as replacement for the previous boolean return value and can be used to check if all snapshots have been removed. If there are no protected snapshots, the group is considered to be deleted. Signed-off-by: Christian Ebner --- pbs-datastore/src/backup_info.rs | 44 +++++++++++++++++++++++++++----- pbs-datastore/src/datastore.rs | 11 ++++---- src/api2/admin/datastore.rs | 3 ++- src/server/pull.rs | 21 +++++++-------- 4 files changed, 57 insertions(+), 22 deletions(-) diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs index 128315ba..bdfaeabc 100644 --- a/pbs-datastore/src/backup_info.rs +++ b/pbs-datastore/src/backup_info.rs @@ -17,6 +17,36 @@ use crate::manifest::{ }; use crate::{DataBlob, DataStore}; +#[derive(Default)] +pub struct BackupGroupDeleteStats { + // Count of protected snapshots, therefore not removed + unremoved_protected: usize, + // Count of deleted snapshots + removed_snapshots: usize, +} + +impl BackupGroupDeleteStats { + pub fn all_removed(&self) -> bool { + self.unremoved_protected == 0 + } + + pub fn removed_snapshots(&self) -> usize { + self.removed_snapshots + } + + pub fn protected_snapshots(&self) -> usize { + self.unremoved_protected + } + + fn increment_removed_snapshots(&mut self) { + self.removed_snapshots += 1; + } + + fn increment_protected_snapshots(&mut self) { + self.unremoved_protected += 1; + } +} + /// BackupGroup is a directory containing a list of BackupDir #[derive(Clone)] pub struct BackupGroup { @@ -197,30 +227,32 @@ impl BackupGroup { /// Destroy the group inclusive all its backup snapshots (BackupDir's) /// - /// Returns true if all snapshots were removed, and false if some were protected - pub fn destroy(&self) -> Result { + /// Returns `BackupGroupDeleteStats`, containing the number of deleted snapshots + /// and number of protected snaphsots, which therefore were not removed. + pub fn destroy(&self) -> Result { let path = self.full_group_path(); let _guard = proxmox_sys::fs::lock_dir_noblock(&path, "backup group", "possible running backup")?; log::info!("removing backup group {:?}", path); - let mut removed_all_snaps = true; + let mut delete_stats = BackupGroupDeleteStats::default(); for snap in self.iter_snapshots()? { let snap = snap?; if snap.is_protected() { - removed_all_snaps = false; + delete_stats.increment_protected_snapshots(); continue; } snap.destroy(false)?; + delete_stats.increment_removed_snapshots(); } - if removed_all_snaps { + if delete_stats.all_removed() { std::fs::remove_dir_all(&path).map_err(|err| { format_err!("removing group directory {:?} failed - {}", path, err) })?; } - Ok(removed_all_snaps) + Ok(delete_stats) } /// Returns the backup owner. diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs index 2f0e5279..2afaf7a1 100644 --- a/pbs-datastore/src/datastore.rs +++ b/pbs-datastore/src/datastore.rs @@ -23,7 +23,7 @@ use pbs_api_types::{ DatastoreTuning, GarbageCollectionStatus, Operation, UPID, }; -use crate::backup_info::{BackupDir, BackupGroup}; +use crate::backup_info::{BackupDir, BackupGroup, BackupGroupDeleteStats}; use crate::chunk_store::ChunkStore; use crate::dynamic_index::{DynamicIndexReader, DynamicIndexWriter}; use crate::fixed_index::{FixedIndexReader, FixedIndexWriter}; @@ -464,8 +464,8 @@ impl DataStore { let mut removed_all_groups = true; for group in self.iter_backup_groups(ns.to_owned())? { - let removed_group = group?.destroy()?; - removed_all_groups = removed_all_groups && removed_group; + let delete_stats = group?.destroy()?; + removed_all_groups = removed_all_groups && delete_stats.all_removed(); } let base_file = std::fs::File::open(self.base_path())?; @@ -545,12 +545,13 @@ impl DataStore { /// Remove a complete backup group including all snapshots. /// - /// Returns true if all snapshots were removed, and false if some were protected + /// Returns `BackupGroupDeleteStats`, containing the number of deleted snapshots + /// and number of protected snaphsots, which therefore were not removed. pub fn remove_backup_group( self: &Arc, ns: &BackupNamespace, backup_group: &pbs_api_types::BackupGroup, - ) -> Result { + ) -> Result { let backup_group = self.backup_group(ns.clone(), backup_group.clone()); backup_group.destroy() diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs index a95031e7..f7164b87 100644 --- a/src/api2/admin/datastore.rs +++ b/src/api2/admin/datastore.rs @@ -294,7 +294,8 @@ pub async fn delete_group( &group, )?; - if !datastore.remove_backup_group(&ns, &group)? { + let delete_stats = datastore.remove_backup_group(&ns, &group)?; + if !delete_stats.all_removed() { bail!("group only partially deleted due to protected snapshots"); } diff --git a/src/server/pull.rs b/src/server/pull.rs index fc369056..d2bdd35b 100644 --- a/src/server/pull.rs +++ b/src/server/pull.rs @@ -1498,18 +1498,19 @@ pub(crate) async fn pull_ns( continue; } task_log!(worker, "delete vanished group '{local_group}'",); - match params + let delete_stats_result = params .target .store - .remove_backup_group(&target_ns, local_group) - { - Ok(true) => {} - Ok(false) => { - task_log!( - worker, - "kept some protected snapshots of group '{}'", - local_group - ); + .remove_backup_group(&target_ns, local_group); + + match delete_stats_result { + Ok(stats) => { + if !stats.all_removed() { + task_log!( + worker, + "kept some protected snapshots of group '{local_group}'", + ); + } } Err(err) => { task_log!(worker, "{}", err); -- 2.39.2