Re: [pbs-devel] [PATCH proxmox-backup v6 10/37] api: backup: conditionally upload indices to s3 object store backend

[Christian Ebner <c.ebner@proxmox.com>]

On 7/8/25 19:00, Christian Ebner wrote:
> If the datastore is backed by an S3 compatible object store, upload
> the dynamic or fixed index files to the object store after closing
> them. The local index files are kept in the local caching datastore
> to allow for fast and efficient content lookups, avoiding expensive
> (as in monetary cost and IO latency) requests.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>   src/api2/backup/environment.rs | 41 ++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)
> 
> diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
> index 3d4677975..1ee923005 100644
> --- a/src/api2/backup/environment.rs
> +++ b/src/api2/backup/environment.rs
> @@ -2,6 +2,7 @@ use anyhow::{bail, format_err, Context, Error};
>   use pbs_config::BackupLockGuard;
>   
>   use std::collections::HashMap;
> +use std::io::Read;
>   use std::sync::{Arc, Mutex};
>   use tracing::info;
>   
> @@ -18,6 +19,7 @@ use pbs_datastore::dynamic_index::DynamicIndexWriter;
>   use pbs_datastore::fixed_index::FixedIndexWriter;
>   use pbs_datastore::{DataBlob, DataStore, DatastoreBackend};
>   use proxmox_rest_server::{formatter::*, WorkerTask};
> +use proxmox_s3_client::S3Client;
>   
>   use crate::backup::VerifyWorker;
>   
> @@ -479,6 +481,13 @@ impl BackupEnvironment {
>               );
>           }
>   
> +        // For S3 backends, upload the index file to the object store after closing
> +        if let DatastoreBackend::S3(s3_client) = &self.backend {
> +            self.s3_upload_index(s3_client, &data.name)
> +                .context("failed to upload dynamic index to s3 backend")?;
> +            self.log(format!("Uploaded index file to s3 backend: {}", data.name))
> +        }
> +
>           self.log_upload_stat(
>               &data.name,
>               &csum,
> @@ -553,6 +562,16 @@ impl BackupEnvironment {
>               );
>           }
>   
> +        // For S3 backends, upload the index file to the object store after closing
> +        if let DatastoreBackend::S3(s3_client) = &self.backend {
> +            self.s3_upload_index(s3_client, &data.name)
> +                .context("failed to upload fixed index to s3 backend")?;
> +            self.log(format!(
> +                "Uploaded fixed index file to object store: {}",
> +                data.name
> +            ))
> +        }
> +
>           self.log_upload_stat(
>               &data.name,
>               &expected_csum,
> @@ -753,6 +772,28 @@ impl BackupEnvironment {
>   
>           Ok(())
>       }
> +
> +    fn s3_upload_index(&self, s3_client: &S3Client, name: &str) -> Result<(), Error> {
> +        let mut object_key = self.backup_dir.relative_path();
> +        object_key.push(name);
> +        let object_key = object_key
> +            .as_os_str()
> +            .to_str()
> +            .ok_or_else(|| format_err!("invalid file name"))?;

There is a regression from v5 here:

This was forgotten to refactor to the new s3 object key helper, 
therefore the key is now not correctly prefixed by the `.cnt` content 
prefix, and slipped trough testing as with the locally cached index 
files PBS behaves just fine.

The following diff fixes the issue and will be included in a new version:

diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
index 7bfd27c49..9cfc2d528 100644
--- a/src/api2/backup/environment.rs
+++ b/src/api2/backup/environment.rs
@@ -778,15 +777,12 @@ impl BackupEnvironment {
      }

      fn s3_upload_index(&self, s3_client: &S3Client, name: &str) -> 
Result<(), Error> {
-        let mut object_key = self.backup_dir.relative_path();
-        object_key.push(name);
-        let object_key = object_key
-            .as_os_str()
-            .to_str()
-            .ok_or_else(|| format_err!("invalid file name"))?;
-
-        let mut full_path = self.datastore.base_path();
-        full_path.push(object_key);
+        let object_key =
+ 
pbs_datastore::s3::object_key_from_path(&self.backup_dir.relative_path(), 
name)
+                .context("invalid index file object key")?;
+
+        let mut full_path = self.backup_dir.full_path();
+        full_path.push(name);
          let mut file = std::fs::File::open(&full_path)?;
          let mut buffer = Vec::new();
          file.read_to_end(&mut buffer)?;


I'm holding off from re-sending the patch series for now in case other 
regressions arise or there are further comments on the code.


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