* [pbs-devel] [PATCH proxmox-backup 1/4] remove BackupGroup::list_groups
2020-11-30 15:27 [pbs-devel] [PATCH proxmox-backup 0/4] pull/verify unified progress Fabian Grünbichler
@ 2020-11-30 15:27 ` Fabian Grünbichler
2020-11-30 15:27 ` [pbs-devel] [PATCH proxmox-backup 2/4] pull: factor out interpolated progress Fabian Grünbichler
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2020-11-30 15:27 UTC (permalink / raw)
To: pbs-devel
BackupInfo::list_backup_groups is identical code-wise, and makes more
sense as entry point for listing groups.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
we could of course remove the other one, or move this to datastore.rs altogether?
src/backup/backup_info.rs | 14 --------------
src/backup/verify.rs | 2 +-
src/client/pull.rs | 2 +-
src/server/prune_job.rs | 4 ++--
4 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/src/backup/backup_info.rs b/src/backup/backup_info.rs
index 367cb8ee..5ff1a6f8 100644
--- a/src/backup/backup_info.rs
+++ b/src/backup/backup_info.rs
@@ -145,20 +145,6 @@ impl BackupGroup {
Ok(last)
}
-
- pub fn list_groups(base_path: &Path) -> Result<Vec<BackupGroup>, 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| {
- if file_type != nix::dir::Type::Directory { return Ok(()); }
- list.push(BackupGroup::new(backup_type, backup_id));
- Ok(())
- })
- })?;
- Ok(list)
- }
}
impl std::fmt::Display for BackupGroup {
diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 1eccdd67..21a31a3f 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -533,7 +533,7 @@ pub fn verify_all_backups(
}
};
- let mut list = match BackupGroup::list_groups(&datastore.base_path()) {
+ let mut list = match BackupInfo::list_backup_groups(&datastore.base_path()) {
Ok(list) => list
.into_iter()
.filter(|group| !(group.backup_type() == "host" && group.backup_id() == "benchmark"))
diff --git a/src/client/pull.rs b/src/client/pull.rs
index 7d55f9fa..0c9afe0a 100644
--- a/src/client/pull.rs
+++ b/src/client/pull.rs
@@ -565,7 +565,7 @@ pub async fn pull_store(
if delete {
let result: Result<(), Error> = proxmox::try_block!({
- let local_groups = BackupGroup::list_groups(&tgt_store.base_path())?;
+ let local_groups = BackupInfo::list_backup_groups(&tgt_store.base_path())?;
for local_group in local_groups {
if new_groups.contains(&local_group) { continue; }
worker.log(format!("delete vanished group '{}/{}'", local_group.backup_type(), local_group.backup_id()));
diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
index 67d438d9..572f1b04 100644
--- a/src/server/prune_job.rs
+++ b/src/server/prune_job.rs
@@ -4,7 +4,7 @@ use proxmox::try_block;
use crate::{
api2::types::*,
- backup::{compute_prune_info, BackupGroup, DataStore, PruneOptions},
+ backup::{compute_prune_info, BackupInfo, DataStore, PruneOptions},
server::jobstate::Job,
server::WorkerTask,
task_log,
@@ -43,7 +43,7 @@ pub fn do_prune_job(
let base_path = datastore.base_path();
- let groups = BackupGroup::list_groups(&base_path)?;
+ let groups = BackupInfo::list_backup_groups(&base_path)?;
for group in groups {
let list = group.list_backups(&base_path)?;
let mut prune_info = compute_prune_info(list, &prune_options)?;
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [pbs-devel] [PATCH proxmox-backup 2/4] pull: factor out interpolated progress
2020-11-30 15:27 [pbs-devel] [PATCH proxmox-backup 0/4] pull/verify unified progress Fabian Grünbichler
2020-11-30 15:27 ` [pbs-devel] [PATCH proxmox-backup 1/4] remove BackupGroup::list_groups Fabian Grünbichler
@ 2020-11-30 15:27 ` Fabian Grünbichler
2020-12-01 5:21 ` Dietmar Maurer
2020-11-30 15:27 ` [pbs-devel] [PATCH proxmox-backup 3/4] progress: add format variants Fabian Grünbichler
` (2 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Fabian Grünbichler @ 2020-11-30 15:27 UTC (permalink / raw)
To: pbs-devel
and add group/snapshot count info.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Note: not 100% happy with struct and field naming here, very open for
better suggestions..
src/backup/datastore.rs | 47 +++++++++++++++++++++++++++++++++++++++++
src/client/pull.rs | 27 ++++++++++-------------
2 files changed, 58 insertions(+), 16 deletions(-)
diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
index 19efc23f..4f49d03f 100644
--- a/src/backup/datastore.rs
+++ b/src/backup/datastore.rs
@@ -739,3 +739,50 @@ impl DataStore {
self.verify_new
}
}
+
+#[derive(Debug, Clone, Default)]
+/// Tracker for progress of operations iterating over `Datastore` contents.
+pub struct StoreProgress {
+ /// Completed groups
+ pub done_groups: u64,
+ /// Total groups
+ pub total_groups: u64,
+ /// Completed snapshots within current group
+ pub done_snapshots: u64,
+ /// Total snapshots in current group
+ pub group_snapshots: u64,
+}
+
+impl StoreProgress {
+ pub fn new(total_groups: u64) -> Self {
+ StoreProgress {
+ total_groups,
+ .. Default::default()
+ }
+ }
+
+ /// Calculates an interpolated relative progress based on current counters.
+ pub fn percentage(&self) -> f64 {
+ let per_groups = (self.done_groups as f64) / (self.total_groups as f64);
+ if self.group_snapshots == 0 {
+ per_groups
+ } else {
+ let per_snapshots = (self.done_snapshots as f64) / (self.group_snapshots as f64);
+ per_groups + (1.0 / self.total_groups as f64) * per_snapshots
+ }
+ }
+}
+
+impl std::fmt::Display for StoreProgress {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ write!(
+ f,
+ "{:.2}% ({} of {} groups, {} of {} group snapshots)",
+ self.percentage() * 100.0,
+ self.done_groups,
+ self.total_groups,
+ self.done_snapshots,
+ self.group_snapshots,
+ )
+ }
+}
diff --git a/src/client/pull.rs b/src/client/pull.rs
index 0c9afe0a..2555a14c 100644
--- a/src/client/pull.rs
+++ b/src/client/pull.rs
@@ -395,7 +395,7 @@ pub async fn pull_group(
tgt_store: Arc<DataStore>,
group: &BackupGroup,
delete: bool,
- progress: Option<(usize, usize)>, // (groups_done, group_count)
+ progress: &mut StoreProgress,
) -> Result<(), Error> {
let path = format!("api2/json/admin/datastore/{}/snapshots", src_repo.store());
@@ -418,18 +418,10 @@ pub async fn pull_group(
let mut remote_snapshots = std::collections::HashSet::new();
- let (per_start, per_group) = if let Some((groups_done, group_count)) = progress {
- let per_start = (groups_done as f64)/(group_count as f64);
- let per_group = 1.0/(group_count as f64);
- (per_start, per_group)
- } else {
- (0.0, 1.0)
- };
-
// start with 16384 chunks (up to 65GB)
let downloaded_chunks = Arc::new(Mutex::new(HashSet::with_capacity(1024*64)));
- let snapshot_count = list.len();
+ progress.group_snapshots = list.len() as u64;
for (pos, item) in list.into_iter().enumerate() {
let snapshot = BackupDir::new(item.backup_type, item.backup_id, item.backup_time)?;
@@ -469,9 +461,8 @@ pub async fn pull_group(
let result = pull_snapshot_from(worker, reader, tgt_store.clone(), &snapshot, downloaded_chunks.clone()).await;
- let percentage = (pos as f64)/(snapshot_count as f64);
- let percentage = per_start + percentage*per_group;
- worker.log(format!("percentage done: {:.2}%", percentage*100.0));
+ progress.done_snapshots = pos as u64 + 1;
+ worker.log(format!("percentage done: {}", progress.clone()));
result?; // stop on error
}
@@ -523,9 +514,13 @@ pub async fn pull_store(
new_groups.insert(BackupGroup::new(&item.backup_type, &item.backup_id));
}
- let group_count = list.len();
+ let mut progress = StoreProgress::new(list.len() as u64);
+
+ for (done, item) in list.into_iter().enumerate() {
+ progress.done_groups = done as u64;
+ progress.done_snapshots = 0;
+ progress.group_snapshots = 0;
- for (groups_done, item) in list.into_iter().enumerate() {
let group = BackupGroup::new(&item.backup_type, &item.backup_id);
let (owner, _lock_guard) = match tgt_store.create_locked_backup_group(&group, &auth_id) {
@@ -551,7 +546,7 @@ pub async fn pull_store(
tgt_store.clone(),
&group,
delete,
- Some((groups_done, group_count)),
+ &mut progress,
).await {
worker.log(format!(
"sync group {}/{} failed - {}",
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [pbs-devel] [PATCH proxmox-backup 3/4] progress: add format variants
2020-11-30 15:27 [pbs-devel] [PATCH proxmox-backup 0/4] pull/verify unified progress Fabian Grünbichler
2020-11-30 15:27 ` [pbs-devel] [PATCH proxmox-backup 1/4] remove BackupGroup::list_groups Fabian Grünbichler
2020-11-30 15:27 ` [pbs-devel] [PATCH proxmox-backup 2/4] pull: factor out interpolated progress Fabian Grünbichler
@ 2020-11-30 15:27 ` Fabian Grünbichler
2020-11-30 15:27 ` [pbs-devel] [PATCH proxmox-backup 4/4] verify: use same progress as pull Fabian Grünbichler
2020-12-01 5:40 ` [pbs-devel] applied: [PATCH proxmox-backup 0/4] pull/verify unified progress Dietmar Maurer
4 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2020-11-30 15:27 UTC (permalink / raw)
To: pbs-devel
for iterating over a single group, or iterating just on the group level
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/backup/datastore.rs | 36 +++++++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
index 4f49d03f..20617d38 100644
--- a/src/backup/datastore.rs
+++ b/src/backup/datastore.rs
@@ -775,14 +775,32 @@ impl StoreProgress {
impl std::fmt::Display for StoreProgress {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
- write!(
- f,
- "{:.2}% ({} of {} groups, {} of {} group snapshots)",
- self.percentage() * 100.0,
- self.done_groups,
- self.total_groups,
- self.done_snapshots,
- self.group_snapshots,
- )
+ if self.group_snapshots == 0 {
+ write!(
+ f,
+ "{:.2}% ({} of {} groups)",
+ self.percentage() * 100.0,
+ self.done_groups,
+ self.total_groups,
+ )
+ } else if self.total_groups == 1 {
+ write!(
+ f,
+ "{:.2}% ({} of {} snapshots)",
+ self.percentage() * 100.0,
+ self.done_snapshots,
+ self.group_snapshots,
+ )
+ } else {
+ write!(
+ f,
+ "{:.2}% ({} of {} groups, {} of {} group snapshots)",
+ self.percentage() * 100.0,
+ self.done_groups,
+ self.total_groups,
+ self.done_snapshots,
+ self.group_snapshots,
+ )
+ }
}
}
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [pbs-devel] [PATCH proxmox-backup 4/4] verify: use same progress as pull
2020-11-30 15:27 [pbs-devel] [PATCH proxmox-backup 0/4] pull/verify unified progress Fabian Grünbichler
` (2 preceding siblings ...)
2020-11-30 15:27 ` [pbs-devel] [PATCH proxmox-backup 3/4] progress: add format variants Fabian Grünbichler
@ 2020-11-30 15:27 ` Fabian Grünbichler
2020-12-01 5:40 ` [pbs-devel] applied: [PATCH proxmox-backup 0/4] pull/verify unified progress Dietmar Maurer
4 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2020-11-30 15:27 UTC (permalink / raw)
To: pbs-devel
percentage of verified groups, interpolating based on snapshot count
within the group. in most cases, this will also be closer to 'real'
progress since added snapshots (those which will be verified) in active
backup groups will be roughly evenly distributed, while number of total
snapshots per group will be heavily skewed towards those groups which
have existed the longest, even though most of those old snapshots will
only be re-verified very infrequently.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/api2/admin/datastore.rs | 4 +--
src/backup/verify.rs | 59 ++++++++++++++++---------------------
2 files changed, 28 insertions(+), 35 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index bce58a78..df60dab6 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -687,12 +687,12 @@ pub fn verify(
}
res
} else if let Some(backup_group) = backup_group {
- let (_count, failed_dirs) = verify_backup_group(
+ let failed_dirs = verify_backup_group(
datastore,
&backup_group,
verified_chunks,
corrupt_chunks,
- None,
+ &mut StoreProgress::new(1),
worker.clone(),
worker.upid(),
None,
diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index 21a31a3f..7ba8d56a 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -10,6 +10,7 @@ use crate::{
api2::types::*,
backup::{
DataStore,
+ StoreProgress,
DataBlob,
BackupGroup,
BackupDir,
@@ -425,11 +426,11 @@ pub fn verify_backup_group(
group: &BackupGroup,
verified_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
corrupt_chunks: Arc<Mutex<HashSet<[u8;32]>>>,
- progress: Option<(usize, usize)>, // (done, snapshot_count)
+ progress: &mut StoreProgress,
worker: Arc<dyn TaskState + Send + Sync>,
upid: &UPID,
filter: Option<&dyn Fn(&BackupManifest) -> bool>,
-) -> Result<(usize, Vec<String>), Error> {
+) -> Result<Vec<String>, Error> {
let mut errors = Vec::new();
let mut list = match group.list_backups(&datastore.base_path()) {
@@ -442,19 +443,17 @@ pub fn verify_backup_group(
group,
err,
);
- return Ok((0, errors));
+ return Ok(errors);
}
};
- task_log!(worker, "verify group {}:{}", datastore.name(), group);
+ let snapshot_count = list.len();
+ task_log!(worker, "verify group {}:{} ({} snapshots)", datastore.name(), group, snapshot_count);
- let (done, snapshot_count) = progress.unwrap_or((0, list.len()));
+ progress.group_snapshots = snapshot_count as u64;
- let mut count = 0;
BackupInfo::sort_list(&mut list, false); // newest first
- for info in list {
- count += 1;
-
+ for (pos, info) in list.into_iter().enumerate() {
if !verify_backup_dir(
datastore.clone(),
&info.backup_dir,
@@ -466,20 +465,15 @@ pub fn verify_backup_group(
)? {
errors.push(info.backup_dir.to_string());
}
- if snapshot_count != 0 {
- let pos = done + count;
- let percentage = ((pos as f64) * 100.0)/(snapshot_count as f64);
- task_log!(
- worker,
- "percentage done: {:.2}% ({} of {} snapshots)",
- percentage,
- pos,
- snapshot_count,
- );
- }
+ progress.done_snapshots = pos as u64 + 1;
+ task_log!(
+ worker,
+ "percentage done: {}",
+ progress
+ );
}
- Ok((count, errors))
+ Ok(errors)
}
/// Verify all (owned) backups inside a datastore
@@ -551,34 +545,33 @@ pub fn verify_all_backups(
list.sort_unstable();
- let mut snapshot_count = 0;
- for group in list.iter() {
- snapshot_count += group.list_backups(&datastore.base_path())?.len();
- }
-
// start with 16384 chunks (up to 65GB)
let verified_chunks = Arc::new(Mutex::new(HashSet::with_capacity(1024*16)));
// start with 64 chunks since we assume there are few corrupt ones
let corrupt_chunks = Arc::new(Mutex::new(HashSet::with_capacity(64)));
- task_log!(worker, "found {} snapshots", snapshot_count);
+ let group_count = list.len();
+ task_log!(worker, "found {} groups", group_count);
- let mut done = 0;
- for group in list {
- let (count, mut group_errors) = verify_backup_group(
+ let mut progress = StoreProgress::new(group_count as u64);
+
+ for (pos, group) in list.into_iter().enumerate() {
+ progress.done_groups = pos as u64;
+ progress.done_snapshots = 0;
+ progress.group_snapshots = 0;
+
+ let mut group_errors = verify_backup_group(
datastore.clone(),
&group,
verified_chunks.clone(),
corrupt_chunks.clone(),
- Some((done, snapshot_count)),
+ &mut progress,
worker.clone(),
upid,
filter,
)?;
errors.append(&mut group_errors);
-
- done += count;
}
Ok(errors)
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [pbs-devel] applied: [PATCH proxmox-backup 0/4] pull/verify unified progress
2020-11-30 15:27 [pbs-devel] [PATCH proxmox-backup 0/4] pull/verify unified progress Fabian Grünbichler
` (3 preceding siblings ...)
2020-11-30 15:27 ` [pbs-devel] [PATCH proxmox-backup 4/4] verify: use same progress as pull Fabian Grünbichler
@ 2020-12-01 5:40 ` Dietmar Maurer
4 siblings, 0 replies; 7+ messages in thread
From: Dietmar Maurer @ 2020-12-01 5:40 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
applied, with some cleanups on top:
- removed unnecessary clone
- moved code into extra file: src/backup/store_progress.rs
> On 11/30/2020 4:27 PM Fabian Grünbichler <f.gruenbichler@proxmox.com> wrote:
>
>
> this series factors out and reuses the progress calculcation used by
> sync/pull for verification tasks, since the progress characteristics are
> rather similar:
> - we often skip big parts of groups (already synced/recently verified)
> - adding more snapshots within a group is usually fast (because of
> deduplication/chunk reuse)
>
> Fabian Grünbichler (4):
> remove BackupGroup::list_groups
> pull: factor out interpolated progress
> progress: add format variants
> verify: use same progress as pull
>
> src/api2/admin/datastore.rs | 4 +--
> src/backup/backup_info.rs | 14 --------
> src/backup/datastore.rs | 65 +++++++++++++++++++++++++++++++++++++
> src/backup/verify.rs | 61 +++++++++++++++-------------------
> src/client/pull.rs | 29 +++++++----------
> src/server/prune_job.rs | 4 +--
> 6 files changed, 108 insertions(+), 69 deletions(-)
>
> --
> 2.20.1
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread