From 8a42fc1c9dc3da6936fe8e1d0d16fbe8a60ec520 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Tue, 5 Jan 2021 12:54:53 +0200 Subject: refactor: move policy on whether to back up a file into a module This commit also splits up the src/cmd/backup.rs module into other, smaller, more cohesive modules that are easier to understand and use. --- src/backup_progress.rs | 45 ++++++++++++++ src/backup_reason.rs | 34 +++++++++++ src/backup_run.rs | 92 ++++++++++++++++++++++++++++ src/client.rs | 6 +- src/cmd/backup.rs | 163 ++++--------------------------------------------- src/cmd/mod.rs | 2 +- src/generation.rs | 4 +- src/lib.rs | 4 ++ src/policy.rs | 64 +++++++++++++++++++ 9 files changed, 258 insertions(+), 156 deletions(-) create mode 100644 src/backup_progress.rs create mode 100644 src/backup_reason.rs create mode 100644 src/backup_run.rs create mode 100644 src/policy.rs diff --git a/src/backup_progress.rs b/src/backup_progress.rs new file mode 100644 index 0000000..d65f104 --- /dev/null +++ b/src/backup_progress.rs @@ -0,0 +1,45 @@ +use indicatif::{ProgressBar, ProgressStyle}; +use std::path::Path; + +pub struct BackupProgress { + progress: ProgressBar, +} + +impl BackupProgress { + pub fn new() -> 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 files_in_previous_generation(&self, count: u64) { + self.progress.set_length(count); + } + + pub fn found_problem(&self) { + self.progress.inc(1); + } + + pub fn found_live_file(&self, filename: &Path) { + self.progress.inc(1); + self.progress + .set_message(&format!("{}", filename.display())); + } + + pub fn finish(&self) { + self.progress.set_length(self.progress.position()); + } +} diff --git a/src/backup_reason.rs b/src/backup_reason.rs new file mode 100644 index 0000000..cc0d49a --- /dev/null +++ b/src/backup_reason.rs @@ -0,0 +1,34 @@ +use rusqlite::types::ToSqlOutput; +use rusqlite::ToSql; +use std::fmt; + +#[derive(Debug)] +pub enum Reason { + Skipped, + IsNew, + Changed, + Unchanged, + Error, +} + +impl ToSql for Reason { + fn to_sql(&self) -> rusqlite::Result { + Ok(ToSqlOutput::Owned(rusqlite::types::Value::Text(format!( + "{}", + self + )))) + } +} + +impl fmt::Display for Reason { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let reason = match self { + Reason::Skipped => "skipped", + Reason::IsNew => "new", + Reason::Changed => "changed", + Reason::Unchanged => "unchanged", + Reason::Error => "error", + }; + write!(f, "{}", reason) + } +} diff --git a/src/backup_run.rs b/src/backup_run.rs new file mode 100644 index 0000000..e3bfc5a --- /dev/null +++ b/src/backup_run.rs @@ -0,0 +1,92 @@ +use crate::backup_progress::BackupProgress; +use crate::backup_reason::Reason; +use crate::chunkid::ChunkId; +use crate::client::{BackupClient, ClientConfig}; +use crate::fsentry::FilesystemEntry; +use crate::generation::LocalGeneration; +use crate::policy::BackupPolicy; +use log::{info, warn}; + +pub struct BackupRun { + client: BackupClient, + policy: BackupPolicy, + buffer_size: usize, + progress: BackupProgress, +} + +impl BackupRun { + pub fn new(config: &ClientConfig, buffer_size: usize) -> anyhow::Result { + let client = BackupClient::new(&config.server_url)?; + let policy = BackupPolicy::new(); + let progress = BackupProgress::new(); + Ok(Self { + client, + policy, + buffer_size, + progress, + }) + } + + pub fn client(&self) -> &BackupClient { + &self.client + } + + pub fn progress(&self) -> &BackupProgress { + &self.progress + } + + pub fn backup_file_initially( + &self, + entry: anyhow::Result, + ) -> anyhow::Result<(FilesystemEntry, Vec, Reason)> { + match entry { + Err(err) => Err(err.into()), + Ok(entry) => { + let path = &entry.pathbuf(); + info!("backup: {}", path.display()); + self.progress.found_live_file(path); + let ids = self + .client + .upload_filesystem_entry(&entry, self.buffer_size)?; + Ok((entry.clone(), ids, Reason::IsNew)) + } + } + } + + pub fn backup_file_incrementally( + &self, + entry: anyhow::Result, + old: &LocalGeneration, + ) -> anyhow::Result<(FilesystemEntry, Vec, Reason)> { + match entry { + Err(err) => { + warn!("backup: {}", err); + self.progress.found_problem(); + Err(err) + } + Ok(entry) => { + let path = &entry.pathbuf(); + info!("backup: {}", path.display()); + self.progress.found_live_file(path); + let reason = self.policy.needs_backup(&old, &entry); + match reason { + Reason::IsNew | Reason::Changed | Reason::Error => { + let ids = self + .client + .upload_filesystem_entry(&entry, self.buffer_size)?; + Ok((entry.clone(), ids, reason)) + } + Reason::Unchanged | Reason::Skipped => { + let fileno = old.get_fileno(&entry.pathbuf())?; + let ids = if let Some(fileno) = fileno { + old.chunkids(fileno)? + } else { + vec![] + }; + Ok((entry.clone(), ids, reason)) + } + } + } + } + } +} diff --git a/src/client.rs b/src/client.rs index 1b507d3..d075f43 100644 --- a/src/client.rs +++ b/src/client.rs @@ -64,16 +64,16 @@ impl BackupClient { pub fn upload_filesystem_entry( &self, - e: FilesystemEntry, + e: &FilesystemEntry, size: usize, - ) -> anyhow::Result<(FilesystemEntry, Vec)> { + ) -> anyhow::Result> { debug!("entry: {:?}", e); let ids = match e.kind() { FilesystemKind::Regular => self.read_file(e.pathbuf(), size)?, FilesystemKind::Directory => vec![], FilesystemKind::Symlink => vec![], }; - Ok((e, ids)) + Ok(ids) } pub fn upload_generation(&self, filename: &Path, size: usize) -> anyhow::Result { diff --git a/src/cmd/backup.rs b/src/cmd/backup.rs index 1521cab..398a8aa 100644 --- a/src/cmd/backup.rs +++ b/src/cmd/backup.rs @@ -1,16 +1,12 @@ -use crate::client::{BackupClient, ClientConfig}; -use crate::fsentry::FilesystemEntry; +use crate::backup_run::BackupRun; +use crate::client::ClientConfig; use crate::fsiter::FsIterator; -use crate::generation::{LocalGeneration, NascentGeneration}; -use indicatif::{ProgressBar, ProgressStyle}; -use log::{debug, info}; -use rusqlite::types::ToSqlOutput; -use rusqlite::ToSql; -use std::fmt; +use crate::generation::NascentGeneration; +use log::info; use tempfile::NamedTempFile; pub fn backup(config: &ClientConfig, buffer_size: usize) -> anyhow::Result<()> { - let client = BackupClient::new(&config.server_url)?; + let run = BackupRun::new(config, buffer_size)?; // Create a named temporary file. We don't meed the open file // handle, so we discard that. @@ -28,72 +24,30 @@ pub fn backup(config: &ClientConfig, buffer_size: usize) -> anyhow::Result<()> { dbname }; - let genlist = client.list_generations()?; + let genlist = run.client().list_generations()?; { let iter = FsIterator::new(&config.root); let mut new = NascentGeneration::create(&newname)?; - let progress = create_progress_bar(true); - progress.enable_steady_tick(100); match genlist.resolve("latest") { None => { info!("fresh backup without a previous generation"); - new.insert_iter(iter.map(|entry| { - progress.inc(1); - match entry { - Err(err) => Err(err), - Ok(entry) => { - let path = &entry.pathbuf(); - info!("backup: {}", path.display()); - progress.set_message(&format!("{}", path.display())); - let (new_entry, ids) = - client.upload_filesystem_entry(entry, buffer_size)?; - Ok((new_entry, ids, Reason::IsNew)) - } - } - }))?; + new.insert_iter(iter.map(|entry| run.backup_file_initially(entry)))?; } Some(old) => { info!("incremental backup based on {}", old); - let old = client.fetch_generation(&old, &oldname)?; - progress.set_length(old.file_count()? as u64); - new.insert_iter(iter.map(|entry| { - progress.inc(1); - match entry { - Err(err) => Err(err), - Ok(entry) => { - let path = &entry.pathbuf(); - info!("backup: {}", path.display()); - progress.set_message(&format!("{}", path.display())); - let reason = needs_backup(&old, &entry); - match reason { - Reason::IsNew | Reason::Changed | Reason::Error => { - let (new_entry, ids) = - client.upload_filesystem_entry(entry, buffer_size)?; - Ok((new_entry, ids, reason)) - } - Reason::Unchanged => { - let fileno = old.get_fileno(&entry.pathbuf())?; - let ids = if let Some(fileno) = fileno { - old.chunkids(fileno)? - } else { - vec![] - }; - Ok((entry.clone(), ids, Reason::Unchanged)) - } - } - } - } - }))?; + let old = run.client().fetch_generation(&old, &oldname)?; + run.progress() + .files_in_previous_generation(old.file_count()? as u64); + new.insert_iter(iter.map(|entry| run.backup_file_incrementally(entry, &old)))?; } } - progress.set_length(new.file_count() as u64); - progress.finish(); + run.progress().finish(); } // Upload the SQLite file, i.e., the named temporary file, which // still exists, since we persisted it above. - let gen_id = client.upload_generation(&newname, buffer_size)?; + let gen_id = run.client().upload_generation(&newname, buffer_size)?; println!("gen id: {}", gen_id); // Delete the temporary file.q @@ -102,94 +56,3 @@ pub fn backup(config: &ClientConfig, buffer_size: usize) -> anyhow::Result<()> { Ok(()) } - -#[derive(Debug)] -pub enum Reason { - IsNew, - Changed, - Unchanged, - Error, -} - -impl ToSql for Reason { - fn to_sql(&self) -> rusqlite::Result { - Ok(ToSqlOutput::Owned(rusqlite::types::Value::Text(format!( - "{}", - self - )))) - } -} - -impl fmt::Display for Reason { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let reason = match self { - Reason::IsNew => "new", - Reason::Changed => "changed", - Reason::Unchanged => "unchanged", - Reason::Error => "error", - }; - write!(f, "{}", reason) - } -} - -fn create_progress_bar(verbose: bool) -> ProgressBar { - let progress = if verbose { - 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 -} - -fn needs_backup(old: &LocalGeneration, new_entry: &FilesystemEntry) -> Reason { - let new_name = new_entry.pathbuf(); - match old.get_file(&new_name) { - // File is not in old generation. - Ok(None) => { - debug!( - "needs_backup: file is not in old generation, needs backup: {:?}", - new_name - ); - Reason::IsNew - } - - // File is in old generation. Has its metadata changed? - Ok(Some(old_entry)) => { - if file_has_changed(&old_entry, new_entry) { - debug!("needs_backup: file has changed: {:?}", new_name); - Reason::Changed - } else { - debug!("needs_backup: file has NOT changed: {:?}", new_name); - Reason::Unchanged - } - } - - // There was an error, which we ignore, but we indicate the - // file needs to be backed up now. - Err(err) => { - debug!( - "needs_backup: lookup in old generation returned error, ignored: {:?}: {}", - new_name, err - ); - Reason::Error - } - } -} - -fn file_has_changed(old: &FilesystemEntry, new: &FilesystemEntry) -> bool { - let unchanged = old.kind() == new.kind() - && old.len() == new.len() - && old.mode() == new.mode() - && old.mtime() == new.mtime() - && old.mtime_ns() == new.mtime_ns() - && old.symlink_target() == new.symlink_target(); - !unchanged -} diff --git a/src/cmd/mod.rs b/src/cmd/mod.rs index c3cebe2..0a22a8a 100644 --- a/src/cmd/mod.rs +++ b/src/cmd/mod.rs @@ -1,5 +1,5 @@ mod backup; -pub use backup::{backup, Reason}; +pub use backup::backup; mod list; pub use list::list; diff --git a/src/generation.rs b/src/generation.rs index d445a00..5bf749f 100644 --- a/src/generation.rs +++ b/src/generation.rs @@ -1,5 +1,5 @@ +use crate::backup_reason::Reason; use crate::chunkid::ChunkId; -use crate::cmd::Reason; use crate::fsentry::FilesystemEntry; use rusqlite::Connection; use std::path::Path; @@ -142,8 +142,8 @@ impl LocalGeneration { mod sql { use super::FileId; + use crate::backup_reason::Reason; use crate::chunkid::ChunkId; - use crate::cmd::Reason; use crate::error::ObnamError; use crate::fsentry::FilesystemEntry; use rusqlite::{params, Connection, OpenFlags, Row, Transaction}; diff --git a/src/lib.rs b/src/lib.rs index a06d396..a12b8a3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,6 @@ +pub mod backup_progress; +pub mod backup_reason; +pub mod backup_run; pub mod benchmark; pub mod checksummer; pub mod chunk; @@ -13,5 +16,6 @@ pub mod generation; pub mod genlist; pub mod index; pub mod indexedstore; +pub mod policy; pub mod server; pub mod store; diff --git a/src/policy.rs b/src/policy.rs new file mode 100644 index 0000000..032b851 --- /dev/null +++ b/src/policy.rs @@ -0,0 +1,64 @@ +use crate::backup_reason::Reason; +use crate::fsentry::FilesystemEntry; +use crate::generation::LocalGeneration; +use log::{debug, warn}; + +pub struct BackupPolicy { + new: bool, + old_if_changed: bool, +} + +impl BackupPolicy { + pub fn new() -> Self { + Self { + new: true, + old_if_changed: true, + } + } + + pub fn needs_backup(&self, old: &LocalGeneration, new_entry: &FilesystemEntry) -> Reason { + let new_name = new_entry.pathbuf(); + let reason = match old.get_file(&new_name) { + Ok(None) => { + if self.new { + Reason::IsNew + } else { + Reason::Skipped + } + } + Ok(Some(old_entry)) => { + if self.old_if_changed { + if file_has_changed(&old_entry, new_entry) { + Reason::Changed + } else { + Reason::Unchanged + } + } else { + Reason::Skipped + } + } + Err(err) => { + warn!( + "needs_backup: lookup in old generation returned error, ignored: {:?}: {}", + new_name, err + ); + Reason::Error + } + }; + debug!( + "needs_backup: file {:?}: policy decision: {}", + new_name, reason + ); + reason + } +} + +fn file_has_changed(old: &FilesystemEntry, new: &FilesystemEntry) -> bool { + let unchanged = old.kind() == new.kind() + && old.len() == new.len() + && old.mode() == new.mode() + && old.mtime() == new.mtime() + && old.mtime_ns() == new.mtime_ns() + && old.symlink_target() == new.symlink_target(); + !unchanged +} -- cgit v1.2.1