public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup v10 07/46] api: backup: store datastore backend in runtime environment
Date: Mon, 21 Jul 2025 18:44:28 +0200	[thread overview]
Message-ID: <20250721164507.1045869-11-c.ebner@proxmox.com> (raw)
In-Reply-To: <20250721164507.1045869-1-c.ebner@proxmox.com>

Get and store the datastore's backend during creation of the backup
runtime environment and upload the chunks to the local filesystem or
s3 object store based on the backend variant.

By storing the backend variant in the environment the s3 client is
instantiated only once and reused for all api calls in the same
backup http/2 connection.

Refactor the upgrade method by moving all logic into the async block,
such that the now possible error on backup environment creation gets
propagated to the thread spawn call side.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 9:
- no changes

 src/api2/backup/environment.rs |  11 +--
 src/api2/backup/mod.rs         | 128 ++++++++++++++++-----------------
 2 files changed, 71 insertions(+), 68 deletions(-)

diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
index 1d8f64aa0..7bd86f39c 100644
--- a/src/api2/backup/environment.rs
+++ b/src/api2/backup/environment.rs
@@ -16,7 +16,7 @@ use pbs_api_types::Authid;
 use pbs_datastore::backup_info::{BackupDir, BackupInfo};
 use pbs_datastore::dynamic_index::DynamicIndexWriter;
 use pbs_datastore::fixed_index::FixedIndexWriter;
-use pbs_datastore::{DataBlob, DataStore};
+use pbs_datastore::{DataBlob, DataStore, DatastoreBackend};
 use proxmox_rest_server::{formatter::*, WorkerTask};
 
 use crate::backup::VerifyWorker;
@@ -116,6 +116,7 @@ pub struct BackupEnvironment {
     pub datastore: Arc<DataStore>,
     pub backup_dir: BackupDir,
     pub last_backup: Option<BackupInfo>,
+    pub backend: DatastoreBackend,
     state: Arc<Mutex<SharedBackupState>>,
 }
 
@@ -126,7 +127,7 @@ impl BackupEnvironment {
         worker: Arc<WorkerTask>,
         datastore: Arc<DataStore>,
         backup_dir: BackupDir,
-    ) -> Self {
+    ) -> Result<Self, Error> {
         let state = SharedBackupState {
             finished: false,
             uid_counter: 0,
@@ -138,7 +139,8 @@ impl BackupEnvironment {
             backup_stat: UploadStatistic::new(),
         };
 
-        Self {
+        let backend = datastore.backend()?;
+        Ok(Self {
             result_attributes: json!({}),
             env_type,
             auth_id,
@@ -148,8 +150,9 @@ impl BackupEnvironment {
             formatter: JSON_FORMATTER,
             backup_dir,
             last_backup: None,
+            backend,
             state: Arc::new(Mutex::new(state)),
-        }
+        })
     }
 
     /// Register a Chunk with associated length.
diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
index a723e7cb0..026f1f106 100644
--- a/src/api2/backup/mod.rs
+++ b/src/api2/backup/mod.rs
@@ -187,7 +187,8 @@ fn upgrade_to_backup_protocol(
             }
 
             // lock last snapshot to prevent forgetting/pruning it during backup
-            let guard = last.backup_dir
+            let guard = last
+                .backup_dir
                 .lock_shared()
                 .with_context(|| format!("while locking last snapshot during backup '{last:?}'"))?;
             Some(guard)
@@ -206,14 +207,14 @@ fn upgrade_to_backup_protocol(
             Some(worker_id),
             auth_id.to_string(),
             true,
-            move |worker| {
+            move |worker| async move {
                 let mut env = BackupEnvironment::new(
                     env_type,
                     auth_id,
                     worker.clone(),
                     datastore,
                     backup_dir,
-                );
+                )?;
 
                 env.debug = debug;
                 env.last_backup = last_backup;
@@ -247,74 +248,73 @@ fn upgrade_to_backup_protocol(
                         http.max_frame_size(4 * 1024 * 1024);
 
                         let env3 = env2.clone();
-                        http.serve_connection(conn, TowerToHyperService::new(service)).map(move |result| {
-                            match result {
-                                Err(err) => {
-                                    // Avoid  Transport endpoint is not connected (os error 107)
-                                    // fixme: find a better way to test for that error
-                                    if err.to_string().starts_with("connection error")
-                                        && env3.finished()
-                                    {
-                                        Ok(())
-                                    } else {
-                                        Err(Error::from(err))
+                        http.serve_connection(conn, TowerToHyperService::new(service))
+                            .map(move |result| {
+                                match result {
+                                    Err(err) => {
+                                        // Avoid  Transport endpoint is not connected (os error 107)
+                                        // fixme: find a better way to test for that error
+                                        if err.to_string().starts_with("connection error")
+                                            && env3.finished()
+                                        {
+                                            Ok(())
+                                        } else {
+                                            Err(Error::from(err))
+                                        }
                                     }
+                                    Ok(()) => Ok(()),
                                 }
-                                Ok(()) => Ok(()),
-                            }
-                        })
+                            })
                     });
                 let mut abort_future = abort_future.map(|_| Err(format_err!("task aborted")));
 
-                async move {
-                    // keep flock until task ends
-                    let _group_guard = _group_guard;
-                    let snap_guard = snap_guard;
-                    let _last_guard = _last_guard;
-
-                    let res = select! {
-                        req = req_fut => req,
-                        abrt = abort_future => abrt,
-                    };
-                    if benchmark {
-                        env.log("benchmark finished successfully");
-                        proxmox_async::runtime::block_in_place(|| env.remove_backup())?;
-                        return Ok(());
+                // keep flock until task ends
+                let _group_guard = _group_guard;
+                let snap_guard = snap_guard;
+                let _last_guard = _last_guard;
+
+                let res = select! {
+                    req = req_fut => req,
+                    abrt = abort_future => abrt,
+                };
+                if benchmark {
+                    env.log("benchmark finished successfully");
+                    proxmox_async::runtime::block_in_place(|| env.remove_backup())?;
+                    return Ok(());
+                }
+
+                let verify = |env: BackupEnvironment| {
+                    if let Err(err) = env.verify_after_complete(snap_guard) {
+                        env.log(format!(
+                            "backup finished, but starting the requested verify task failed: {}",
+                            err
+                        ));
                     }
+                };
 
-                    let verify = |env: BackupEnvironment| {
-                        if let Err(err) = env.verify_after_complete(snap_guard) {
-                            env.log(format!(
-                                "backup finished, but starting the requested verify task failed: {}",
-                                err
-                            ));
-                        }
-                    };
-
-                    match (res, env.ensure_finished()) {
-                        (Ok(_), Ok(())) => {
-                            env.log("backup finished successfully");
-                            verify(env);
-                            Ok(())
-                        }
-                        (Err(err), Ok(())) => {
-                            // ignore errors after finish
-                            env.log(format!("backup had errors but finished: {}", err));
-                            verify(env);
-                            Ok(())
-                        }
-                        (Ok(_), Err(err)) => {
-                            env.log(format!("backup ended and finish failed: {}", err));
-                            env.log("removing unfinished backup");
-                            proxmox_async::runtime::block_in_place(|| env.remove_backup())?;
-                            Err(err)
-                        }
-                        (Err(err), Err(_)) => {
-                            env.log(format!("backup failed: {}", err));
-                            env.log("removing failed backup");
-                            proxmox_async::runtime::block_in_place(|| env.remove_backup())?;
-                            Err(err)
-                        }
+                match (res, env.ensure_finished()) {
+                    (Ok(_), Ok(())) => {
+                        env.log("backup finished successfully");
+                        verify(env);
+                        Ok(())
+                    }
+                    (Err(err), Ok(())) => {
+                        // ignore errors after finish
+                        env.log(format!("backup had errors but finished: {}", err));
+                        verify(env);
+                        Ok(())
+                    }
+                    (Ok(_), Err(err)) => {
+                        env.log(format!("backup ended and finish failed: {}", err));
+                        env.log("removing unfinished backup");
+                        proxmox_async::runtime::block_in_place(|| env.remove_backup())?;
+                        Err(err)
+                    }
+                    (Err(err), Err(_)) => {
+                        env.log(format!("backup failed: {}", err));
+                        env.log("removing failed backup");
+                        proxmox_async::runtime::block_in_place(|| env.remove_backup())?;
+                        Err(err)
                     }
                 }
             },
-- 
2.47.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


  parent reply	other threads:[~2025-07-21 16:44 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-21 16:44 [pbs-devel] [PATCH proxmox{, -backup} v10 00/49] fix #2943: S3 storage backend for datastores Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox v10 1/3] pbs-api-types: extend datastore config by backend config enum Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox v10 2/3] pbs-api-types: maintenance: add new maintenance mode S3 refresh Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox v10 3/3] s3 client: Add missing S3 object key max length check Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 01/46] datastore: add helpers for path/digest to s3 object key conversion Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 02/46] config: introduce s3 object store client configuration Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 03/46] api: config: implement endpoints to manipulate and list s3 configs Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 04/46] api: datastore: check s3 backend bucket access on datastore create Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 05/46] api/cli: add endpoint and command to check s3 client connection Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 06/46] datastore: allow to get the backend for a datastore Christian Ebner
2025-07-21 16:44 ` Christian Ebner [this message]
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 08/46] api: backup: conditionally upload chunks to s3 object store backend Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 09/46] api: backup: conditionally upload blobs " Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 10/46] api: backup: conditionally upload indices " Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 11/46] api: backup: conditionally upload manifest " Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 12/46] api: datastore: conditionally upload client log to s3 backend Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 13/46] sync: pull: conditionally upload content " Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 14/46] api: reader: fetch chunks based on datastore backend Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 15/46] datastore: local chunk reader: read chunks based on backend Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 16/46] verify worker: add datastore backed to verify worker Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 17/46] verify: implement chunk verification for stores with s3 backend Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 18/46] datastore: create namespace marker in " Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 19/46] datastore: create/delete protected marker file on s3 storage backend Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 20/46] datastore: prune groups/snapshots from s3 object store backend Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 21/46] datastore: get and set owner for s3 " Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 22/46] datastore: implement garbage collection for s3 backend Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 23/46] ui: add datastore type selector and reorganize component layout Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 24/46] ui: add s3 client edit window for configuration create/edit Christian Ebner
2025-07-21 20:14   ` Thomas Lamprecht
2025-07-22  6:24     ` Christian Ebner
2025-07-22  7:00       ` Thomas Lamprecht
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 25/46] ui: add s3 client view for configuration Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 26/46] ui: expose the s3 client view in the navigation tree Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 27/46] ui: add s3 client selector and bucket field for s3 backend setup Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 28/46] tools: lru cache: add removed callback for evicted cache nodes Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 29/46] tools: async lru cache: implement insert, remove and contains methods Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 30/46] datastore: add local datastore cache for network attached storages Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 31/46] api: backup: use local datastore cache on s3 backend chunk upload Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 32/46] api: reader: use local datastore cache on s3 backend chunk fetching Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 33/46] datastore: local chunk reader: get cached chunk from local cache store Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 34/46] backup writer: refactor parameters into backup writer options struct Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 35/46] api: backup: add no-cache flag to bypass local datastore cache Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 36/46] api/datastore: implement refresh endpoint for stores with s3 backend Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 37/46] cli: add dedicated subcommand for datastore s3 refresh Christian Ebner
2025-07-21 16:44 ` [pbs-devel] [PATCH proxmox-backup v10 38/46] ui: render s3 refresh as valid maintenance type and task description Christian Ebner
2025-07-21 16:45 ` [pbs-devel] [PATCH proxmox-backup v10 39/46] ui: expose s3 refresh button for datastores backed by object store Christian Ebner
2025-07-21 16:45 ` [pbs-devel] [PATCH proxmox-backup v10 40/46] datastore: conditionally upload atime marker chunk to s3 backend Christian Ebner
2025-07-21 16:45 ` [pbs-devel] [PATCH proxmox-backup v10 41/46] bin: implement client subcommands for s3 configuration manipulation Christian Ebner
2025-07-21 16:45 ` [pbs-devel] [PATCH proxmox-backup v10 42/46] bin: expose reuse-datastore flag for proxmox-backup-manager Christian Ebner
2025-07-21 16:45 ` [pbs-devel] [PATCH proxmox-backup v10 43/46] datastore: mark store as in-use by setting marker on s3 backend Christian Ebner
2025-07-21 16:45 ` [pbs-devel] [PATCH proxmox-backup v10 44/46] datastore: run s3-refresh when reusing a datastore with " Christian Ebner
2025-07-21 16:45 ` [pbs-devel] [PATCH proxmox-backup v10 45/46] api/ui: add flag to allow overwriting in-use marker for " Christian Ebner
2025-07-21 16:45 ` [pbs-devel] [PATCH proxmox-backup v10 46/46] docs: Add section describing how to setup s3 backed datastore Christian Ebner
2025-07-22  8:14 ` [pbs-devel] [PATCH proxmox{, -backup} v10 00/49] fix #2943: S3 storage backend for datastores Hannes Laimer
2025-07-22  9:29 ` Lukas Wagner
2025-07-22 10:13 ` [pbs-devel] superseded: " Christian Ebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250721164507.1045869-11-c.ebner@proxmox.com \
    --to=c.ebner@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal