all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup v2 5/6] api: admin: datastore: optimize `groups` api call
Date: Wed,  8 Oct 2025 15:43:35 +0200	[thread overview]
Message-ID: <20251008134344.3512958-7-d.csapak@proxmox.com> (raw)
In-Reply-To: <20251008134344.3512958-1-d.csapak@proxmox.com>

Currently we always touch all files for each snapshot in a group when
listing them, even though we don't need all that info.

We're only interested in getting either the last finished snapshot
information, or the last unfinished one (which must the only one in
normal use, we can't have multiple unfinished snapshots usually)

Instead of getting all the information upfront, use the snapshot
iterator of the group to get only the id, sort them by time, and
use the first we're interested in, getting the snapshot specific info
only for those we want to check.

In my (admittedly extreme) setup with ~600 groups with ~1000 snapshots
each, this changes the time this api call needs from ~40s to <1s.
(on a relatively fast disk).

While at it, lift the restriction of only returning groups with
snapshots in them, now returning also empty ones.

To keep api compatibility, use a timestamp of 0 for those.
(no valid backup could have been made at that time anyway)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* split out the BackupGroup -> BackupInfo conversion into own
  'filer_map' call, and use 'find' to find the first finished one

 src/api2/admin/datastore.rs | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 0b133d166..643d1694b 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -166,36 +166,38 @@ fn backup_group_to_group_list_item(
         return None;
     }
 
-    let snapshots = match group.list_backups() {
-        Ok(snapshots) => snapshots,
+    let mut snapshots: Vec<_> = match group.iter_snapshots() {
+        Ok(snapshots) => snapshots.collect::<Result<Vec<_>, Error>>().ok()?,
         Err(_) => return None,
     };
 
     let backup_count: u64 = snapshots.len() as u64;
-    if backup_count == 0 {
-        return None;
-    }
+    let last = if backup_count == 1 {
+        // we may have only one unfinished snapshot
+        snapshots.pop().and_then(|dir| BackupInfo::new(dir).ok())
+    } else {
+        // we either have no snapshots, or at least one finished one, since we cannot have
+        // multiple unfinished ones
+        snapshots.sort_by_key(|b| std::cmp::Reverse(b.backup_time()));
+        snapshots
+            .iter()
+            .filter_map(|backup| BackupInfo::new(backup.clone()).ok())
+            .find(|info| info.is_finished())
+    };
 
-    let last_backup = snapshots
-        .iter()
-        .fold(&snapshots[0], |a, b| {
-            if a.is_finished() && a.backup_dir.backup_time() > b.backup_dir.backup_time() {
-                a
-            } else {
-                b
-            }
-        })
-        .to_owned();
+    let (last_backup, files) = last
+        .map(|info| (info.backup_dir.backup_time(), info.files))
+        .unwrap_or((0, Vec::new()));
 
     let notes_path = datastore.group_notes_path(ns, group.as_ref());
     let comment = file_read_firstline(notes_path).ok();
 
     let item = GroupListItem {
         backup: group.into(),
-        last_backup: last_backup.backup_dir.backup_time(),
+        last_backup,
         owner: Some(owner),
         backup_count,
-        files: last_backup.files,
+        files,
         comment,
     };
 
-- 
2.47.3



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


  parent reply	other threads:[~2025-10-08 13:43 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-08 13:43 [pbs-devel] [PATCH proxmox{, -backup} v2 0/7] introduce streaming content " Dominik Csapak
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox v2 1/1] pbs-api-types: add api types for " Dominik Csapak
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 1/6] backup: hierarchy: add new can_access_any_namespace_below helper Dominik Csapak
2025-10-08 20:57   ` [pbs-devel] applied: " Thomas Lamprecht
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 2/6] backup: hierarchy: reuse 'NS_PRIVS_OK' for namespace helper Dominik Csapak
2025-10-08 20:57   ` [pbs-devel] applied: " Thomas Lamprecht
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 3/6] api: admin: datastore: refactor BackupGroup to GroupListItem conversion Dominik Csapak
2025-10-08 20:57   ` [pbs-devel] applied: " Thomas Lamprecht
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 4/6] api: admin: datastore: factor out 'get_group_owner' Dominik Csapak
2025-10-08 20:57   ` [pbs-devel] applied: " Thomas Lamprecht
2025-10-08 13:43 ` Dominik Csapak [this message]
2025-10-08 20:57   ` [pbs-devel] applied: [PATCH proxmox-backup v2 5/6] api: admin: datastore: optimize `groups` api call Thomas Lamprecht
2025-10-08 13:43 ` [pbs-devel] [PATCH proxmox-backup v2 6/6] api: admin: datastore: implement streaming content " Dominik Csapak
2025-10-08 19:49   ` Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251008134344.3512958-7-d.csapak@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal