public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup v2 3/3] GC: S3: phase2: do not force delete for every list iteration
Date: Fri, 21 Nov 2025 11:18:43 +0100	[thread overview]
Message-ID: <20251121101849.463119-4-f.gruenbichler@proxmox.com> (raw)
In-Reply-To: <20251121101849.463119-1-f.gruenbichler@proxmox.com>

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

  parent reply	other threads:[~2025-11-21 10:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-21 10:18 [pbs-devel] [PATCH proxmox-backup v2 0/3] reduce GC S3 locking Fabian Grünbichler
2025-11-21 10:18 ` [pbs-devel] [PATCH proxmox-backup v2 1/3] GC: S3: reduce number of open FDs for to-be-deleted objects Fabian Grünbichler
2025-11-21 10:18 ` [pbs-devel] [PATCH proxmox-backup v2 2/3] GC: S3: factor out batch object deletion Fabian Grünbichler
2025-11-21 10:18 ` Fabian Grünbichler [this message]
2025-11-21 11:28   ` [pbs-devel] [PATCH proxmox-backup v2 3/3] GC: S3: phase2: do not force delete for every list iteration Christian Ebner
2025-11-21 11:54   ` [pbs-devel] [PATCH RESEND " Fabian Grünbichler
2025-11-21 12:04     ` Christian Ebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251121101849.463119-4-f.gruenbichler@proxmox.com \
    --to=f.gruenbichler@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal