* [pbs-devel] [RFC PATCH proxmox-backup 1/2] tape: media_catalog: commit as much as possible
@ 2021-07-20 9:56 Dominik Csapak
2021-07-20 9:56 ` [pbs-devel] [RFC PATCH promxox-backup 2/2] api2: tape/backup: commit pool_writer even on error Dominik Csapak
0 siblings, 1 reply; 3+ messages in thread
From: Dominik Csapak @ 2021-07-20 9:56 UTC (permalink / raw)
To: pbs-devel
record when we finish an archive in the catalogs pending data,
and commit data until there. This way we can commit the catalog
even if we do not finish a chunk archive.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
sending as RFC, since i am not completely sure if i thought this
through completely, but in my tests it worked..
src/tape/media_catalog.rs | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/src/tape/media_catalog.rs b/src/tape/media_catalog.rs
index 65b52a42..8dacf986 100644
--- a/src/tape/media_catalog.rs
+++ b/src/tape/media_catalog.rs
@@ -66,6 +66,7 @@ pub struct MediaCatalog {
content: HashMap<String, DatastoreContent>,
+ finished_archive_len: usize,
pending: Vec<u8>,
}
@@ -223,6 +224,7 @@ impl MediaCatalog {
current_archive: None,
last_entry: None,
content: HashMap::new(),
+ finished_archive_len: 0,
pending: Vec::new(),
};
@@ -231,6 +233,7 @@ impl MediaCatalog {
if !found_magic_number {
me.pending.extend(&Self::PROXMOX_BACKUP_MEDIA_CATALOG_MAGIC_1_1);
}
+ me.finished_archive_len = me.pending.len();
if write {
me.file = Some(file);
@@ -300,12 +303,15 @@ impl MediaCatalog {
current_archive: None,
last_entry: None,
content: HashMap::new(),
+ finished_archive_len: 0,
pending: Vec::new(),
};
me.log_to_stdout = log_to_stdout;
me.pending.extend(&Self::PROXMOX_BACKUP_MEDIA_CATALOG_MAGIC_1_1);
+ me.finished_archive_len = me.pending.len();
+
me.register_label(&media_id.label.uuid, 0, 0)?;
@@ -366,31 +372,34 @@ impl MediaCatalog {
///
/// Fixme: this should be atomic ...
pub fn commit(&mut self) -> Result<(), Error> {
-
- if self.pending.is_empty() {
+ if self.finished_archive_len == 0 {
return Ok(());
}
match self.file {
Some(ref mut file) => {
- file.write_all(&self.pending)?;
+ file.write_all(&self.pending[0..self.finished_archive_len])?;
file.flush()?;
file.sync_data()?;
}
None => bail!("media catalog not writable (opened read only)"),
}
- self.pending = Vec::new();
+ let pending = self.pending.len();
+ if self.finished_archive_len < pending {
+ self.pending
+ .copy_within(self.finished_archive_len..pending, 0);
+ }
+ self.pending
+ .truncate(self.pending.len() - self.finished_archive_len);
+ self.finished_archive_len = 0;
Ok(())
}
- /// Conditionally commit if in pending data is large (> 1Mb)
+ /// Conditionally commit if finished archives in pending data is large (> 1MiB)
pub fn commit_if_large(&mut self) -> Result<(), Error> {
- if self.current_archive.is_some() {
- bail!("can't commit catalog in the middle of an chunk archive");
- }
- if self.pending.len() > 1024*1024 {
+ if self.finished_archive_len > 1024*1024 {
self.commit()?;
}
Ok(())
@@ -498,6 +507,7 @@ impl MediaCatalog {
unsafe { self.pending.write_le_value(entry)?; }
+ self.finished_archive_len = self.pending.len();
self.last_entry = Some((uuid.clone(), file_number));
Ok(())
@@ -625,6 +635,7 @@ impl MediaCatalog {
unsafe { self.pending.write_le_value(entry)?; }
+ self.finished_archive_len = self.pending.len();
self.last_entry = Some((uuid, file_number));
}
}
@@ -688,6 +699,8 @@ impl MediaCatalog {
self.pending.push(b':');
self.pending.extend(snapshot.as_bytes());
+ self.finished_archive_len = self.pending.len();
+
let content = self.content.entry(store.to_string())
.or_insert(DatastoreContent::new());
--
2.30.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* [pbs-devel] [RFC PATCH promxox-backup 2/2] api2: tape/backup: commit pool_writer even on error
2021-07-20 9:56 [pbs-devel] [RFC PATCH proxmox-backup 1/2] tape: media_catalog: commit as much as possible Dominik Csapak
@ 2021-07-20 9:56 ` Dominik Csapak
0 siblings, 0 replies; 3+ messages in thread
From: Dominik Csapak @ 2021-07-20 9:56 UTC (permalink / raw)
To: pbs-devel
this way we store all finished snapshots/chunk archives we in the
catalog, and not onlye those until the last commit
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/api2/tape/backup.rs | 115 +++++++++++++++++++++-------------------
1 file changed, 60 insertions(+), 55 deletions(-)
diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index 8119482f..82cdd428 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -456,72 +456,77 @@ fn backup_worker(
let mut need_catalog = false; // avoid writing catalog for empty jobs
- for (group_number, group) in group_list.into_iter().enumerate() {
- progress.done_groups = group_number as u64;
- progress.done_snapshots = 0;
- progress.group_snapshots = 0;
-
- let snapshot_list = group.list_backups(&datastore.base_path())?;
-
- // filter out unfinished backups
- let mut snapshot_list = snapshot_list
- .into_iter()
- .filter(|item| item.is_finished())
- .collect();
-
- BackupInfo::sort_list(&mut snapshot_list, true); // oldest first
-
- if latest_only {
- progress.group_snapshots = 1;
- if let Some(info) = snapshot_list.pop() {
- if pool_writer.contains_snapshot(datastore_name, &info.backup_dir.to_string()) {
- task_log!(worker, "skip snapshot {}", info.backup_dir);
- continue;
- }
+ let res: Result<(), Error> = proxmox::try_block!({
+ for (group_number, group) in group_list.into_iter().enumerate() {
+ progress.done_groups = group_number as u64;
+ progress.done_snapshots = 0;
+ progress.group_snapshots = 0;
+
+ let snapshot_list = group.list_backups(&datastore.base_path())?;
+
+ // filter out unfinished backups
+ let mut snapshot_list = snapshot_list
+ .into_iter()
+ .filter(|item| item.is_finished())
+ .collect();
+
+ BackupInfo::sort_list(&mut snapshot_list, true); // oldest first
+
+ if latest_only {
+ progress.group_snapshots = 1;
+ if let Some(info) = snapshot_list.pop() {
+ if pool_writer.contains_snapshot(datastore_name, &info.backup_dir.to_string()) {
+ task_log!(worker, "skip snapshot {}", info.backup_dir);
+ continue;
+ }
- need_catalog = true;
+ need_catalog = true;
- let snapshot_name = info.backup_dir.to_string();
- if !backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)? {
- errors = true;
- } else {
- summary.snapshot_list.push(snapshot_name);
- }
- progress.done_snapshots = 1;
- task_log!(
- worker,
- "percentage done: {}",
- progress
- );
- }
- } else {
- progress.group_snapshots = snapshot_list.len() as u64;
- for (snapshot_number, info) in snapshot_list.into_iter().enumerate() {
- if pool_writer.contains_snapshot(datastore_name, &info.backup_dir.to_string()) {
- task_log!(worker, "skip snapshot {}", info.backup_dir);
- continue;
+ let snapshot_name = info.backup_dir.to_string();
+ if !backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)? {
+ errors = true;
+ } else {
+ summary.snapshot_list.push(snapshot_name);
+ }
+ progress.done_snapshots = 1;
+ task_log!(
+ worker,
+ "percentage done: {}",
+ progress
+ );
}
+ } else {
+ progress.group_snapshots = snapshot_list.len() as u64;
+ for (snapshot_number, info) in snapshot_list.into_iter().enumerate() {
+ if pool_writer.contains_snapshot(datastore_name, &info.backup_dir.to_string()) {
+ task_log!(worker, "skip snapshot {}", info.backup_dir);
+ continue;
+ }
- need_catalog = true;
+ need_catalog = true;
- let snapshot_name = info.backup_dir.to_string();
- if !backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)? {
- errors = true;
- } else {
- summary.snapshot_list.push(snapshot_name);
+ let snapshot_name = info.backup_dir.to_string();
+ if !backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)? {
+ errors = true;
+ } else {
+ summary.snapshot_list.push(snapshot_name);
+ }
+ progress.done_snapshots = snapshot_number as u64 + 1;
+ task_log!(
+ worker,
+ "percentage done: {}",
+ progress
+ );
}
- progress.done_snapshots = snapshot_number as u64 + 1;
- task_log!(
- worker,
- "percentage done: {}",
- progress
- );
}
}
- }
+ Ok(())
+ });
pool_writer.commit()?;
+ let _ = res?; // bubble errors up
+
if need_catalog {
task_log!(worker, "append media catalog");
--
2.30.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pbs-devel] [RFC PATCH proxmox-backup 1/2] tape: media_catalog: commit as much as possible
@ 2021-07-21 11:18 Dietmar Maurer
0 siblings, 0 replies; 3+ messages in thread
From: Dietmar Maurer @ 2021-07-21 11:18 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
this looks clumsy.
Instead, can we make catalog.start_chunk_archive and catalog.end_chunk_archive() private,
and add a new method to register whole archives (catalog.register_chunk_archive())
That way all pending changes are valid and can be commited?
> On 07/20/2021 11:56 AM Dominik Csapak <d.csapak@proxmox.com> wrote:
>
>
> record when we finish an archive in the catalogs pending data,
> and commit data until there. This way we can commit the catalog
> even if we do not finish a chunk archive.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> sending as RFC, since i am not completely sure if i thought this
> through completely, but in my tests it worked..
>
> src/tape/media_catalog.rs | 31 ++++++++++++++++++++++---------
> 1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/src/tape/media_catalog.rs b/src/tape/media_catalog.rs
> index 65b52a42..8dacf986 100644
> --- a/src/tape/media_catalog.rs
> +++ b/src/tape/media_catalog.rs
> @@ -66,6 +66,7 @@ pub struct MediaCatalog {
>
> content: HashMap<String, DatastoreContent>,
>
> + finished_archive_len: usize,
> pending: Vec<u8>,
> }
>
> @@ -223,6 +224,7 @@ impl MediaCatalog {
> current_archive: None,
> last_entry: None,
> content: HashMap::new(),
> + finished_archive_len: 0,
> pending: Vec::new(),
> };
>
> @@ -231,6 +233,7 @@ impl MediaCatalog {
> if !found_magic_number {
> me.pending.extend(&Self::PROXMOX_BACKUP_MEDIA_CATALOG_MAGIC_1_1);
> }
> + me.finished_archive_len = me.pending.len();
>
> if write {
> me.file = Some(file);
> @@ -300,12 +303,15 @@ impl MediaCatalog {
> current_archive: None,
> last_entry: None,
> content: HashMap::new(),
> + finished_archive_len: 0,
> pending: Vec::new(),
> };
>
> me.log_to_stdout = log_to_stdout;
>
> me.pending.extend(&Self::PROXMOX_BACKUP_MEDIA_CATALOG_MAGIC_1_1);
> + me.finished_archive_len = me.pending.len();
> +
>
> me.register_label(&media_id.label.uuid, 0, 0)?;
>
> @@ -366,31 +372,34 @@ impl MediaCatalog {
> ///
> /// Fixme: this should be atomic ...
> pub fn commit(&mut self) -> Result<(), Error> {
> -
> - if self.pending.is_empty() {
> + if self.finished_archive_len == 0 {
> return Ok(());
> }
>
> match self.file {
> Some(ref mut file) => {
> - file.write_all(&self.pending)?;
> + file.write_all(&self.pending[0..self.finished_archive_len])?;
> file.flush()?;
> file.sync_data()?;
> }
> None => bail!("media catalog not writable (opened read only)"),
> }
>
> - self.pending = Vec::new();
> + let pending = self.pending.len();
> + if self.finished_archive_len < pending {
> + self.pending
> + .copy_within(self.finished_archive_len..pending, 0);
> + }
> + self.pending
> + .truncate(self.pending.len() - self.finished_archive_len);
> + self.finished_archive_len = 0;
>
> Ok(())
> }
>
> - /// Conditionally commit if in pending data is large (> 1Mb)
> + /// Conditionally commit if finished archives in pending data is large (> 1MiB)
> pub fn commit_if_large(&mut self) -> Result<(), Error> {
> - if self.current_archive.is_some() {
> - bail!("can't commit catalog in the middle of an chunk archive");
> - }
> - if self.pending.len() > 1024*1024 {
> + if self.finished_archive_len > 1024*1024 {
> self.commit()?;
> }
> Ok(())
> @@ -498,6 +507,7 @@ impl MediaCatalog {
>
> unsafe { self.pending.write_le_value(entry)?; }
>
> + self.finished_archive_len = self.pending.len();
> self.last_entry = Some((uuid.clone(), file_number));
>
> Ok(())
> @@ -625,6 +635,7 @@ impl MediaCatalog {
>
> unsafe { self.pending.write_le_value(entry)?; }
>
> + self.finished_archive_len = self.pending.len();
> self.last_entry = Some((uuid, file_number));
> }
> }
> @@ -688,6 +699,8 @@ impl MediaCatalog {
> self.pending.push(b':');
> self.pending.extend(snapshot.as_bytes());
>
> + self.finished_archive_len = self.pending.len();
> +
> let content = self.content.entry(store.to_string())
> .or_insert(DatastoreContent::new());
>
> --
> 2.30.2
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-07-21 11:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-20 9:56 [pbs-devel] [RFC PATCH proxmox-backup 1/2] tape: media_catalog: commit as much as possible Dominik Csapak
2021-07-20 9:56 ` [pbs-devel] [RFC PATCH promxox-backup 2/2] api2: tape/backup: commit pool_writer even on error Dominik Csapak
2021-07-21 11:18 [pbs-devel] [RFC PATCH proxmox-backup 1/2] tape: media_catalog: commit as much as possible Dietmar Maurer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal