[pbs-devel] [PATCH proxmox-backup 1/3] datastore: group: return basic stats on backup group destroy

[Christian Ebner <c.ebner@proxmox.com>]

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 <c.ebner@proxmox.com>
---
 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<bool, Error> {
+    /// Returns `BackupGroupDeleteStats`, containing the number of deleted snapshots
+    /// and number of protected snaphsots, which therefore were not removed.
+    pub fn destroy(&self) -> Result<BackupGroupDeleteStats, Error> {
         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<Self>,
         ns: &BackupNamespace,
         backup_group: &pbs_api_types::BackupGroup,
-    ) -> Result<bool, Error> {
+    ) -> Result<BackupGroupDeleteStats, Error> {
         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