[pbs-devel] [PATCH v4 proxmox 31/31] server: sync job: use delete stats provided by the api

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

Use the API exposed additional delete statistics to generate the
task log output for sync jobs in push direction instead of fetching the
contents before and after deleting.

Detect older api versions of the target server for fallback.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 3:
- fetch target api version to remain backwards compatible

 src/server/push.rs | 126 +++++++++++++++++++++++++++++----------------
 1 file changed, 82 insertions(+), 44 deletions(-)

diff --git a/src/server/push.rs b/src/server/push.rs
index bf6045214..a915b6964 100644
--- a/src/server/push.rs
+++ b/src/server/push.rs
@@ -8,11 +8,12 @@ use anyhow::{bail, format_err, Error};
 use futures::stream::{self, StreamExt, TryStreamExt};
 use tokio::sync::mpsc;
 use tokio_stream::wrappers::ReceiverStream;
-use tracing::info;
+use tracing::{info, warn};
 
 use pbs_api_types::{
-    print_store_and_ns, Authid, BackupDir, BackupGroup, BackupNamespace, CryptMode, GroupFilter,
-    GroupListItem, NamespaceListItem, Operation, RateLimitConfig, Remote, SnapshotListItem,
+    print_store_and_ns, ApiVersion, ApiVersionInfo, Authid, BackupDir, BackupGroup,
+    BackupGroupDeleteStats, BackupNamespace, CryptMode, GroupFilter, GroupListItem,
+    NamespaceListItem, Operation, RateLimitConfig, Remote, SnapshotListItem,
     PRIV_REMOTE_DATASTORE_BACKUP, PRIV_REMOTE_DATASTORE_MODIFY, PRIV_REMOTE_DATASTORE_PRUNE,
 };
 use pbs_client::{BackupRepository, BackupWriter, HttpClient, MergedChunkInfo, UploadOptions};
@@ -41,6 +42,8 @@ pub(crate) struct PushTarget {
     ns: BackupNamespace,
     // Http client to connect to remote
     client: HttpClient,
+    // Api version reported by the target
+    api_version: Option<ApiVersion>,
 }
 
 /// Parameters for a push operation
@@ -106,6 +109,7 @@ impl PushParameters {
             repo,
             ns: remote_ns,
             client,
+            api_version: None,
         };
         let group_filter = group_filter.unwrap_or_default();
 
@@ -124,6 +128,17 @@ impl PushParameters {
     fn map_to_target(&self, namespace: &BackupNamespace) -> Result<BackupNamespace, Error> {
         namespace.map_prefix(&self.source.ns, &self.target.ns)
     }
+
+    // Fetch and set the api version info for the target
+    pub(crate) async fn fetch_target_api_version_info(&mut self) -> Result<(), Error> {
+        let api_path = "api2/json/version";
+        let mut result = self.target.client.get(api_path, None).await?;
+        let data = result["data"].take();
+        let version_info: ApiVersionInfo = serde_json::from_value(data)?;
+        self.target.api_version = Some(ApiVersion::try_from(version_info)?);
+
+        Ok(())
+    }
 }
 
 // Check if the job user given in the push parameters has the provided privs on the remote
@@ -167,7 +182,7 @@ async fn fetch_target_namespaces(params: &PushParameters) -> Result<Vec<BackupNa
 async fn remove_target_namespace(
     params: &PushParameters,
     namespace: &BackupNamespace,
-) -> Result<(), Error> {
+) -> Result<BackupGroupDeleteStats, Error> {
     if namespace.is_root() {
         bail!("cannot remove root namespace from target");
     }
@@ -181,14 +196,23 @@ async fn remove_target_namespace(
     );
 
     let target_ns = params.map_to_target(namespace)?;
-    let args = serde_json::json!({
+    let mut args = serde_json::json!({
         "ns": target_ns.name(),
         "delete-groups": true,
     });
 
-    params.target.client.delete(&api_path, Some(args)).await?;
+    if let Some(version) = &params.target.api_version {
+        if version.is_min_required(ApiVersion::new(3, 2, 8)) {
+            args["ignore-protected"] = serde_json::to_value(true)?;
+        }
+    }
 
-    Ok(())
+    let mut result = params.target.client.delete(&api_path, Some(args)).await?;
+    let data = result["data"].take();
+    let delete_stats: BackupGroupDeleteStats =
+        serde_json::from_value(data).unwrap_or_else(|_| BackupGroupDeleteStats::default());
+
+    Ok(delete_stats)
 }
 
 // Fetch the list of groups found on target in given namespace
@@ -229,7 +253,7 @@ async fn remove_target_group(
     params: &PushParameters,
     namespace: &BackupNamespace,
     backup_group: &BackupGroup,
-) -> Result<(), Error> {
+) -> Result<BackupGroupDeleteStats, Error> {
     check_ns_remote_datastore_privs(params, namespace, PRIV_REMOTE_DATASTORE_PRUNE)
         .map_err(|err| format_err!("Pruning remote datastore contents not allowed - {err}"))?;
 
@@ -242,14 +266,23 @@ async fn remove_target_group(
         "backup-id": backup_group.id,
         "backup-type": backup_group.ty,
     });
+
+    if let Some(version) = &params.target.api_version {
+        if version.is_min_required(ApiVersion::new(3, 2, 8)) {
+            args["ignore-protected"] = serde_json::to_value(true)?;
+        }
+    }
     if !namespace.is_root() {
         let target_ns = params.map_to_target(namespace)?;
         args["ns"] = serde_json::to_value(target_ns.name())?;
     }
 
-    params.target.client.delete(&api_path, Some(args)).await?;
+    let mut result = params.target.client.delete(&api_path, Some(args)).await?;
+    let data = result["data"].take();
+    let delete_stats: BackupGroupDeleteStats =
+        serde_json::from_value(data).unwrap_or_else(|_| BackupGroupDeleteStats::default());
 
-    Ok(())
+    Ok(delete_stats)
 }
 
 // Check if the namespace is already present on the target, create it otherwise
@@ -298,6 +331,7 @@ async fn check_or_create_target_namespace(
 /// Push contents of source datastore matched by given push parameters to target.
 pub(crate) async fn push_store(mut params: PushParameters) -> Result<SyncStats, Error> {
     let mut errors = false;
+    params.fetch_target_api_version_info().await?;
 
     // Generate list of source namespaces to push to target, limited by max-depth
     let mut namespaces = params.source.list_namespaces(&mut params.max_depth).await?;
@@ -365,9 +399,25 @@ pub(crate) async fn push_store(mut params: PushParameters) -> Result<SyncStats,
             if synced_namespaces.contains(&target_namespace) {
                 continue;
             }
-            if let Err(err) = remove_target_namespace(&params, &target_namespace).await {
-                info!("failed to remove vanished namespace {target_namespace} - {err}");
-                continue;
+            match remove_target_namespace(&params, &target_namespace).await {
+                Ok(delete_stats) => {
+                    stats.add(SyncStats::from(RemovedVanishedStats {
+                        snapshots: delete_stats.removed_snapshots(),
+                        groups: delete_stats.removed_groups(),
+                        namespaces: 1,
+                    }));
+                    if delete_stats.protected_snapshots() > 0 {
+                        warn!(
+                            "kept {protected_count} protected snapshots of namespace '{target_namespace}'",
+                            protected_count = delete_stats.protected_snapshots(),
+                        );
+                        continue;
+                    }
+                }
+                Err(err) => {
+                    warn!("failed to remove vanished namespace {target_namespace} - {err}");
+                    continue;
+                }
             }
             info!("removed vanished namespace {target_namespace}");
         }
@@ -449,38 +499,26 @@ pub(crate) async fn push_namespace(
 
             info!("delete vanished group '{target_group}'");
 
-            let count_before = match fetch_target_groups(params, namespace).await {
-                Ok(snapshots) => snapshots.len(),
-                Err(_err) => 0, // ignore errors
-            };
-
-            if let Err(err) = remove_target_group(params, namespace, &target_group).await {
-                info!("{err}");
-                errors = true;
-                continue;
-            }
-
-            let mut count_after = match fetch_target_groups(params, namespace).await {
-                Ok(snapshots) => snapshots.len(),
-                Err(_err) => 0, // ignore errors
-            };
-
-            let deleted_groups = if count_after > 0 {
-                info!("kept some protected snapshots of group '{target_group}'");
-                0
-            } else {
-                1
-            };
-
-            if count_after > count_before {
-                count_after = count_before;
+            match remove_target_group(params, namespace, &target_group).await {
+                Ok(delete_stats) => {
+                    if delete_stats.protected_snapshots() > 0 {
+                        warn!(
+                            "kept {protected_count} protected snapshots of group '{target_group}'",
+                            protected_count = delete_stats.protected_snapshots(),
+                        );
+                    }
+                    stats.add(SyncStats::from(RemovedVanishedStats {
+                        snapshots: delete_stats.removed_snapshots(),
+                        groups: delete_stats.removed_groups(),
+                        namespaces: 0,
+                    }));
+                }
+                Err(err) => {
+                    info!("failed to delete vanished group - {err}");
+                    errors = true;
+                    continue;
+                }
             }
-
-            stats.add(SyncStats::from(RemovedVanishedStats {
-                snapshots: count_before - count_after,
-                groups: deleted_groups,
-                namespaces: 0,
-            }));
         }
     }
 
-- 
2.39.5



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