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 9AC6A1FF15E for ; Fri, 18 Oct 2024 10:42:52 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 651261E1C9; Fri, 18 Oct 2024 10:43:27 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Date: Fri, 18 Oct 2024 10:42:42 +0200 Message-Id: <20241018084242.144010-32-c.ebner@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20241018084242.144010-1-c.ebner@proxmox.com> References: <20241018084242.144010-1-c.ebner@proxmox.com> 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 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 v5 proxmox-backup 31/31] server: sync job: use delete stats provided by the api 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" 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 --- changes since version 4: - no changes 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, } /// 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 { 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 Result<(), Error> { +) -> Result { 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) = ¶ms.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 { 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) = ¶ms.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 { 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 { + 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