From 0bcbb1badd47142b55a31f4086d5fe65f0cd43ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
Date: Wed, 20 Nov 2024 19:34:54 +0100
Subject: [PATCH] push: reduce calls to list_snapshots on target side
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

instead of calling this three times, call it once:

retrieving the highest backup timestamp doesn't need its own request, it can
re-use the "main" result, the corresponding helper can thus be dropped.

remove_vanished can re-use the earlier result - if anybody prunes the backup
group or adds new snapshots while the sync is running, the whole group sync is
racy and might cause spurious errors anyway.

since re-syncing the last already existing snapshot is not possible at the
moment, the code can also be simplified by treating such a snapshots already
fully synced.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/server/push.rs | 30 ++++++------------------------
 1 file changed, 6 insertions(+), 24 deletions(-)

diff --git a/src/server/push.rs b/src/server/push.rs
index 5d537605..cb6afbe8 100644
--- a/src/server/push.rs
+++ b/src/server/push.rs
@@ -579,16 +579,6 @@ async fn fetch_target_snapshots(
     Ok(snapshots)
 }
 
-async fn fetch_previous_backup_time(
-    params: &PushParameters,
-    target_namespace: &BackupNamespace,
-    group: &BackupGroup,
-) -> Result<Option<i64>, Error> {
-    let mut snapshots = fetch_target_snapshots(params, target_namespace, group).await?;
-    snapshots.sort_unstable_by(|a, b| a.backup.time.cmp(&b.backup.time));
-    Ok(snapshots.last().map(|snapshot| snapshot.backup.time))
-}
-
 async fn forget_target_snapshot(
     params: &PushParameters,
     target_namespace: &BackupNamespace,
@@ -635,8 +625,12 @@ pub(crate) async fn push_group(
         .unwrap_or_default();
 
     let target_namespace = params.map_to_target(namespace)?;
-    let last_snapshot_time = fetch_previous_backup_time(params, &target_namespace, group)
-        .await?
+    let mut target_snapshots = fetch_target_snapshots(params, &target_namespace, group).await?;
+    target_snapshots.sort_unstable_by_key(|a| a.backup.time);
+
+    let last_snapshot_time = target_snapshots
+        .last()
+        .map(|snapshot| snapshot.backup.time)
         .unwrap_or(i64::MIN);
 
     let mut source_snapshots = HashSet::new();
@@ -669,20 +663,9 @@ pub(crate) async fn push_group(
 
     progress.group_snapshots = snapshots.len() as u64;
 
-    let target_snapshots = fetch_target_snapshots(params, &target_namespace, group).await?;
-    let target_snapshots: Vec<BackupDir> = target_snapshots
-        .into_iter()
-        .map(|snapshot| snapshot.backup)
-        .collect();
-
     let mut stats = SyncStats::default();
     let mut fetch_previous_manifest = !target_snapshots.is_empty();
     for (pos, source_snapshot) in snapshots.into_iter().enumerate() {
-        if target_snapshots.contains(&source_snapshot) {
-            progress.done_snapshots = pos as u64 + 1;
-            info!("percentage done: {progress}");
-            continue;
-        }
         let result =
             push_snapshot(params, namespace, &source_snapshot, fetch_previous_manifest).await;
         fetch_previous_manifest = true;
@@ -696,7 +679,6 @@ pub(crate) async fn push_group(
     }
 
     if params.remove_vanished {
-        let target_snapshots = fetch_target_snapshots(params, &target_namespace, group).await?;
         for snapshot in target_snapshots {
             if source_snapshots.contains(&snapshot.backup.time) {
                 continue;