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 4E40B1FF15C for ; Fri, 3 Oct 2025 12:18:42 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A3DFEA4A; Fri, 3 Oct 2025 12:18:42 +0200 (CEST) Message-ID: <5304f093-fb23-42ad-ab0c-bca839572d73@proxmox.com> Date: Fri, 3 Oct 2025 12:18:38 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Proxmox Backup Server development discussion , Dominik Csapak References: <20251003085045.1346864-1-d.csapak@proxmox.com> <20251003085045.1346864-7-d.csapak@proxmox.com> Content-Language: en-US From: Thomas Lamprecht In-Reply-To: <20251003085045.1346864-7-d.csapak@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1759486694289 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.026 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: Re: [pbs-devel] [PATCH proxmox-backup 5/6] api: admin: datastore: optimize `groups` api call 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" Am 03.10.25 um 10:50 schrieb Dominik Csapak: > 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 upgfront, use the snapshot s/upgfront/upfront/ > 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). nice! In the UI we mainly (only?) uses this for gathering the notes of a group, FWICT? > > 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) lazy question: Is this dependent on the previous patches? From a quick check, it looks like it might be rather independent and one could apply this one already. > Signed-off-by: Dominik Csapak > --- > src/api2/admin/datastore.rs | 45 +++++++++++++++++++++++-------------- > 1 file changed, 28 insertions(+), 17 deletions(-) > > diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs > index 0b133d166..2252dcfa4 100644 > --- a/src/api2/admin/datastore.rs > +++ b/src/api2/admin/datastore.rs > @@ -166,36 +166,47 @@ 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::, 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| match BackupInfo::new(backup.clone()) { > + Ok(info) => { > + if info.is_finished() { > + Some(info) > + } else { > + None > + } > + } > + Err(_) => None, nit: Could be written as a bit more concise way: snapshots .iter() .filter_map(|backup| BackupInfo::new(backup.clone()).ok()) .filter(|info| info.is_finished()) .next() With splitting the conversion to BackupInfo and filtering for finished backups into two makes it a bit easier (or rather quicker) to follow to me. > + }) > + .next() > + }; > > - 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, > }; > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel