From c714af9e675fe5762914c28a566e679a98a417ab Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Tue, 16 Feb 2021 09:41:47 +0200 Subject: refactor: split BackupProgress into initial, increemental variants This makes it possible to later have different progress bars for initial and incremental backup runs. However, for now the bars are identical. --- src/backup_progress.rs | 21 ++++++++++++++++++++- src/backup_run.rs | 2 +- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/backup_progress.rs b/src/backup_progress.rs index 6c1d3e6..d924ce1 100644 --- a/src/backup_progress.rs +++ b/src/backup_progress.rs @@ -6,7 +6,26 @@ pub struct BackupProgress { } impl BackupProgress { - pub fn new() -> Self { + pub fn initial() -> Self { + let progress = if true { + ProgressBar::new(0) + } else { + ProgressBar::hidden() + }; + let parts = vec![ + "{wide_bar}", + "elapsed: {elapsed}", + "files: {pos}/{len}", + "current: {wide_msg}", + "{spinner}", + ]; + progress.set_style(ProgressStyle::default_bar().template(&parts.join("\n"))); + progress.enable_steady_tick(100); + + Self { progress } + } + + pub fn incremental() -> Self { let progress = if true { ProgressBar::new(0) } else { diff --git a/src/backup_run.rs b/src/backup_run.rs index 7bb4440..91e06f0 100644 --- a/src/backup_run.rs +++ b/src/backup_run.rs @@ -34,7 +34,7 @@ impl BackupRun { pub fn new(config: &ClientConfig) -> BackupResult { let client = BackupClient::new(config)?; let policy = BackupPolicy::new(); - let progress = BackupProgress::new(); + let progress = BackupProgress::initial(); Ok(Self { client, policy, -- cgit v1.2.1 From 89c46016090b345b9acdea5398178d0c90aef4a7 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Tue, 16 Feb 2021 09:44:54 +0200 Subject: refactor: split BackupRun into initial, incremental variants This is clearer, easier to modify than having a flag to indicate which variant we're running. --- src/backup_run.rs | 46 +++++++++++++++++++++++++++++++++------------- src/cmd/backup.rs | 40 +++++++++++++++++++++++++--------------- 2 files changed, 58 insertions(+), 28 deletions(-) diff --git a/src/backup_run.rs b/src/backup_run.rs index 91e06f0..024f486 100644 --- a/src/backup_run.rs +++ b/src/backup_run.rs @@ -9,8 +9,14 @@ use crate::policy::BackupPolicy; use log::{info, warn}; use std::path::Path; -pub struct BackupRun { - client: BackupClient, +pub struct InitialBackup<'a> { + client: &'a BackupClient, + buffer_size: usize, + progress: BackupProgress, +} + +pub struct IncrementalBackup<'a> { + client: &'a BackupClient, policy: BackupPolicy, buffer_size: usize, progress: BackupProgress, @@ -30,28 +36,21 @@ pub enum BackupError { pub type BackupResult = Result; -impl BackupRun { - pub fn new(config: &ClientConfig) -> BackupResult { - let client = BackupClient::new(config)?; - let policy = BackupPolicy::new(); +impl<'a> InitialBackup<'a> { + pub fn new(config: &ClientConfig, client: &'a BackupClient) -> BackupResult { let progress = BackupProgress::initial(); Ok(Self { client, - policy, buffer_size: config.chunk_size, progress, }) } - pub fn client(&self) -> &BackupClient { - &self.client - } - pub fn progress(&self) -> &BackupProgress { &self.progress } - pub fn backup_file_initially( + pub fn backup( &self, entry: FsIterResult, ) -> BackupResult<(FilesystemEntry, Vec, Reason)> { @@ -65,8 +64,29 @@ impl BackupRun { } } } +} + +impl<'a> IncrementalBackup<'a> { + pub fn new(config: &ClientConfig, client: &'a BackupClient) -> BackupResult { + let policy = BackupPolicy::new(); + let progress = BackupProgress::incremental(); + Ok(Self { + client, + policy, + buffer_size: config.chunk_size, + progress, + }) + } + + pub fn client(&self) -> &BackupClient { + self.client + } + + pub fn progress(&self) -> &BackupProgress { + &self.progress + } - pub fn backup_file_incrementally( + pub fn backup( &self, entry: FsIterResult, old: &LocalGeneration, diff --git a/src/cmd/backup.rs b/src/cmd/backup.rs index cb2e9af..16d3f69 100644 --- a/src/cmd/backup.rs +++ b/src/cmd/backup.rs @@ -1,6 +1,6 @@ -use crate::backup_run::BackupRun; +use crate::backup_run::{IncrementalBackup, InitialBackup}; use crate::chunkid::ChunkId; -use crate::client::ClientConfig; +use crate::client::{BackupClient, ClientConfig}; use crate::error::ObnamError; use crate::fsiter::FsIterator; use crate::generation::NascentGeneration; @@ -14,8 +14,6 @@ const SQLITE_CHUNK_SIZE: usize = 1024 * 1024; pub fn backup(config: &ClientConfig) -> Result<(), ObnamError> { let runtime = SystemTime::now(); - let run = BackupRun::new(config)?; - // Create a named temporary file. We don't meed the open file // handle, so we discard that. let oldname = { @@ -32,18 +30,26 @@ pub fn backup(config: &ClientConfig) -> Result<(), ObnamError> { dbname }; - let genlist = run.client().list_generations()?; + let client = BackupClient::new(config)?; + let genlist = client.list_generations()?; let file_count = match genlist.resolve("latest") { - Err(_) => initial_backup(&config.roots, &newname, &run)?, - Ok(old) => incremental_backup(&old, &config.roots, &newname, &oldname, &run)?, + Err(_) => { + let run = InitialBackup::new(config, &client)?; + let count = initial_backup(&config.roots, &newname, &run)?; + run.progress().finish(); + count + } + Ok(old) => { + let run = IncrementalBackup::new(config, &client)?; + let count = incremental_backup(&old, &config.roots, &newname, &oldname, &run)?; + run.progress().finish(); + count + } }; - run.progress().finish(); // Upload the SQLite file, i.e., the named temporary file, which // still exists, since we persisted it above. - let gen_id = run - .client() - .upload_generation(&newname, SQLITE_CHUNK_SIZE)?; + let gen_id = client.upload_generation(&newname, SQLITE_CHUNK_SIZE)?; // Delete the temporary file.q std::fs::remove_file(&newname)?; @@ -62,13 +68,17 @@ fn report_stats(runtime: &SystemTime, file_count: i64, gen_id: &ChunkId) -> Resu Ok(()) } -fn initial_backup(roots: &[PathBuf], newname: &Path, run: &BackupRun) -> Result { +fn initial_backup( + roots: &[PathBuf], + newname: &Path, + run: &InitialBackup, +) -> Result { info!("fresh backup without a previous generation"); let mut new = NascentGeneration::create(&newname)?; for root in roots { let iter = FsIterator::new(root); - new.insert_iter(iter.map(|entry| run.backup_file_initially(entry)))?; + new.insert_iter(iter.map(|entry| run.backup(entry)))?; } Ok(new.file_count()) } @@ -78,7 +88,7 @@ fn incremental_backup( roots: &[PathBuf], newname: &Path, oldname: &Path, - run: &BackupRun, + run: &IncrementalBackup, ) -> Result { info!("incremental backup based on {}", old); @@ -88,7 +98,7 @@ fn incremental_backup( let iter = FsIterator::new(root); run.progress() .files_in_previous_generation(old.file_count()? as u64); - new.insert_iter(iter.map(|entry| run.backup_file_incrementally(entry, &old)))?; + new.insert_iter(iter.map(|entry| run.backup(entry, &old)))?; } Ok(new.file_count()) } -- cgit v1.2.1 From eb8000d325a1798cc30484a863bb1a76ee90d003 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Tue, 16 Feb 2021 10:52:53 +0200 Subject: fix: drop initial backup progress bar It didn't actually show any progress and so was useless. --- src/backup_progress.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/backup_progress.rs b/src/backup_progress.rs index d924ce1..c9c2955 100644 --- a/src/backup_progress.rs +++ b/src/backup_progress.rs @@ -13,7 +13,6 @@ impl BackupProgress { ProgressBar::hidden() }; let parts = vec![ - "{wide_bar}", "elapsed: {elapsed}", "files: {pos}/{len}", "current: {wide_msg}", -- cgit v1.2.1 From 9bc9ef697d786c48fada5062c1db4792e0c5e1f2 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Tue, 16 Feb 2021 11:10:46 +0200 Subject: feat: change progress bars to show which kind of backup is happening --- src/backup_progress.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/backup_progress.rs b/src/backup_progress.rs index c9c2955..fac56fb 100644 --- a/src/backup_progress.rs +++ b/src/backup_progress.rs @@ -13,6 +13,7 @@ impl BackupProgress { ProgressBar::hidden() }; let parts = vec![ + "initial backup", "elapsed: {elapsed}", "files: {pos}/{len}", "current: {wide_msg}", @@ -31,6 +32,7 @@ impl BackupProgress { ProgressBar::hidden() }; let parts = vec![ + "incremental backup", "{wide_bar}", "elapsed: {elapsed}", "files: {pos}/{len}", -- cgit v1.2.1 From f8161b8551f982af61eb4385734adbef5642113e Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Tue, 16 Feb 2021 11:17:11 +0200 Subject: fix: initial backup progress bar only shows total backup count Previously it showed "N/M", where N was the number of files found in the current backup run, and M was the number of files found in the previous generation. But that made no sense for an initial backup, so now we only show N. --- src/backup_progress.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backup_progress.rs b/src/backup_progress.rs index fac56fb..b5ecc26 100644 --- a/src/backup_progress.rs +++ b/src/backup_progress.rs @@ -15,7 +15,7 @@ impl BackupProgress { let parts = vec![ "initial backup", "elapsed: {elapsed}", - "files: {pos}/{len}", + "files: {pos}", "current: {wide_msg}", "{spinner}", ]; -- cgit v1.2.1 From e6fe102b49014551f955e72ca0af2b0435a26d40 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Tue, 16 Feb 2021 11:18:49 +0200 Subject: fix: in an incremental backup, update total file count if needed Previously, we were showing "N/M", where N is the number of files found in the current backup run, and M the number of files in the previous backup. I found this confusing if the new run finds more files than were in the previous run. Now we increment M if N goes beyond it. --- src/backup_progress.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/backup_progress.rs b/src/backup_progress.rs index b5ecc26..c6d16d9 100644 --- a/src/backup_progress.rs +++ b/src/backup_progress.rs @@ -55,6 +55,9 @@ impl BackupProgress { pub fn found_live_file(&self, filename: &Path) { self.progress.inc(1); + if self.progress.length() < self.progress.position() { + self.progress.set_length(self.progress.position()); + } self.progress .set_message(&format!("{}", filename.display())); } -- cgit v1.2.1 From 8389799fb2d9aca35c46419244b6ef60f855c40f Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Tue, 16 Feb 2021 11:02:59 +0200 Subject: config change for testing --- client.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client.yaml b/client.yaml index a1a63e7..4386b90 100644 --- a/client.yaml +++ b/client.yaml @@ -1,5 +1,5 @@ server_url: https://localhost:8888 verify_tls_cert: false roots: - - x + - /home/liw/pers/debian-ansible log: obnam.log -- cgit v1.2.1