Re: [pbs-devel] [PATCH proxmox-backup v2 5/7] api2: tape/backup: commit pool_writer even on error

Re: [pbs-devel] [PATCH proxmox-backup v2 5/7] api2: tape/backup: commit pool_writer even on error

[Dietmar Maurer]

> On 07/22/2021 3:41 PM Dominik Csapak <d.csapak@proxmox.com> wrote:
> 
>  
> this way we store all finished snapshots/chunk archives we in the
> catalog, and not onlye those until the last commit

I guess this is wrong, because there is no guarrantee that this data is flushed to tape?

[pbs-devel] [PATCH proxmox-backup v2 0/7] improve catalog handling

[Dominik Csapak]

this series combines my previous catalog related patch-series[0][1][2]

changes the catalog interface to be more concise, optimizes catalog
commit calls during restore, and implements a fast catalog for the
gui which only contains the snapshot lists

changes from v1:
* only write snapshot list in new 'finish' method of the catalog
* add 'finish' also to pool writer
* replace pending offset counter with reducing the chunk_archive
  interface of the catalog

0: https://lists.proxmox.com/pipermail/pbs-devel/2021-July/003711.html
1: https://lists.proxmox.com/pipermail/pbs-devel/2021-July/003715.html
2: https://lists.proxmox.com/pipermail/pbs-devel/2021-July/003714.html

Dominik Csapak (7):
  tape: media_catalog: improve chunk_archive interface
  tape: media_catalog: add fast_catalog beside normal catalog
  tape: pool_writer: finish the catalog when its done
  tape: media_catalog: add local type aliases to make code more clear
  api2: tape/backup: commit pool_writer even on error
  api2: tape/restore: finish temporary catalog at the end
  api2: tape: media: use MediaCatalog::snapshot_list for content listing

 src/api2/tape/backup.rs             | 117 +++++++++----------
 src/api2/tape/media.rs              |  44 ++++----
 src/api2/tape/restore.rs            |  11 +-
 src/tape/media_catalog.rs           | 167 +++++++++++++++++++++++++---
 src/tape/pool_writer/catalog_set.rs |   9 +-
 src/tape/pool_writer/mod.rs         |  15 ++-
 6 files changed, 250 insertions(+), 113 deletions(-)

-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 5/7] api2: tape/backup: commit pool_writer even on error

[Dominik Csapak]

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 45ee4bad..c78b697b 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.finish()?;
 
+    let _ = res?; // bubble errors up
+
     if need_catalog {
         task_log!(worker, "append media catalog");
 
-- 
2.30.2