[pbs-devel] [PATCH proxmox-backup v2 0/3] reduce GC S3 locking

[pbs-devel] [PATCH proxmox-backup v2 0/3] reduce GC S3 locking

[Fabian Grünbichler]

this patch series tries to reduce the number of open locks held by GC,
in particular in case most objects returned by the S3 backend are
garbage that need deletion.

the first patch reduces the number of open locks by at least a factor of
10 in the worst case (from up to 1000 to up to 100).

the second patch just refactors some now common code.

the third patch tries to reduce the number of delete calls for regular
GC runs by batching deletes more efficiently, while still periodically
flushing the deferred deletes.

Fabian Grünbichler (3):
  GC: S3: reduce number of open FDs for to-be-deleted objects
  GC: S3: factor out batch object deletion
  GC: S3: phase2: do not force delete for every list iteration

 pbs-datastore/src/datastore.rs | 61 ++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 17 deletions(-)

-- 
2.47.3



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

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

[Fabian Grünbichler]

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@proxmox.com>
Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>
---

Notes:
    v2: >= LIMIT instead of > LIMIT, thanks Chris

 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..09ec23fc4 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 {
+                        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(
-- 
2.47.3



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

[pbs-devel] [PATCH proxmox-backup v2 2/3] GC: S3: factor out batch object deletion

[Fabian Grünbichler]

since we do it twice with identical code, move that code to a closure.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>
---

Notes:
    v1->v2: don't drop comment when extracting helper

 pbs-datastore/src/datastore.rs | 47 +++++++++++++++-------------------
 1 file changed, 21 insertions(+), 26 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 09ec23fc4..e9d6b46f3 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1660,6 +1660,25 @@ impl DataStore {
                     .context("failed to list chunk in s3 object store")?;
 
             let mut delete_list = Vec::with_capacity(S3_DELETE_BATCH_LIMIT);
+
+            let s3_delete_batch = |delete_list: &mut Vec<(S3ObjectKey, BackupLockGuard)>,
+                                   s3_client: &Arc<S3Client>|
+             -> Result<(), Error> {
+                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");
+                }
+                // drops all chunk flock guards
+                delete_list.clear();
+                Ok(())
+            };
             loop {
                 for content in list_bucket_result.contents {
                     let (chunk_path, digest, bad) =
@@ -1724,37 +1743,13 @@ impl DataStore {
 
                     // limit pending deletes to avoid holding too many chunk flocks
                     if delete_list.len() >= S3_DELETE_BATCH_LIMIT {
-                        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();
+                        s3_delete_batch(&mut delete_list, s3_client)?;
                     }
                 }
 
                 // 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(
-                            &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();
+                    s3_delete_batch(&mut delete_list, s3_client)?;
                 }
 
                 // Process next batch of chunks if there is more
-- 
2.47.3



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

[pbs-devel] [PATCH proxmox-backup v2 3/3] GC: S3: phase2: do not force delete for every list iteration

[Fabian Grünbichler]

delete after at most 100 iterations, if the laster iteration that started with
an empty delete list was more than 5 minutes ago and at the very end, instead
of after every processing every batch of 1000 listed objects. this reduces the
number of delete calls made to the backend, making regular garbage collections
that do not delete most objects cheaper, but means holding the flocks for
garbage chunks/objects longer.

Suggested-by: Chris-Ebner <c.ebner@proxmox.com>
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v1->v2: reworked to use age-based cutoff

    the 5 minutes there are pretty arbitrary, feel free to go up or
    down..

 pbs-datastore/src/datastore.rs | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index e9d6b46f3..4cac12406 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -22,7 +22,7 @@ use proxmox_sys::error::SysError;
 use proxmox_sys::fs::{file_read_optional_string, replace_file, CreateOptions};
 use proxmox_sys::linux::procfs::MountInfo;
 use proxmox_sys::process_locker::{ProcessLockExclusiveGuard, ProcessLockSharedGuard};
-use proxmox_time::TimeSpan;
+use proxmox_time::{epoch_i64, TimeSpan};
 use proxmox_worker_task::WorkerTaskContext;
 
 use pbs_api_types::{
@@ -60,6 +60,8 @@ const NAMESPACE_MARKER_FILENAME: &str = ".namespace";
 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;
+// max defer time for s3 batch deletions
+const S3_DELETE_DEFER_LIMIT_SECONDS: i64 = 60 * 5;
 
 /// checks if auth_id is owner, or, if owner is a token, if
 /// auth_id is the user of the token
@@ -1660,6 +1662,7 @@ impl DataStore {
                     .context("failed to list chunk in s3 object store")?;
 
             let mut delete_list = Vec::with_capacity(S3_DELETE_BATCH_LIMIT);
+            let mut delete_list_age = epoch_i64();
 
             let s3_delete_batch = |delete_list: &mut Vec<(S3ObjectKey, BackupLockGuard)>,
                                    s3_client: &Arc<S3Client>|
@@ -1742,16 +1745,12 @@ impl DataStore {
                     drop(_guard);
 
                     // limit pending deletes to avoid holding too many chunk flocks
-                    if delete_list.len() >= S3_DELETE_BATCH_LIMIT {
+                    if delete_list.len() >= S3_DELETE_BATCH_LIMIT
+                        || epoch_i64() - delete_list_age > S3_DELETE_DEFER_LIMIT_SECONDS
+                    {
                         s3_delete_batch(&mut delete_list, s3_client)?;
                     }
                 }
-
-                // delete the last batch of objects, if there are any remaining
-                if !delete_list.is_empty() {
-                    s3_delete_batch(&mut delete_list, s3_client)?;
-                }
-
                 // Process next batch of chunks if there is more
                 if list_bucket_result.is_truncated {
                     list_bucket_result =
@@ -1759,11 +1758,21 @@ impl DataStore {
                             &prefix,
                             list_bucket_result.next_continuation_token.as_deref(),
                         ))?;
+                    if delete_list.is_empty() {
+                        // reset delete list age while queue is empty
+                        delete_list_age = epoch_i64();
+                    }
                     continue;
                 }
 
                 break;
             }
+
+            // delete the last batch of objects, if there are any remaining
+            if !delete_list.is_empty() {
+                s3_delete_batch(&mut delete_list, s3_client)?;
+            }
+
             info!("processed {chunk_count} total chunks");
 
             // Phase 2 GC of Filesystem backed storage is phase 3 for S3 backed GC
-- 
2.47.3



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

Re: [pbs-devel] [PATCH proxmox-backup v2 3/3] GC: S3: phase2: do not force delete for every list iteration

[Christian Ebner]

Code looks good to me and behaves as expected, only 2 small nits which 
might however be folded in when applied.

Tested by customizing the timeout and adding delays and some log output.

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>

On 11/21/25 11:18 AM, Fabian Grünbichler wrote:
> delete after at most 100 iterations, if the laster iteration that started with

nit: above does not match the code behavior though, leftover from 
previous approach?

> an empty delete list was more than 5 minutes ago and at the very end, instead
> of after every processing every batch of 1000 listed objects. this reduces the
> number of delete calls made to the backend, making regular garbage collections
> that do not delete most objects cheaper, but means holding the flocks for
> garbage chunks/objects longer.
> 
> Suggested-by: Chris-Ebner <c.ebner@proxmox.com>

nit: name ;)

> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>      v1->v2: reworked to use age-based cutoff
> 
>      the 5 minutes there are pretty arbitrary, feel free to go up or
>      down..
> 
>   pbs-datastore/src/datastore.rs | 25 +++++++++++++++++--------
>   1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index e9d6b46f3..4cac12406 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -22,7 +22,7 @@ use proxmox_sys::error::SysError;
>   use proxmox_sys::fs::{file_read_optional_string, replace_file, CreateOptions};
>   use proxmox_sys::linux::procfs::MountInfo;
>   use proxmox_sys::process_locker::{ProcessLockExclusiveGuard, ProcessLockSharedGuard};
> -use proxmox_time::TimeSpan;
> +use proxmox_time::{epoch_i64, TimeSpan};
>   use proxmox_worker_task::WorkerTaskContext;
>   
>   use pbs_api_types::{
> @@ -60,6 +60,8 @@ const NAMESPACE_MARKER_FILENAME: &str = ".namespace";
>   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;
> +// max defer time for s3 batch deletions
> +const S3_DELETE_DEFER_LIMIT_SECONDS: i64 = 60 * 5;
>   
>   /// checks if auth_id is owner, or, if owner is a token, if
>   /// auth_id is the user of the token
> @@ -1660,6 +1662,7 @@ impl DataStore {
>                       .context("failed to list chunk in s3 object store")?;
>   
>               let mut delete_list = Vec::with_capacity(S3_DELETE_BATCH_LIMIT);
> +            let mut delete_list_age = epoch_i64();
>   
>               let s3_delete_batch = |delete_list: &mut Vec<(S3ObjectKey, BackupLockGuard)>,
>                                      s3_client: &Arc<S3Client>|
> @@ -1742,16 +1745,12 @@ impl DataStore {
>                       drop(_guard);
>   
>                       // limit pending deletes to avoid holding too many chunk flocks
> -                    if delete_list.len() >= S3_DELETE_BATCH_LIMIT {
> +                    if delete_list.len() >= S3_DELETE_BATCH_LIMIT
> +                        || epoch_i64() - delete_list_age > S3_DELETE_DEFER_LIMIT_SECONDS
> +                    {
>                           s3_delete_batch(&mut delete_list, s3_client)?;
>                       }
>                   }
> -
> -                // delete the last batch of objects, if there are any remaining
> -                if !delete_list.is_empty() {
> -                    s3_delete_batch(&mut delete_list, s3_client)?;
> -                }
> -
>                   // Process next batch of chunks if there is more
>                   if list_bucket_result.is_truncated {
>                       list_bucket_result =
> @@ -1759,11 +1758,21 @@ impl DataStore {
>                               &prefix,
>                               list_bucket_result.next_continuation_token.as_deref(),
>                           ))?;
> +                    if delete_list.is_empty() {
> +                        // reset delete list age while queue is empty
> +                        delete_list_age = epoch_i64();
> +                    }
>                       continue;
>                   }
>   
>                   break;
>               }
> +
> +            // delete the last batch of objects, if there are any remaining
> +            if !delete_list.is_empty() {
> +                s3_delete_batch(&mut delete_list, s3_client)?;
> +            }
> +
>               info!("processed {chunk_count} total chunks");
>   
>               // Phase 2 GC of Filesystem backed storage is phase 3 for S3 backed GC



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

[pbs-devel] [PATCH RESEND proxmox-backup v2 3/3] GC: S3: phase2: do not force delete for every list iteration

[Fabian Grünbichler]

force delete if the last iteration that started with an empty delete list was
started more than 5 minutes ago, and at the very end after the last iteration,
instead of after processing every batch of 1000 listed objects. this
reduces the number of delete calls made to the backend, making regular
garbage collections that do not delete most objects cheaper, but means
holding the flocks for garbage chunks/objects longer.

Suggested-by: Christian Ebner <c.ebner@proxmox.com>
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v1->v2: reworked to use age-based cutoff

    resent with trailer and commit message fixed up, thanks Chris!

 pbs-datastore/src/datastore.rs | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index e9d6b46f3..4cac12406 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -22,7 +22,7 @@ use proxmox_sys::error::SysError;
 use proxmox_sys::fs::{file_read_optional_string, replace_file, CreateOptions};
 use proxmox_sys::linux::procfs::MountInfo;
 use proxmox_sys::process_locker::{ProcessLockExclusiveGuard, ProcessLockSharedGuard};
-use proxmox_time::TimeSpan;
+use proxmox_time::{epoch_i64, TimeSpan};
 use proxmox_worker_task::WorkerTaskContext;
 
 use pbs_api_types::{
@@ -60,6 +60,8 @@ const NAMESPACE_MARKER_FILENAME: &str = ".namespace";
 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;
+// max defer time for s3 batch deletions
+const S3_DELETE_DEFER_LIMIT_SECONDS: i64 = 60 * 5;
 
 /// checks if auth_id is owner, or, if owner is a token, if
 /// auth_id is the user of the token
@@ -1660,6 +1662,7 @@ impl DataStore {
                     .context("failed to list chunk in s3 object store")?;
 
             let mut delete_list = Vec::with_capacity(S3_DELETE_BATCH_LIMIT);
+            let mut delete_list_age = epoch_i64();
 
             let s3_delete_batch = |delete_list: &mut Vec<(S3ObjectKey, BackupLockGuard)>,
                                    s3_client: &Arc<S3Client>|
@@ -1742,16 +1745,12 @@ impl DataStore {
                     drop(_guard);
 
                     // limit pending deletes to avoid holding too many chunk flocks
-                    if delete_list.len() >= S3_DELETE_BATCH_LIMIT {
+                    if delete_list.len() >= S3_DELETE_BATCH_LIMIT
+                        || epoch_i64() - delete_list_age > S3_DELETE_DEFER_LIMIT_SECONDS
+                    {
                         s3_delete_batch(&mut delete_list, s3_client)?;
                     }
                 }
-
-                // delete the last batch of objects, if there are any remaining
-                if !delete_list.is_empty() {
-                    s3_delete_batch(&mut delete_list, s3_client)?;
-                }
-
                 // Process next batch of chunks if there is more
                 if list_bucket_result.is_truncated {
                     list_bucket_result =
@@ -1759,11 +1758,21 @@ impl DataStore {
                             &prefix,
                             list_bucket_result.next_continuation_token.as_deref(),
                         ))?;
+                    if delete_list.is_empty() {
+                        // reset delete list age while queue is empty
+                        delete_list_age = epoch_i64();
+                    }
                     continue;
                 }
 
                 break;
             }
+
+            // delete the last batch of objects, if there are any remaining
+            if !delete_list.is_empty() {
+                s3_delete_batch(&mut delete_list, s3_client)?;
+            }
+
             info!("processed {chunk_count} total chunks");
 
             // Phase 2 GC of Filesystem backed storage is phase 3 for S3 backed GC
-- 
2.47.3



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

Re: [pbs-devel] [PATCH RESEND proxmox-backup v2 3/3] GC: S3: phase2: do not force delete for every list iteration

[Christian Ebner]

unfortunately you were a bit faster as I was just to reply with another 
corner case which needs to be fixed. See inline.

On 11/21/25 12:54 PM, Fabian Grünbichler wrote:
> force delete if the last iteration that started with an empty delete list was
> started more than 5 minutes ago, and at the very end after the last iteration,
> instead of after processing every batch of 1000 listed objects. this
> reduces the number of delete calls made to the backend, making regular
> garbage collections that do not delete most objects cheaper, but means
> holding the flocks for garbage chunks/objects longer.
> 
> Suggested-by: Christian Ebner <c.ebner@proxmox.com>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>      v1->v2: reworked to use age-based cutoff
> 
>      resent with trailer and commit message fixed up, thanks Chris!
> 
>   pbs-datastore/src/datastore.rs | 25 +++++++++++++++++--------
>   1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index e9d6b46f3..4cac12406 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -22,7 +22,7 @@ use proxmox_sys::error::SysError;
>   use proxmox_sys::fs::{file_read_optional_string, replace_file, CreateOptions};
>   use proxmox_sys::linux::procfs::MountInfo;
>   use proxmox_sys::process_locker::{ProcessLockExclusiveGuard, ProcessLockSharedGuard};
> -use proxmox_time::TimeSpan;
> +use proxmox_time::{epoch_i64, TimeSpan};
>   use proxmox_worker_task::WorkerTaskContext;
>   
>   use pbs_api_types::{
> @@ -60,6 +60,8 @@ const NAMESPACE_MARKER_FILENAME: &str = ".namespace";
>   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;
> +// max defer time for s3 batch deletions
> +const S3_DELETE_DEFER_LIMIT_SECONDS: i64 = 60 * 5;
>   
>   /// checks if auth_id is owner, or, if owner is a token, if
>   /// auth_id is the user of the token
> @@ -1660,6 +1662,7 @@ impl DataStore {
>                       .context("failed to list chunk in s3 object store")?;
>   
>               let mut delete_list = Vec::with_capacity(S3_DELETE_BATCH_LIMIT);
> +            let mut delete_list_age = epoch_i64();
>   
>               let s3_delete_batch = |delete_list: &mut Vec<(S3ObjectKey, BackupLockGuard)>,
>                                      s3_client: &Arc<S3Client>|
> @@ -1742,16 +1745,12 @@ impl DataStore {
>                       drop(_guard);
>   
>                       // limit pending deletes to avoid holding too many chunk flocks
> -                    if delete_list.len() >= S3_DELETE_BATCH_LIMIT {
> +                    if delete_list.len() >= S3_DELETE_BATCH_LIMIT
> +                        || epoch_i64() - delete_list_age > S3_DELETE_DEFER_LIMIT_SECONDS
> +                    {
>                           s3_delete_batch(&mut delete_list, s3_client)?;

this needs to reset the delete list age as well, as otherwise the next 
set of batches being processed might run into this as well if that is 
slow for some reason, even if the list is empty

>                       }
>                   }
> -
> -                // delete the last batch of objects, if there are any remaining
> -                if !delete_list.is_empty() {
> -                    s3_delete_batch(&mut delete_list, s3_client)?;
> -                }
> -
>                   // Process next batch of chunks if there is more
>                   if list_bucket_result.is_truncated {
>                       list_bucket_result =
> @@ -1759,11 +1758,21 @@ impl DataStore {
>                               &prefix,
>                               list_bucket_result.next_continuation_token.as_deref(),
>                           ))?;
> +                    if delete_list.is_empty() {
> +                        // reset delete list age while queue is empty
> +                        delete_list_age = epoch_i64();
> +                    }
>                       continue;
>                   }
>   
>                   break;
>               }
> +
> +            // delete the last batch of objects, if there are any remaining
> +            if !delete_list.is_empty() {
> +                s3_delete_batch(&mut delete_list, s3_client)?;
> +            }
> +
>               info!("processed {chunk_count} total chunks");
>   
>               // Phase 2 GC of Filesystem backed storage is phase 3 for S3 backed GC



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