[pbs-devel] [PATCH proxmox-backup 1/3] GC: S3: reduce number of open FDs for to-be-deleted objects

[Christian Ebner]

one comment inline

On 11/21/25 10:06 AM, Fabian Grünbichler wrote:
> listing objects on the S3 side will return batches containing up to 1000
> objects. previously, if all those objects were garbage, phase2 would open and
> hold the lock file for each of them and delete them using a single call. this
> can easily run afoul the maximum number of open files allowed by the default
> process limits, which is 1024.
> 
> converting the code to instead delete batches of (at most) 100 objects should
> alleviate this issue until bumping the limit is deemed safe, while (in the
> worst case) causing 10x the number of delete requests.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> ---
>   pbs-datastore/src/datastore.rs | 25 ++++++++++++++++++++++++-
>   1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index 0a5179230..1afcef53a 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -58,6 +58,8 @@ pub const S3_DATASTORE_IN_USE_MARKER: &str = ".in-use";
>   const NAMESPACE_MARKER_FILENAME: &str = ".namespace";
>   // s3 put request times out after upload_size / 1 Kib/s, so about 2.3 hours for 8 MiB
>   const CHUNK_LOCK_TIMEOUT: Duration = Duration::from_secs(3 * 60 * 60);
> +// s3 deletion batch size to avoid 1024 open files soft limit
> +const S3_DELETE_BATCH_LIMIT: usize = 100;
>   
>   /// checks if auth_id is owner, or, if owner is a token, if
>   /// auth_id is the user of the token
> @@ -1657,7 +1659,7 @@ impl DataStore {
>                   proxmox_async::runtime::block_on(s3_client.list_objects_v2(&prefix, None))
>                       .context("failed to list chunk in s3 object store")?;
>   
> -            let mut delete_list = Vec::with_capacity(1000);
> +            let mut delete_list = Vec::with_capacity(S3_DELETE_BATCH_LIMIT);
>               loop {
>                   for content in list_bucket_result.contents {
>                       let (chunk_path, digest, bad) =
> @@ -1716,8 +1718,29 @@ impl DataStore {
>                       }
>   
>                       chunk_count += 1;
> +
> +                    // drop guard because of async S3 call below
> +                    drop(_guard);
> +
> +                    // limit pending deletes to avoid holding too many chunk flocks
> +                    if delete_list.len() > S3_DELETE_BATCH_LIMIT {

off by one error: this should never exceed the limit but rather delete 
once it is reached.

> +                        let delete_objects_result = proxmox_async::runtime::block_on(
> +                            s3_client.delete_objects(
> +                                &delete_list
> +                                    .iter()
> +                                    .map(|(key, _)| key.clone())
> +                                    .collect::<Vec<S3ObjectKey>>(),
> +                            ),
> +                        )?;
> +                        if let Some(_err) = delete_objects_result.error {
> +                            bail!("failed to delete some objects");
> +                        }
> +                        // release all chunk guards
> +                        delete_list.clear();
> +                    }
>                   }
>   
> +                // delete the last batch of objects, if there are any remaining
>                   if !delete_list.is_empty() {
>                       let delete_objects_result = proxmox_async::runtime::block_on(
>                           s3_client.delete_objects(