[pbs-devel] [PATCH proxmox-backup 0/2] raise nofile limit for GC on S3 stores

[pbs-devel] [PATCH proxmox-backup 0/2] raise nofile limit for GC on S3 stores

[Christian Ebner]

Datastores backed by s3 object stores rely on per-chunk file locks
for consistency. However, the soft limit for open file handles might
be to low during garbage collection, as there chunks are processed in
batches of up to 1000 objects. Therefore, use the pre-existing helper
defined in the proxmox-backup-client to raise the resource limits to
the hard limit at start of garbage collection for s3 backed
datastores.

Christian Ebner (2):
  tools: move rlimit helper from pbs-client to pbs-tools
  GC: raise nofile soft limit to the hard limit on s3 backed stores

 pbs-client/src/tools/mod.rs       | 23 -----------------------
 pbs-datastore/src/datastore.rs    |  7 +++++++
 pbs-tools/src/lib.rs              | 25 +++++++++++++++++++++++++
 proxmox-backup-client/src/main.rs |  4 ++--
 4 files changed, 34 insertions(+), 25 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 1/2] tools: move rlimit helper from pbs-client to pbs-tools

[Christian Ebner]

Move the helper to the more common tools, since it will be reused
for bumping the limits during garbage collection for datastores
with S3 backend.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-client/src/tools/mod.rs       | 23 -----------------------
 pbs-tools/src/lib.rs              | 25 +++++++++++++++++++++++++
 proxmox-backup-client/src/main.rs |  4 ++--
 3 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index 7a496d14c..8b0d3c806 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -711,29 +711,6 @@ pub fn has_pxar_filename_extension(name: &str, with_didx_extension: bool) -> boo
     }
 }
 
-/// Raise the soft limit for open file handles to the hard limit
-///
-/// Returns the values set before raising the limit as libc::rlimit64
-pub fn raise_nofile_limit() -> Result<libc::rlimit64, Error> {
-    let mut old = libc::rlimit64 {
-        rlim_cur: 0,
-        rlim_max: 0,
-    };
-    if 0 != unsafe { libc::getrlimit64(libc::RLIMIT_NOFILE, &mut old as *mut libc::rlimit64) } {
-        bail!("Failed to get nofile rlimit");
-    }
-
-    let mut new = libc::rlimit64 {
-        rlim_cur: old.rlim_max,
-        rlim_max: old.rlim_max,
-    };
-    if 0 != unsafe { libc::setrlimit64(libc::RLIMIT_NOFILE, &mut new as *mut libc::rlimit64) } {
-        bail!("Failed to set nofile rlimit");
-    }
-
-    Ok(old)
-}
-
 /// Creates a temporary file (with `O_TMPFILE`) in `XDG_CACHE_HOME`. If we
 /// cannot create the file there it will be created in `/tmp` instead.
 pub fn create_tmp_file() -> std::io::Result<std::fs::File> {
diff --git a/pbs-tools/src/lib.rs b/pbs-tools/src/lib.rs
index af900c925..e9be0c6f0 100644
--- a/pbs-tools/src/lib.rs
+++ b/pbs-tools/src/lib.rs
@@ -1,3 +1,5 @@
+use anyhow::{bail, Error};
+
 pub mod cert;
 pub mod crypt_config;
 pub mod format;
@@ -26,3 +28,26 @@ pub fn setup_libc_malloc_opts() {
         libc::mallopt(libc::M_MMAP_THRESHOLD, 4096 * 32);
     }
 }
+
+/// Raise the soft limit for open file handles to the hard limit
+///
+/// Returns the values set before raising the limit as libc::rlimit64
+pub fn raise_nofile_limit() -> Result<libc::rlimit64, Error> {
+    let mut old = libc::rlimit64 {
+        rlim_cur: 0,
+        rlim_max: 0,
+    };
+    if 0 != unsafe { libc::getrlimit64(libc::RLIMIT_NOFILE, &mut old as *mut libc::rlimit64) } {
+        bail!("Failed to get nofile rlimit");
+    }
+
+    let mut new = libc::rlimit64 {
+        rlim_cur: old.rlim_max,
+        rlim_max: old.rlim_max,
+    };
+    if 0 != unsafe { libc::setrlimit64(libc::RLIMIT_NOFILE, &mut new as *mut libc::rlimit64) } {
+        bail!("Failed to set nofile rlimit");
+    }
+
+    Ok(old)
+}
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index 999e50205..16b02ac99 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -41,7 +41,7 @@ use pbs_client::tools::{
         crypto_parameters, format_key_source, get_encryption_key_password, KEYFD_SCHEMA,
         KEYFILE_SCHEMA, MASTER_PUBKEY_FD_SCHEMA, MASTER_PUBKEY_FILE_SCHEMA,
     },
-    raise_nofile_limit, CHUNK_SIZE_SCHEMA, REPO_URL_SCHEMA,
+    CHUNK_SIZE_SCHEMA, REPO_URL_SCHEMA,
 };
 use pbs_client::{
     delete_ticket_info, parse_backup_specification, view_task_result, BackupDetectionMode,
@@ -58,7 +58,7 @@ use pbs_datastore::manifest::BackupManifest;
 use pbs_datastore::read_chunk::AsyncReadChunk;
 use pbs_key_config::{decrypt_key, rsa_encrypt_key_config, KeyConfig};
 use pbs_tools::crypt_config::CryptConfig;
-use pbs_tools::json;
+use pbs_tools::{json, raise_nofile_limit};
 
 pub mod key;
 pub mod namespace;
-- 
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 2/2] GC: raise nofile soft limit to the hard limit on s3 backed stores

[Christian Ebner]

Since commit 86d5d073 ("GC: fix race with chunk upload/insert on s3
backends"), per-chunk file locks are acquired during phase 2 of
garbage collection for datastores backed by s3 object stores. This
however means that up to 1000 file locks might be held at once, which
can result in the limit of open file handles to be reached.

Therefore, bump the nolimit from the soft to the hard limit.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/datastore.rs | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 0a5179230..ac22c10c5 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -11,6 +11,7 @@ use http_body_util::BodyExt;
 use hyper::body::Bytes;
 use nix::unistd::{unlinkat, UnlinkatFlags};
 use pbs_tools::lru_cache::LruCache;
+use pbs_tools::raise_nofile_limit;
 use tokio::io::AsyncWriteExt;
 use tracing::{info, warn};
 
@@ -1589,6 +1590,12 @@ impl DataStore {
         let s3_client = match self.backend()? {
             DatastoreBackend::Filesystem => None,
             DatastoreBackend::S3(s3_client) => {
+                // required for per-chunk file locks in GC phase 2 on S3 backed stores
+                let old_rlimit =
+                    raise_nofile_limit().context("failed to raise open file handle limit")?;
+                if old_rlimit.rlim_max <= 4096 {
+                    info!("limit for open file handles low: {}", old_rlimit.rlim_max);
+                }
                 proxmox_async::runtime::block_on(s3_client.head_bucket())
                     .context("failed to reach bucket")?;
                 Some(s3_client)
-- 
2.47.3



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