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

[Christian Ebner]

On 7/18/25 10:20 AM, Lukas Wagner wrote:
> 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

Ah yes, good catch! Specified for both cases that is either the dynamic 
or fixed index.

> 
>> +
>>           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 :)

Ah, makes this much nicer indeed, thanks for the hint!

> 
>> +        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 {
>