From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id DD7C869101 for ; Thu, 12 Nov 2020 11:31:13 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D4100D7D5 for ; Thu, 12 Nov 2020 11:30:43 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id C1387D7C0 for ; Thu, 12 Nov 2020 11:30:42 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 851D142155 for ; Thu, 12 Nov 2020 11:30:42 +0100 (CET) From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= To: pbs-devel@lists.proxmox.com Date: Thu, 12 Nov 2020 11:30:30 +0100 Message-Id: <20201112103034.159849-2-f.gruenbichler@proxmox.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20201112103034.159849-1-f.gruenbichler@proxmox.com> References: <20201112103034.159849-1-f.gruenbichler@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.023 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [datastore.rs] Subject: [pbs-devel] [PATCH proxmox-backup 1/5] improve group/snapshot listing 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: , X-List-Received-Date: Thu, 12 Nov 2020 10:31:13 -0000 by listing groups first, then filtering, then listing group snapshots. this cuts down the number of openat/getdirents calls for users that just have a partial view of the datastore. Signed-off-by: Fabian Grünbichler --- Notes: error behaviour should remain the same, unless I missed something.. src/api2/admin/datastore.rs | 200 ++++++++++++++++++++---------------- src/backup/backup_info.rs | 28 +++-- 2 files changed, 131 insertions(+), 97 deletions(-) diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs index e76867c7..df666156 100644 --- a/src/api2/admin/datastore.rs +++ b/src/api2/admin/datastore.rs @@ -1,4 +1,4 @@ -use std::collections::{HashSet, HashMap}; +use std::collections::HashSet; use std::ffi::OsStr; use std::os::unix::ffi::OsStrExt; use std::sync::{Arc, Mutex}; @@ -125,19 +125,6 @@ fn get_all_snapshot_files( Ok((manifest, files)) } -fn group_backups(backup_list: Vec) -> HashMap> { - - let mut group_hash = HashMap::new(); - - for info in backup_list { - let group_id = info.backup_dir.group().group_path().to_str().unwrap().to_owned(); - let time_list = group_hash.entry(group_id).or_insert(vec![]); - time_list.push(info); - } - - group_hash -} - #[api( input: { properties: { @@ -171,45 +158,61 @@ fn list_groups( let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); let datastore = DataStore::lookup_datastore(&store)?; + let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0; + + let backup_groups = BackupInfo::list_backup_groups(&datastore.base_path())?; + + let group_info = backup_groups + .into_iter() + .fold(Vec::new(), |mut group_info, group| { + let owner = match datastore.get_owner(&group) { + Ok(auth_id) => auth_id, + Err(err) => { + println!("Failed to get owner of group '{}' - {}", group, err); + return group_info; + }, + }; + if !list_all && check_backup_owner(&owner, &auth_id).is_err() { + return group_info; + } - let backup_list = BackupInfo::list_backups(&datastore.base_path())?; - - let group_hash = group_backups(backup_list); - - let mut groups = Vec::new(); - - for (_group_id, mut list) in group_hash { - - BackupInfo::sort_list(&mut list, false); - - let info = &list[0]; + let snapshots = match group.list_backups(&datastore.base_path()) { + Ok(snapshots) => snapshots, + Err(_) => { + return group_info; + }, + }; - let group = info.backup_dir.group(); + let backup_count: u64 = snapshots.len() as u64; + if backup_count == 0 { + return group_info; + } - let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0; - let owner = match datastore.get_owner(group) { - Ok(auth_id) => auth_id, - Err(err) => { - println!("Failed to get owner of group '{}' - {}", group, err); - continue; - }, - }; - if !list_all && check_backup_owner(&owner, &auth_id).is_err() { - continue; - } + let last_backup = snapshots + .iter() + .fold(&snapshots[0], |last, curr| { + if curr.is_finished() + && curr.backup_dir.backup_time() > last.backup_dir.backup_time() { + curr + } else { + last + } + }) + .to_owned(); + + group_info.push(GroupListItem { + backup_type: group.backup_type().to_string(), + backup_id: group.backup_id().to_string(), + last_backup: last_backup.backup_dir.backup_time(), + owner: Some(owner), + backup_count, + files: last_backup.files, + }); - let result_item = GroupListItem { - backup_type: group.backup_type().to_string(), - backup_id: group.backup_id().to_string(), - last_backup: info.backup_dir.backup_time(), - backup_count: list.len() as u64, - files: info.files.clone(), - owner: Some(owner), - }; - groups.push(result_item); - } + group_info + }); - Ok(groups) + Ok(group_info) } #[api( @@ -357,41 +360,38 @@ pub fn list_snapshots ( let user_info = CachedUserInfo::new()?; let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]); + let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0; + let datastore = DataStore::lookup_datastore(&store)?; let base_path = datastore.base_path(); - let backup_list = BackupInfo::list_backups(&base_path)?; - - let mut snapshots = vec![]; - - for info in backup_list { - let group = info.backup_dir.group(); - if let Some(ref backup_type) = backup_type { - if backup_type != group.backup_type() { continue; } - } - if let Some(ref backup_id) = backup_id { - if backup_id != group.backup_id() { continue; } - } - - let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0; - let owner = match datastore.get_owner(group) { - Ok(auth_id) => auth_id, - Err(err) => { - println!("Failed to get owner of group '{}' - {}", group, err); - continue; - }, - }; - - if !list_all && check_backup_owner(&owner, &auth_id).is_err() { - continue; - } + let groups = match (backup_type, backup_id) { + (Some(backup_type), Some(backup_id)) => { + let mut groups = Vec::with_capacity(1); + groups.push(BackupGroup::new(backup_type, backup_id)); + groups + }, + (Some(backup_type), None) => { + BackupInfo::list_backup_groups(&base_path)? + .into_iter() + .filter(|group| group.backup_type() == backup_type) + .collect() + }, + (None, Some(backup_id)) => { + BackupInfo::list_backup_groups(&base_path)? + .into_iter() + .filter(|group| group.backup_id() == backup_id) + .collect() + }, + _ => BackupInfo::list_backup_groups(&base_path)?, + }; - let mut size = None; + let info_to_snapshot_list_item = |group: &BackupGroup, owner, info: BackupInfo| { + let backup_time = info.backup_dir.backup_time(); - let (comment, verification, files) = match get_all_snapshot_files(&datastore, &info) { + let (comment, verification, files, size) = match get_all_snapshot_files(&datastore, &info) { Ok((manifest, files)) => { - size = Some(files.iter().map(|x| x.size.unwrap_or(0)).sum()); // extract the first line from notes let comment: Option = manifest.unprotected["notes"] .as_str() @@ -407,7 +407,9 @@ pub fn list_snapshots ( } }; - (comment, verify, files) + let size = Some(files.iter().map(|x| x.size.unwrap_or(0)).sum()); + + (comment, verify, files, size) }, Err(err) => { eprintln!("error during snapshot file listing: '{}'", err); @@ -416,32 +418,58 @@ pub fn list_snapshots ( None, info .files - .iter() + .into_iter() .map(|x| BackupContent { filename: x.to_string(), size: None, crypt_mode: None, }) - .collect() + .collect(), + None, ) }, }; - let result_item = SnapshotListItem { + SnapshotListItem { backup_type: group.backup_type().to_string(), backup_id: group.backup_id().to_string(), - backup_time: info.backup_dir.backup_time(), + backup_time, comment, verification, files, size, - owner: Some(owner), - }; + owner, + } + }; - snapshots.push(result_item); - } + groups + .iter() + .try_fold(Vec::new(), |mut snapshots, group| { + let owner = match datastore.get_owner(group) { + Ok(auth_id) => auth_id, + Err(err) => { + eprintln!("Failed to get owner of group '{}/{}' - {}", + &store, + group, + err); + return Ok(snapshots); + }, + }; + + if !list_all && check_backup_owner(&owner, &auth_id).is_err() { + return Ok(snapshots); + } + + let group_backups = group.list_backups(&datastore.base_path())?; + + snapshots.extend( + group_backups + .into_iter() + .map(|info| info_to_snapshot_list_item(&group, Some(owner.clone()), info)) + ); - Ok(snapshots) + Ok(snapshots) + }) } fn get_snapshots_count(store: &DataStore) -> Result { diff --git a/src/backup/backup_info.rs b/src/backup/backup_info.rs index 59849848..ebb5325b 100644 --- a/src/backup/backup_info.rs +++ b/src/backup/backup_info.rs @@ -327,29 +327,35 @@ impl BackupInfo { Ok(files) } - pub fn list_backups(base_path: &Path) -> Result, Error> { + pub fn list_backup_groups(base_path: &Path) -> Result, Error> { let mut list = Vec::new(); tools::scandir(libc::AT_FDCWD, base_path, &BACKUP_TYPE_REGEX, |l0_fd, backup_type, file_type| { if file_type != nix::dir::Type::Directory { return Ok(()); } - tools::scandir(l0_fd, backup_type, &BACKUP_ID_REGEX, |l1_fd, backup_id, file_type| { + tools::scandir(l0_fd, backup_type, &BACKUP_ID_REGEX, |_, backup_id, file_type| { if file_type != nix::dir::Type::Directory { return Ok(()); } - tools::scandir(l1_fd, backup_id, &BACKUP_DATE_REGEX, |l2_fd, backup_time_string, file_type| { - if file_type != nix::dir::Type::Directory { return Ok(()); } - - let backup_dir = BackupDir::with_rfc3339(backup_type, backup_id, backup_time_string)?; - - let files = list_backup_files(l2_fd, backup_time_string)?; - list.push(BackupInfo { backup_dir, files }); + list.push(BackupGroup::new(backup_type, backup_id)); - Ok(()) - }) + Ok(()) }) })?; + Ok(list) } + pub fn list_backups(base_path: &Path) -> Result, Error> { + let groups = BackupInfo::list_backup_groups(base_path)?; + + groups + .into_iter() + .try_fold(Vec::new(), |mut snapshots, group| { + let group_snapshots = group.list_backups(base_path)?; + snapshots.extend(group_snapshots); + Ok(snapshots) + }) + } + pub fn is_finished(&self) -> bool { // backup is considered unfinished if there is no manifest self.files.iter().any(|name| name == super::MANIFEST_BLOB_NAME) -- 2.20.1