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

[Lukas Wagner]

Reviewed-by: Lukas Wagner <l.wagner at proxmox.com>

Two nits inline.

On  2025-07-15 14:52, 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 at proxmox.com>
> ---
> changes since version 7:
> - fix clippy warning and formatting
> 
>  src/api2/backup/environment.rs | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
> index 3d4677975..9ad13aeb3 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
> +            ))
> +        }

nit: the log message differs between both cases

> +
>          self.log_upload_stat(
>              &data.name,
>              &expected_csum,
> @@ -753,6 +772,21 @@ impl BackupEnvironment {
>  
>          Ok(())
>      }
> +
> +    fn s3_upload_index(&self, s3_client: &S3Client, name: &str) -> Result<(), Error> {
> +        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)?;

nit: You can use std::fs::read() to get the Vec right away :)

> +        let data = hyper::body::Bytes::from(buffer);
> +        proxmox_async::runtime::block_on(s3_client.upload_with_retry(object_key, data, true))?;
> +        Ok(())
> +    }
>  }
>  
>  impl RpcEnvironment for BackupEnvironment {

-- 
- Lukas