From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 67B7C1FF166 for ; Fri, 11 Oct 2024 11:32:23 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id F3FDB39FC7; Fri, 11 Oct 2024 11:32:51 +0200 (CEST) Date: Fri, 11 Oct 2024 11:32:45 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20240912143322.548839-1-c.ebner@proxmox.com> <20240912143322.548839-34-c.ebner@proxmox.com> In-Reply-To: <20240912143322.548839-34-c.ebner@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1728638968.w8kq9gw4fv.astroid@yuna.none> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.047 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: Re: [pbs-devel] [PATCH v3 proxmox-backup 33/33] 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" On September 12, 2024 4:33 pm, Christian Ebner wrote: > 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. > > Signed-off-by: Christian Ebner > --- > changes since version 2: > - no changes > > src/server/push.rs | 65 ++++++++++++++++++++-------------------------- > 1 file changed, 28 insertions(+), 37 deletions(-) > > diff --git a/src/server/push.rs b/src/server/push.rs > index cfbb88728..dbface907 100644 > --- a/src/server/push.rs > +++ b/src/server/push.rs > @@ -11,9 +11,10 @@ use tokio_stream::wrappers::ReceiverStream; > use tracing::info; > > use pbs_api_types::{ > - print_store_and_ns, Authid, BackupDir, BackupGroup, BackupNamespace, CryptMode, GroupFilter, > - GroupListItem, NamespaceListItem, Operation, RateLimitConfig, Remote, SnapshotListItem, > - PRIV_REMOTE_DATASTORE_BACKUP, PRIV_REMOTE_DATASTORE_MODIFY, PRIV_REMOTE_DATASTORE_PRUNE, > + print_store_and_ns, 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, UploadOptions}; > use pbs_config::CachedUserInfo; > @@ -228,7 +229,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}"))?; > > @@ -246,9 +247,11 @@ async fn remove_target_group( > 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)?; what about older target servers that return Value::Null here? from a quick glance, nothing else requires upgrading the target server to "enable" push support, so this should probably gracefully handle that combination as well.. > > - Ok(()) > + Ok(delete_stats) > } > > // Check if the namespace is already present on the target, create it otherwise > @@ -451,38 +454,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 { > + info!( > + "kept {protected_count} protected snapshots of group '{target_group}'", > + protected_count = delete_stats.protected_snapshots(), > + ); should this be a warning? this kind of breaks the expectations of syncing after all.. and wouldn't we also need a similar change for removing namespaces? > + } > + 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.2 > > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > > > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel