* [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances
@ 2025-09-16 12:41 Christian Ebner
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 1/4] rate-limiter: add crate for traffic rate limiter implementations Christian Ebner
` (9 more replies)
0 siblings, 10 replies; 21+ messages in thread
From: Christian Ebner @ 2025-09-16 12:41 UTC (permalink / raw)
To: pbs-devel
This patches implement a shared bandwidth rate limiter for the s3
client instances, with the goal to allow users to avoid network
congestion by traffic to the s3 backend.
The limiter is shared accorss all the clients using the same s3
endpoint id, setting the limits on client instantiation. To utilize
the pre-existing shared rate limiter implementation from PBS, factor
it out into a dedicated crate, together with the non-shared rate limiter
implementation from proxmox-http.
Expose the rate limit configuration in the s3 endpoint configuration
and provide it to the client's https connector. Expose the settings
also in the advanced options of the s3 endpoint edit/crate window.
Changes since version 1 (thanks @Thomas for feedback):
- Move shared/non-shared rate limiter into dedicated crate
- Use a more generic name for the magic number constant of the mmapped
state file
proxmox:
Christian Ebner (4):
rate-limiter: add crate for traffic rate limiter implementations
http: drop factored out rate limiter implementation
rest-server: optionally depend on factored out shared rate limiter
s3-client: add shared rate limiter via https connector
Cargo.toml | 2 +
proxmox-http/Cargo.toml | 4 +-
proxmox-http/debian/control | 19 +--
proxmox-http/src/client/connector.rs | 3 +-
proxmox-http/src/lib.rs | 5 -
proxmox-http/src/rate_limited_stream.rs | 2 +-
proxmox-rate-limiter/Cargo.toml | 29 ++++
proxmox-rate-limiter/debian/changelog | 5 +
proxmox-rate-limiter/debian/control | 70 ++++++++++
proxmox-rate-limiter/debian/copyright | 18 +++
proxmox-rate-limiter/debian/debcargo.toml | 7 +
proxmox-rate-limiter/src/lib.rs | 13 ++
.../src/rate_limiter.rs | 0
.../src/shared_rate_limiter.rs | 130 ++++++++++++++++++
proxmox-rest-server/Cargo.toml | 2 +
proxmox-rest-server/debian/control | 14 +-
proxmox-rest-server/src/connection.rs | 4 +-
proxmox-s3-client/Cargo.toml | 7 +-
proxmox-s3-client/debian/control | 7 +-
proxmox-s3-client/examples/s3_client.rs | 1 +
proxmox-s3-client/src/api_types.rs | 29 ++++
proxmox-s3-client/src/client.rs | 63 ++++++++-
proxmox-s3-client/src/lib.rs | 4 +-
23 files changed, 399 insertions(+), 39 deletions(-)
create mode 100644 proxmox-rate-limiter/Cargo.toml
create mode 100644 proxmox-rate-limiter/debian/changelog
create mode 100644 proxmox-rate-limiter/debian/control
create mode 100644 proxmox-rate-limiter/debian/copyright
create mode 100644 proxmox-rate-limiter/debian/debcargo.toml
create mode 100644 proxmox-rate-limiter/src/lib.rs
rename {proxmox-http => proxmox-rate-limiter}/src/rate_limiter.rs (100%)
create mode 100644 proxmox-rate-limiter/src/shared_rate_limiter.rs
proxmox-backup:
Christian Ebner (4):
traffic control: use factored out shared rate limiter
api: config: update s3 endpoint rate limits in config
datastore: s3: set rate limiter options for s3 client
ui: expose rate and burst limits for s3 endpoints
Cargo.toml | 3 +
debian/control | 1 -
pbs-client/Cargo.toml | 3 +-
pbs-client/src/http_client.rs | 3 +-
pbs-datastore/src/datastore.rs | 18 ++++-
src/api2/admin/s3.rs | 9 ++-
src/api2/config/s3.rs | 34 ++++++++-
src/tools/mod.rs | 4 -
src/tools/shared_rate_limiter.rs | 122 -------------------------------
src/traffic_control_cache.rs | 8 +-
www/window/S3ClientEdit.js | 34 +++++++++
11 files changed, 102 insertions(+), 137 deletions(-)
delete mode 100644 src/tools/shared_rate_limiter.rs
Summary over all repositories:
34 files changed, 501 insertions(+), 176 deletions(-)
--
Generated by git-murpp 0.8.1
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [pbs-devel] [PATCH proxmox v2 1/4] rate-limiter: add crate for traffic rate limiter implementations
2025-09-16 12:41 [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Christian Ebner
@ 2025-09-16 12:41 ` Christian Ebner
2025-11-11 10:34 ` Hannes Laimer
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 2/4] http: drop factored out rate limiter implementation Christian Ebner
` (8 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Christian Ebner @ 2025-09-16 12:41 UTC (permalink / raw)
To: pbs-devel
Factors out the traffic rate limiter implementations currently tied
to the proxmox-backup and proxmox-http crates to make them
independent and easily reusable, e.g. for the s3-client
implementation.
The shared rate limiter implementation from PBS relies on mmapping
for state sharing, the file exposed having a predefined magic number.
In order to be backwards compatible, leave the magic number as is and
only adapt the constant name to be more generic, although the string
the magic number is derived from is PBS specific.
Further, the user for file ownership and base path of the mmapped
file are now passed as parameters during shared rate limiter
instantiation.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Changes since version 1:
- move rate limiter implementations into dedicated crate instead of
proxmox-http
- Adapt shared state file magic constant name to be generic
Cargo.toml | 1 +
proxmox-rate-limiter/Cargo.toml | 29 +++
proxmox-rate-limiter/debian/changelog | 5 +
proxmox-rate-limiter/debian/control | 70 ++++++
proxmox-rate-limiter/debian/copyright | 18 ++
proxmox-rate-limiter/debian/debcargo.toml | 7 +
proxmox-rate-limiter/src/lib.rs | 13 ++
proxmox-rate-limiter/src/rate_limiter.rs | 214 ++++++++++++++++++
.../src/shared_rate_limiter.rs | 130 +++++++++++
9 files changed, 487 insertions(+)
create mode 100644 proxmox-rate-limiter/Cargo.toml
create mode 100644 proxmox-rate-limiter/debian/changelog
create mode 100644 proxmox-rate-limiter/debian/control
create mode 100644 proxmox-rate-limiter/debian/copyright
create mode 100644 proxmox-rate-limiter/debian/debcargo.toml
create mode 100644 proxmox-rate-limiter/src/lib.rs
create mode 100644 proxmox-rate-limiter/src/rate_limiter.rs
create mode 100644 proxmox-rate-limiter/src/shared_rate_limiter.rs
diff --git a/Cargo.toml b/Cargo.toml
index f149af65..bde32b17 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -29,6 +29,7 @@ members = [
"proxmox-notify",
"proxmox-openid",
"proxmox-product-config",
+ "proxmox-rate-limiter",
"proxmox-resource-scheduling",
"proxmox-rest-server",
"proxmox-router",
diff --git a/proxmox-rate-limiter/Cargo.toml b/proxmox-rate-limiter/Cargo.toml
new file mode 100644
index 00000000..6d8d96cc
--- /dev/null
+++ b/proxmox-rate-limiter/Cargo.toml
@@ -0,0 +1,29 @@
+[package]
+name = "proxmox-rate-limiter"
+description = "Token bucket based traffic rate limiter implementation"
+version = "1.0.0"
+
+authors.workspace = true
+edition.workspace = true
+exclude.workspace = true
+homepage.workspace = true
+license.workspace = true
+repository.workspace = true
+rust-version.workspace = true
+
+[dependencies]
+anyhow.workspace = true
+hyper = { workspace = true, optional = true }
+nix = { workspace = true, optional = true }
+
+proxmox-shared-memory = { workspace = true, optional = true }
+proxmox-sys = { workspace = true, optional = true }
+
+[features]
+default = []
+rate-limiter = ["dep:hyper"]
+shared-rate-limiter = [
+ "dep:nix",
+ "dep:proxmox-shared-memory",
+ "dep:proxmox-sys",
+]
diff --git a/proxmox-rate-limiter/debian/changelog b/proxmox-rate-limiter/debian/changelog
new file mode 100644
index 00000000..0bffa551
--- /dev/null
+++ b/proxmox-rate-limiter/debian/changelog
@@ -0,0 +1,5 @@
+rust-proxmox-rate-limiter (1.0.0-1) bookworm; urgency=medium
+
+ * initial packaging
+
+ -- Proxmox Support Team <support@proxmox.com> Tue, 16 Sep 2025 11:06:23 +0200
diff --git a/proxmox-rate-limiter/debian/control b/proxmox-rate-limiter/debian/control
new file mode 100644
index 00000000..689fe02e
--- /dev/null
+++ b/proxmox-rate-limiter/debian/control
@@ -0,0 +1,70 @@
+Source: rust-proxmox-rate-limiter
+Section: rust
+Priority: optional
+Build-Depends: debhelper-compat (= 13),
+ dh-sequence-cargo
+Build-Depends-Arch: cargo:native <!nocheck>,
+ rustc:native (>= 1.82) <!nocheck>,
+ libstd-rust-dev <!nocheck>,
+ librust-anyhow-1+default-dev <!nocheck>
+Maintainer: Proxmox Support Team <support@proxmox.com>
+Standards-Version: 4.7.0
+Vcs-Git: git://git.proxmox.com/git/proxmox.git
+Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
+Homepage: https://proxmox.com
+X-Cargo-Crate: proxmox-rate-limiter
+Rules-Requires-Root: no
+
+Package: librust-proxmox-rate-limiter-dev
+Architecture: any
+Multi-Arch: same
+Depends:
+ ${misc:Depends},
+ librust-anyhow-1+default-dev
+Suggests:
+ librust-proxmox-rate-limiter+rate-limiter-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter+shared-rate-limiter-dev (= ${binary:Version})
+Provides:
+ librust-proxmox-rate-limiter+default-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1+default-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0+default-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0.0-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0.0+default-dev (= ${binary:Version})
+Description: Token bucket based traffic rate limiter implementation - Rust source code
+ Source code for Debianized Rust crate "proxmox-rate-limiter"
+
+Package: librust-proxmox-rate-limiter+rate-limiter-dev
+Architecture: any
+Multi-Arch: same
+Depends:
+ ${misc:Depends},
+ librust-proxmox-rate-limiter-dev (= ${binary:Version}),
+ librust-hyper-1+default-dev
+Provides:
+ librust-proxmox-rate-limiter-1+rate-limiter-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0+rate-limiter-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0.0+rate-limiter-dev (= ${binary:Version})
+Description: Token bucket based traffic rate limiter implementation - feature "rate-limiter"
+ This metapackage enables feature "rate-limiter" for the Rust proxmox-rate-
+ limiter crate, by pulling in any additional dependencies needed by that
+ feature.
+
+Package: librust-proxmox-rate-limiter+shared-rate-limiter-dev
+Architecture: any
+Multi-Arch: same
+Depends:
+ ${misc:Depends},
+ librust-proxmox-rate-limiter-dev (= ${binary:Version}),
+ librust-nix-0.29+default-dev,
+ librust-proxmox-shared-memory-1+default-dev,
+ librust-proxmox-sys-1+default-dev
+Provides:
+ librust-proxmox-rate-limiter-1+shared-rate-limiter-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0+shared-rate-limiter-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0.0+shared-rate-limiter-dev (= ${binary:Version})
+Description: Token bucket based traffic rate limiter implementation - feature "shared-rate-limiter"
+ This metapackage enables feature "shared-rate-limiter" for the Rust proxmox-
+ rate-limiter crate, by pulling in any additional dependencies needed by that
+ feature.
diff --git a/proxmox-rate-limiter/debian/copyright b/proxmox-rate-limiter/debian/copyright
new file mode 100644
index 00000000..d6e3c304
--- /dev/null
+++ b/proxmox-rate-limiter/debian/copyright
@@ -0,0 +1,18 @@
+Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
+
+Files:
+ *
+Copyright: 2025 Proxmox Server Solutions GmbH <support@proxmox.com>
+License: AGPL-3.0-or-later
+ This program is free software: you can redistribute it and/or modify it under
+ the terms of the GNU Affero General Public License as published by the Free
+ Software Foundation, either version 3 of the License, or (at your option) any
+ later version.
+ .
+ This program is distributed in the hope that it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+ details.
+ .
+ You should have received a copy of the GNU Affero General Public License along
+ with this program. If not, see <https://www.gnu.org/licenses/>.
diff --git a/proxmox-rate-limiter/debian/debcargo.toml b/proxmox-rate-limiter/debian/debcargo.toml
new file mode 100644
index 00000000..b7864cdb
--- /dev/null
+++ b/proxmox-rate-limiter/debian/debcargo.toml
@@ -0,0 +1,7 @@
+overlay = "."
+crate_src_path = ".."
+maintainer = "Proxmox Support Team <support@proxmox.com>"
+
+[source]
+vcs_git = "git://git.proxmox.com/git/proxmox.git"
+vcs_browser = "https://git.proxmox.com/?p=proxmox.git"
diff --git a/proxmox-rate-limiter/src/lib.rs b/proxmox-rate-limiter/src/lib.rs
new file mode 100644
index 00000000..a8d7cfdd
--- /dev/null
+++ b/proxmox-rate-limiter/src/lib.rs
@@ -0,0 +1,13 @@
+//! Token bucket based traffic rate limiter implementations.
+
+#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
+
+#[cfg(feature = "rate-limiter")]
+mod rate_limiter;
+#[cfg(feature = "rate-limiter")]
+pub use rate_limiter::{RateLimit, RateLimiter, RateLimiterVec, ShareableRateLimit};
+
+#[cfg(feature = "shared-rate-limiter")]
+mod shared_rate_limiter;
+#[cfg(feature = "shared-rate-limiter")]
+pub use shared_rate_limiter::SharedRateLimiter;
diff --git a/proxmox-rate-limiter/src/rate_limiter.rs b/proxmox-rate-limiter/src/rate_limiter.rs
new file mode 100644
index 00000000..945c77a6
--- /dev/null
+++ b/proxmox-rate-limiter/src/rate_limiter.rs
@@ -0,0 +1,214 @@
+use std::convert::TryInto;
+use std::time::{Duration, Instant};
+
+use anyhow::{bail, Error};
+
+/// Rate limiter interface.
+pub trait RateLimit {
+ /// Update rate and bucket size
+ fn update_rate(&mut self, rate: u64, bucket_size: u64);
+
+ /// Returns the overall traffic (since started)
+ fn traffic(&self) -> u64;
+
+ /// Register traffic, returning a proposed delay to reach the
+ /// expected rate.
+ fn register_traffic(&mut self, current_time: Instant, data_len: u64) -> Duration;
+}
+
+/// Like [`RateLimit`], but does not require self to be mutable.
+///
+/// This is useful for types providing internal mutability (Mutex).
+pub trait ShareableRateLimit: Send + Sync {
+ fn update_rate(&self, rate: u64, bucket_size: u64);
+ fn traffic(&self) -> u64;
+ fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration;
+}
+
+/// IMPORTANT: We use this struct in shared memory, so please do not
+/// change/modify the layout (do not add fields)
+#[derive(Clone)]
+#[repr(C)]
+struct TbfState {
+ traffic: u64, // overall traffic
+ last_update: Instant,
+ consumed_tokens: u64,
+}
+
+impl TbfState {
+ const NO_DELAY: Duration = Duration::from_millis(0);
+
+ fn refill_bucket(&mut self, rate: u64, current_time: Instant) {
+ let time_diff = match current_time.checked_duration_since(self.last_update) {
+ Some(duration) => duration.as_nanos(),
+ None => return,
+ };
+
+ if time_diff == 0 {
+ return;
+ }
+
+ self.last_update = current_time;
+
+ let allowed_traffic = ((time_diff.saturating_mul(rate as u128)) / 1_000_000_000)
+ .try_into()
+ .unwrap_or(u64::MAX);
+
+ self.consumed_tokens = self.consumed_tokens.saturating_sub(allowed_traffic);
+ }
+
+ fn register_traffic(
+ &mut self,
+ rate: u64,
+ bucket_size: u64,
+ current_time: Instant,
+ data_len: u64,
+ ) -> Duration {
+ self.refill_bucket(rate, current_time);
+
+ self.traffic += data_len;
+ self.consumed_tokens += data_len;
+
+ if self.consumed_tokens <= bucket_size {
+ return Self::NO_DELAY;
+ }
+ Duration::from_nanos(
+ (self.consumed_tokens - bucket_size).saturating_mul(1_000_000_000) / rate,
+ )
+ }
+}
+
+/// Token bucket based rate limiter
+///
+/// IMPORTANT: We use this struct in shared memory, so please do not
+/// change/modify the layout (do not add fields)
+#[repr(C)]
+pub struct RateLimiter {
+ rate: u64, // tokens/second
+ bucket_size: u64, // TBF bucket size
+ state: TbfState,
+}
+
+impl RateLimiter {
+ /// Creates a new instance, using [Instant::now] as start time.
+ pub fn new(rate: u64, bucket_size: u64) -> Self {
+ let start_time = Instant::now();
+ Self::with_start_time(rate, bucket_size, start_time)
+ }
+
+ /// Creates a new instance with specified `rate`, `bucket_size` and `start_time`.
+ pub fn with_start_time(rate: u64, bucket_size: u64, start_time: Instant) -> Self {
+ Self {
+ rate,
+ bucket_size,
+ state: TbfState {
+ traffic: 0,
+ last_update: start_time,
+ // start with empty bucket (all tokens consumed)
+ consumed_tokens: bucket_size,
+ },
+ }
+ }
+}
+
+impl RateLimit for RateLimiter {
+ fn update_rate(&mut self, rate: u64, bucket_size: u64) {
+ self.rate = rate;
+
+ if bucket_size < self.bucket_size && self.state.consumed_tokens > bucket_size {
+ self.state.consumed_tokens = bucket_size; // start again
+ }
+
+ self.bucket_size = bucket_size;
+ }
+
+ fn traffic(&self) -> u64 {
+ self.state.traffic
+ }
+
+ fn register_traffic(&mut self, current_time: Instant, data_len: u64) -> Duration {
+ self.state
+ .register_traffic(self.rate, self.bucket_size, current_time, data_len)
+ }
+}
+
+impl<R: RateLimit + Send> ShareableRateLimit for std::sync::Mutex<R> {
+ fn update_rate(&self, rate: u64, bucket_size: u64) {
+ self.lock().unwrap().update_rate(rate, bucket_size);
+ }
+
+ fn traffic(&self) -> u64 {
+ self.lock().unwrap().traffic()
+ }
+
+ fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration {
+ self.lock()
+ .unwrap()
+ .register_traffic(current_time, data_len)
+ }
+}
+
+/// Array of rate limiters.
+///
+/// A group of rate limiters with same configuration.
+pub struct RateLimiterVec {
+ rate: u64, // tokens/second
+ bucket_size: u64, // TBF bucket size
+ state: Vec<TbfState>,
+}
+
+impl RateLimiterVec {
+ /// Creates a new instance, using [Instant::now] as start time.
+ pub fn new(group_size: usize, rate: u64, bucket_size: u64) -> Self {
+ let start_time = Instant::now();
+ Self::with_start_time(group_size, rate, bucket_size, start_time)
+ }
+
+ /// Creates a new instance with specified `rate`, `bucket_size` and `start_time`.
+ pub fn with_start_time(
+ group_size: usize,
+ rate: u64,
+ bucket_size: u64,
+ start_time: Instant,
+ ) -> Self {
+ let state = TbfState {
+ traffic: 0,
+ last_update: start_time,
+ // start with empty bucket (all tokens consumed)
+ consumed_tokens: bucket_size,
+ };
+ Self {
+ rate,
+ bucket_size,
+ state: vec![state; group_size],
+ }
+ }
+
+ #[allow(clippy::len_without_is_empty)]
+ /// Return the number of TBF entries (group_size)
+ pub fn len(&self) -> usize {
+ self.state.len()
+ }
+
+ /// Traffic for the specified index
+ pub fn traffic(&self, index: usize) -> Result<u64, Error> {
+ if index >= self.state.len() {
+ bail!("RateLimiterVec::traffic - index out of range");
+ }
+ Ok(self.state[index].traffic)
+ }
+
+ /// Register traffic at the specified index
+ pub fn register_traffic(
+ &mut self,
+ index: usize,
+ current_time: Instant,
+ data_len: u64,
+ ) -> Result<Duration, Error> {
+ if index >= self.state.len() {
+ bail!("RateLimiterVec::register_traffic - index out of range");
+ }
+
+ Ok(self.state[index].register_traffic(self.rate, self.bucket_size, current_time, data_len))
+ }
+}
diff --git a/proxmox-rate-limiter/src/shared_rate_limiter.rs b/proxmox-rate-limiter/src/shared_rate_limiter.rs
new file mode 100644
index 00000000..2822e7ea
--- /dev/null
+++ b/proxmox-rate-limiter/src/shared_rate_limiter.rs
@@ -0,0 +1,130 @@
+//! Rate limiter designed for shared memory
+
+use std::mem::MaybeUninit;
+use std::path::Path;
+use std::time::{Duration, Instant};
+
+use anyhow::{bail, Error};
+use nix::sys::stat::Mode;
+use nix::unistd::User;
+
+use proxmox_shared_memory::{check_subtype, initialize_subtype};
+use proxmox_shared_memory::{Init, SharedMemory, SharedMutex};
+use proxmox_sys::fs::{create_path, CreateOptions};
+
+use crate::{RateLimit, RateLimiter, ShareableRateLimit};
+
+/// Magic number for shared rate limiter exposed file mappings
+///
+/// Generated by `openssl::sha::sha256(b"Proxmox Backup SharedRateLimiter v1.0")[0..8];`
+/// Original magic number kept when factored out from the initial
+/// PBS implementation for full backwards compatibility.
+pub const PROXMOX_SHARED_RATE_LIMITER_MAGIC_1_0: [u8; 8] = [6, 58, 213, 96, 161, 122, 130, 117];
+
+// Wrap RateLimiter, so that we can provide an Init impl
+#[repr(C)]
+struct WrapLimiter(RateLimiter);
+
+impl Init for WrapLimiter {
+ fn initialize(this: &mut MaybeUninit<Self>) {
+ // default does not matter here, because we override later
+ this.write(WrapLimiter(RateLimiter::new(1_000_000, 1_000_000)));
+ }
+}
+
+#[repr(C)]
+struct SharedRateLimiterData {
+ magic: [u8; 8],
+ tbf: SharedMutex<WrapLimiter>,
+ padding: [u8; 4096 - 104],
+}
+
+impl Init for SharedRateLimiterData {
+ fn initialize(this: &mut MaybeUninit<Self>) {
+ unsafe {
+ let me = &mut *this.as_mut_ptr();
+ me.magic = PROXMOX_SHARED_RATE_LIMITER_MAGIC_1_0;
+ initialize_subtype(&mut me.tbf);
+ }
+ }
+
+ fn check_type_magic(this: &MaybeUninit<Self>) -> Result<(), Error> {
+ unsafe {
+ let me = &*this.as_ptr();
+ if me.magic != PROXMOX_SHARED_RATE_LIMITER_MAGIC_1_0 {
+ bail!("SharedRateLimiterData: wrong magic number");
+ }
+ check_subtype(&me.tbf)?;
+ Ok(())
+ }
+ }
+}
+
+/// Rate limiter designed for shared memory ([SharedMemory])
+///
+/// The actual [RateLimiter] is protected by a [SharedMutex] and
+/// implements [Init]. This way we can share the limiter between
+/// different processes.
+pub struct SharedRateLimiter {
+ shmem: SharedMemory<SharedRateLimiterData>,
+}
+
+impl SharedRateLimiter {
+ /// Creates a new mmap'ed instance.
+ ///
+ /// Data is mapped in `<base_path>/<name>` using
+ /// `TMPFS`.
+ pub fn mmap_shmem<P: AsRef<Path>>(
+ name: &str,
+ rate: u64,
+ burst: u64,
+ user: User,
+ base_path: P,
+ ) -> Result<Self, Error> {
+ let mut path = base_path.as_ref().to_path_buf();
+
+ let dir_opts = CreateOptions::new()
+ .perm(Mode::from_bits_truncate(0o770))
+ .owner(user.uid)
+ .group(user.gid);
+
+ create_path(&path, Some(dir_opts), Some(dir_opts))?;
+
+ path.push(name);
+
+ let file_opts = CreateOptions::new()
+ .perm(Mode::from_bits_truncate(0o660))
+ .owner(user.uid)
+ .group(user.gid);
+
+ let shmem: SharedMemory<SharedRateLimiterData> = SharedMemory::open(&path, file_opts)?;
+
+ shmem.data().tbf.lock().0.update_rate(rate, burst);
+
+ Ok(Self { shmem })
+ }
+}
+
+impl ShareableRateLimit for SharedRateLimiter {
+ fn update_rate(&self, rate: u64, bucket_size: u64) {
+ self.shmem
+ .data()
+ .tbf
+ .lock()
+ .0
+ .update_rate(rate, bucket_size);
+ }
+
+ fn traffic(&self) -> u64 {
+ self.shmem.data().tbf.lock().0.traffic()
+ }
+
+ fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration {
+ self.shmem
+ .data()
+ .tbf
+ .lock()
+ .0
+ .register_traffic(current_time, data_len)
+ }
+}
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [pbs-devel] [PATCH proxmox v2 2/4] http: drop factored out rate limiter implementation
2025-09-16 12:41 [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Christian Ebner
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 1/4] rate-limiter: add crate for traffic rate limiter implementations Christian Ebner
@ 2025-09-16 12:41 ` Christian Ebner
2025-11-11 10:36 ` Hannes Laimer
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 3/4] rest-server: optionally depend on factored out shared rate limiter Christian Ebner
` (7 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Christian Ebner @ 2025-09-16 12:41 UTC (permalink / raw)
To: pbs-devel
The rate limiter implementation has been moved together with the shared
rate limiter into a dedicated crate. Depend on that and drop the now
dead code from the proxmox-http crate.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Changes since version 1:
- not present in previous version
Cargo.toml | 1 +
proxmox-http/Cargo.toml | 4 +-
proxmox-http/debian/control | 19 +--
proxmox-http/src/client/connector.rs | 3 +-
proxmox-http/src/lib.rs | 5 -
proxmox-http/src/rate_limited_stream.rs | 2 +-
proxmox-http/src/rate_limiter.rs | 214 ------------------------
7 files changed, 8 insertions(+), 240 deletions(-)
delete mode 100644 proxmox-http/src/rate_limiter.rs
diff --git a/Cargo.toml b/Cargo.toml
index bde32b17..f62b9882 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -153,6 +153,7 @@ proxmox-login = { version = "1.0.0", path = "proxmox-login" }
proxmox-network-types = { version = "0.1.0", path = "proxmox-network-types" }
proxmox-product-config = { version = "1.0.0", path = "proxmox-product-config" }
proxmox-config-digest = { version = "1.0.0", path = "proxmox-config-digest" }
+proxmox-rate-limiter = { version = "1.0.0", path = "proxmox-rate-limiter" }
proxmox-rest-server = { version = "1.0.0", path = "proxmox-rest-server" }
proxmox-router = { version = "3.2.2", path = "proxmox-router" }
proxmox-s3-client = { version = "1.2", path = "proxmox-s3-client" }
diff --git a/proxmox-http/Cargo.toml b/proxmox-http/Cargo.toml
index 2f83cf5f..0b362bef 100644
--- a/proxmox-http/Cargo.toml
+++ b/proxmox-http/Cargo.toml
@@ -32,6 +32,7 @@ url = { workspace = true, optional = true }
proxmox-async = { workspace = true, optional = true }
proxmox-base64 = { workspace = true, optional = true }
+proxmox-rate-limiter = { workspace = true, optional = true, features = [ "rate-limiter" ] }
proxmox-sys = { workspace = true, optional = true }
proxmox-io = { workspace = true, optional = true }
proxmox-lang = { workspace = true, optional = true }
@@ -53,7 +54,6 @@ body = [
"dep:sync_wrapper",
"sync_wrapper?/futures",
]
-rate-limiter = ["dep:hyper"]
rate-limited-stream = [
"dep:tokio",
"dep:hyper-util",
@@ -61,7 +61,7 @@ rate-limited-stream = [
"hyper-util?/client-legacy",
"hyper-util?/http1",
"tokio?/time",
- "rate-limiter",
+ "dep:proxmox-rate-limiter",
]
client = [
"dep:bytes",
diff --git a/proxmox-http/debian/control b/proxmox-http/debian/control
index f86e58d1..ae3af111 100644
--- a/proxmox-http/debian/control
+++ b/proxmox-http/debian/control
@@ -29,7 +29,6 @@ Suggests:
librust-proxmox-http+http-helpers-dev (= ${binary:Version}),
librust-proxmox-http+proxmox-async-dev (= ${binary:Version}),
librust-proxmox-http+rate-limited-stream-dev (= ${binary:Version}),
- librust-proxmox-http+rate-limiter-dev (= ${binary:Version}),
librust-proxmox-http+websocket-dev (= ${binary:Version})
Provides:
librust-proxmox-http+default-dev (= ${binary:Version}),
@@ -172,12 +171,13 @@ Multi-Arch: same
Depends:
${misc:Depends},
librust-proxmox-http-dev (= ${binary:Version}),
- librust-proxmox-http+rate-limiter-dev (= ${binary:Version}),
librust-hyper-util-0.1+client-dev (>= 0.1.12-~~),
librust-hyper-util-0.1+client-legacy-dev (>= 0.1.12-~~),
librust-hyper-util-0.1+default-dev (>= 0.1.12-~~),
librust-hyper-util-0.1+http1-dev (>= 0.1.12-~~),
librust-hyper-util-0.1+http2-dev (>= 0.1.12-~~),
+ librust-proxmox-rate-limiter-1+default-dev,
+ librust-proxmox-rate-limiter-1+rate-limiter-dev,
librust-tokio-1+default-dev (>= 1.6-~~),
librust-tokio-1+time-dev (>= 1.6-~~)
Provides:
@@ -188,21 +188,6 @@ Description: Proxmox HTTP library - feature "rate-limited-stream"
This metapackage enables feature "rate-limited-stream" for the Rust proxmox-
http crate, by pulling in any additional dependencies needed by that feature.
-Package: librust-proxmox-http+rate-limiter-dev
-Architecture: any
-Multi-Arch: same
-Depends:
- ${misc:Depends},
- librust-proxmox-http-dev (= ${binary:Version}),
- librust-hyper-1+default-dev
-Provides:
- librust-proxmox-http-1+rate-limiter-dev (= ${binary:Version}),
- librust-proxmox-http-1.0+rate-limiter-dev (= ${binary:Version}),
- librust-proxmox-http-1.0.3+rate-limiter-dev (= ${binary:Version})
-Description: Proxmox HTTP library - feature "rate-limiter"
- This metapackage enables feature "rate-limiter" for the Rust proxmox-http
- crate, by pulling in any additional dependencies needed by that feature.
-
Package: librust-proxmox-http+websocket-dev
Architecture: any
Multi-Arch: same
diff --git a/proxmox-http/src/client/connector.rs b/proxmox-http/src/client/connector.rs
index 1600d47c..d5d85cb9 100644
--- a/proxmox-http/src/client/connector.rs
+++ b/proxmox-http/src/client/connector.rs
@@ -13,13 +13,14 @@ use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
use tokio::net::TcpStream;
use tokio_openssl::SslStream;
+use proxmox_rate_limiter::ShareableRateLimit;
use proxmox_sys::linux::socket::set_tcp_keepalive;
use crate::proxy_config::ProxyConfig;
use crate::uri::build_authority;
use super::tls::MaybeTlsStream;
-use crate::{RateLimitedStream, ShareableRateLimit};
+use crate::RateLimitedStream;
type SharedRateLimit = Arc<dyn ShareableRateLimit>;
diff --git a/proxmox-http/src/lib.rs b/proxmox-http/src/lib.rs
index 8b6953b0..7f6f67f8 100644
--- a/proxmox-http/src/lib.rs
+++ b/proxmox-http/src/lib.rs
@@ -26,11 +26,6 @@ mod client_trait;
#[cfg(feature = "client-trait")]
pub use client_trait::HttpClient;
-#[cfg(feature = "rate-limiter")]
-mod rate_limiter;
-#[cfg(feature = "rate-limiter")]
-pub use rate_limiter::{RateLimit, RateLimiter, RateLimiterVec, ShareableRateLimit};
-
#[cfg(feature = "rate-limited-stream")]
mod rate_limited_stream;
#[cfg(feature = "rate-limited-stream")]
diff --git a/proxmox-http/src/rate_limited_stream.rs b/proxmox-http/src/rate_limited_stream.rs
index e9308a47..2253bef8 100644
--- a/proxmox-http/src/rate_limited_stream.rs
+++ b/proxmox-http/src/rate_limited_stream.rs
@@ -11,7 +11,7 @@ use tokio::time::Sleep;
use std::task::{Context, Poll};
-use super::{RateLimiter, ShareableRateLimit};
+use proxmox_rate_limiter::{RateLimiter, ShareableRateLimit};
type SharedRateLimit = Arc<dyn ShareableRateLimit>;
diff --git a/proxmox-http/src/rate_limiter.rs b/proxmox-http/src/rate_limiter.rs
deleted file mode 100644
index 945c77a6..00000000
--- a/proxmox-http/src/rate_limiter.rs
+++ /dev/null
@@ -1,214 +0,0 @@
-use std::convert::TryInto;
-use std::time::{Duration, Instant};
-
-use anyhow::{bail, Error};
-
-/// Rate limiter interface.
-pub trait RateLimit {
- /// Update rate and bucket size
- fn update_rate(&mut self, rate: u64, bucket_size: u64);
-
- /// Returns the overall traffic (since started)
- fn traffic(&self) -> u64;
-
- /// Register traffic, returning a proposed delay to reach the
- /// expected rate.
- fn register_traffic(&mut self, current_time: Instant, data_len: u64) -> Duration;
-}
-
-/// Like [`RateLimit`], but does not require self to be mutable.
-///
-/// This is useful for types providing internal mutability (Mutex).
-pub trait ShareableRateLimit: Send + Sync {
- fn update_rate(&self, rate: u64, bucket_size: u64);
- fn traffic(&self) -> u64;
- fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration;
-}
-
-/// IMPORTANT: We use this struct in shared memory, so please do not
-/// change/modify the layout (do not add fields)
-#[derive(Clone)]
-#[repr(C)]
-struct TbfState {
- traffic: u64, // overall traffic
- last_update: Instant,
- consumed_tokens: u64,
-}
-
-impl TbfState {
- const NO_DELAY: Duration = Duration::from_millis(0);
-
- fn refill_bucket(&mut self, rate: u64, current_time: Instant) {
- let time_diff = match current_time.checked_duration_since(self.last_update) {
- Some(duration) => duration.as_nanos(),
- None => return,
- };
-
- if time_diff == 0 {
- return;
- }
-
- self.last_update = current_time;
-
- let allowed_traffic = ((time_diff.saturating_mul(rate as u128)) / 1_000_000_000)
- .try_into()
- .unwrap_or(u64::MAX);
-
- self.consumed_tokens = self.consumed_tokens.saturating_sub(allowed_traffic);
- }
-
- fn register_traffic(
- &mut self,
- rate: u64,
- bucket_size: u64,
- current_time: Instant,
- data_len: u64,
- ) -> Duration {
- self.refill_bucket(rate, current_time);
-
- self.traffic += data_len;
- self.consumed_tokens += data_len;
-
- if self.consumed_tokens <= bucket_size {
- return Self::NO_DELAY;
- }
- Duration::from_nanos(
- (self.consumed_tokens - bucket_size).saturating_mul(1_000_000_000) / rate,
- )
- }
-}
-
-/// Token bucket based rate limiter
-///
-/// IMPORTANT: We use this struct in shared memory, so please do not
-/// change/modify the layout (do not add fields)
-#[repr(C)]
-pub struct RateLimiter {
- rate: u64, // tokens/second
- bucket_size: u64, // TBF bucket size
- state: TbfState,
-}
-
-impl RateLimiter {
- /// Creates a new instance, using [Instant::now] as start time.
- pub fn new(rate: u64, bucket_size: u64) -> Self {
- let start_time = Instant::now();
- Self::with_start_time(rate, bucket_size, start_time)
- }
-
- /// Creates a new instance with specified `rate`, `bucket_size` and `start_time`.
- pub fn with_start_time(rate: u64, bucket_size: u64, start_time: Instant) -> Self {
- Self {
- rate,
- bucket_size,
- state: TbfState {
- traffic: 0,
- last_update: start_time,
- // start with empty bucket (all tokens consumed)
- consumed_tokens: bucket_size,
- },
- }
- }
-}
-
-impl RateLimit for RateLimiter {
- fn update_rate(&mut self, rate: u64, bucket_size: u64) {
- self.rate = rate;
-
- if bucket_size < self.bucket_size && self.state.consumed_tokens > bucket_size {
- self.state.consumed_tokens = bucket_size; // start again
- }
-
- self.bucket_size = bucket_size;
- }
-
- fn traffic(&self) -> u64 {
- self.state.traffic
- }
-
- fn register_traffic(&mut self, current_time: Instant, data_len: u64) -> Duration {
- self.state
- .register_traffic(self.rate, self.bucket_size, current_time, data_len)
- }
-}
-
-impl<R: RateLimit + Send> ShareableRateLimit for std::sync::Mutex<R> {
- fn update_rate(&self, rate: u64, bucket_size: u64) {
- self.lock().unwrap().update_rate(rate, bucket_size);
- }
-
- fn traffic(&self) -> u64 {
- self.lock().unwrap().traffic()
- }
-
- fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration {
- self.lock()
- .unwrap()
- .register_traffic(current_time, data_len)
- }
-}
-
-/// Array of rate limiters.
-///
-/// A group of rate limiters with same configuration.
-pub struct RateLimiterVec {
- rate: u64, // tokens/second
- bucket_size: u64, // TBF bucket size
- state: Vec<TbfState>,
-}
-
-impl RateLimiterVec {
- /// Creates a new instance, using [Instant::now] as start time.
- pub fn new(group_size: usize, rate: u64, bucket_size: u64) -> Self {
- let start_time = Instant::now();
- Self::with_start_time(group_size, rate, bucket_size, start_time)
- }
-
- /// Creates a new instance with specified `rate`, `bucket_size` and `start_time`.
- pub fn with_start_time(
- group_size: usize,
- rate: u64,
- bucket_size: u64,
- start_time: Instant,
- ) -> Self {
- let state = TbfState {
- traffic: 0,
- last_update: start_time,
- // start with empty bucket (all tokens consumed)
- consumed_tokens: bucket_size,
- };
- Self {
- rate,
- bucket_size,
- state: vec![state; group_size],
- }
- }
-
- #[allow(clippy::len_without_is_empty)]
- /// Return the number of TBF entries (group_size)
- pub fn len(&self) -> usize {
- self.state.len()
- }
-
- /// Traffic for the specified index
- pub fn traffic(&self, index: usize) -> Result<u64, Error> {
- if index >= self.state.len() {
- bail!("RateLimiterVec::traffic - index out of range");
- }
- Ok(self.state[index].traffic)
- }
-
- /// Register traffic at the specified index
- pub fn register_traffic(
- &mut self,
- index: usize,
- current_time: Instant,
- data_len: u64,
- ) -> Result<Duration, Error> {
- if index >= self.state.len() {
- bail!("RateLimiterVec::register_traffic - index out of range");
- }
-
- Ok(self.state[index].register_traffic(self.rate, self.bucket_size, current_time, data_len))
- }
-}
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [pbs-devel] [PATCH proxmox v2 3/4] rest-server: optionally depend on factored out shared rate limiter
2025-09-16 12:41 [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Christian Ebner
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 1/4] rate-limiter: add crate for traffic rate limiter implementations Christian Ebner
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 2/4] http: drop factored out rate limiter implementation Christian Ebner
@ 2025-09-16 12:41 ` Christian Ebner
2025-11-11 10:42 ` Hannes Laimer
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 4/4] s3-client: add shared rate limiter via https connector Christian Ebner
` (6 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Christian Ebner @ 2025-09-16 12:41 UTC (permalink / raw)
To: pbs-devel
The rate limiter implementation has been moved to a dedicated crate,
so depend on it for the rest servers rate limited stream connection.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Changes since version 1:
- not present in previous version
proxmox-rest-server/Cargo.toml | 2 ++
proxmox-rest-server/debian/control | 14 ++++++++------
proxmox-rest-server/src/connection.rs | 4 +++-
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/proxmox-rest-server/Cargo.toml b/proxmox-rest-server/Cargo.toml
index 81d93ca1..7478f6a8 100644
--- a/proxmox-rest-server/Cargo.toml
+++ b/proxmox-rest-server/Cargo.toml
@@ -44,6 +44,7 @@ proxmox-daemon.workspace = true
proxmox-http = { workspace = true, features = ["body"] }
proxmox-lang.workspace = true
proxmox-log.workspace = true
+proxmox-rate-limiter = { workspace = true, optional = true, features = [ "shared-rate-limiter" ] }
proxmox-router.workspace = true
proxmox-schema = { workspace = true, features = [ "api-macro", "upid-api-impl" ] }
proxmox-sys = { workspace = true, features = [ "logrotate", "timer" ] }
@@ -54,5 +55,6 @@ proxmox-worker-task.workspace = true
default = []
templates = ["dep:handlebars"]
rate-limited-stream = [
+ "dep:proxmox-rate-limiter",
"proxmox-http/rate-limited-stream",
]
diff --git a/proxmox-rest-server/debian/control b/proxmox-rest-server/debian/control
index 263ca7ea..ee04bd3f 100644
--- a/proxmox-rest-server/debian/control
+++ b/proxmox-rest-server/debian/control
@@ -29,8 +29,8 @@ Build-Depends-Arch: cargo:native <!nocheck>,
librust-proxmox-async-0.5+default-dev <!nocheck>,
librust-proxmox-compression-1+default-dev <!nocheck>,
librust-proxmox-daemon-1+default-dev <!nocheck>,
- librust-proxmox-http-1+body-dev (>= 1.0.2-~~) <!nocheck>,
- librust-proxmox-http-1+default-dev (>= 1.0.2-~~) <!nocheck>,
+ librust-proxmox-http-1+body-dev (>= 1.0.3-~~) <!nocheck>,
+ librust-proxmox-http-1+default-dev (>= 1.0.3-~~) <!nocheck>,
librust-proxmox-lang-1+default-dev (>= 1.5-~~) <!nocheck>,
librust-proxmox-log-1+default-dev <!nocheck>,
librust-proxmox-router-3+default-dev (>= 3.2.2-~~) <!nocheck>,
@@ -91,8 +91,8 @@ Depends:
librust-proxmox-async-0.5+default-dev,
librust-proxmox-compression-1+default-dev,
librust-proxmox-daemon-1+default-dev,
- librust-proxmox-http-1+body-dev (>= 1.0.2-~~),
- librust-proxmox-http-1+default-dev (>= 1.0.2-~~),
+ librust-proxmox-http-1+body-dev (>= 1.0.3-~~),
+ librust-proxmox-http-1+default-dev (>= 1.0.3-~~),
librust-proxmox-lang-1+default-dev (>= 1.5-~~),
librust-proxmox-log-1+default-dev,
librust-proxmox-router-3+default-dev (>= 3.2.2-~~),
@@ -137,8 +137,10 @@ Multi-Arch: same
Depends:
${misc:Depends},
librust-proxmox-rest-server-dev (= ${binary:Version}),
- librust-proxmox-http-1+body-dev (>= 1.0.2-~~),
- librust-proxmox-http-1+rate-limited-stream-dev (>= 1.0.2-~~)
+ librust-proxmox-http-1+body-dev (>= 1.0.3-~~),
+ librust-proxmox-http-1+rate-limited-stream-dev (>= 1.0.3-~~),
+ librust-proxmox-rate-limiter-1+default-dev,
+ librust-proxmox-rate-limiter-1+shared-rate-limiter-dev
Provides:
librust-proxmox-rest-server-1+rate-limited-stream-dev (= ${binary:Version}),
librust-proxmox-rest-server-1.0+rate-limited-stream-dev (= ${binary:Version}),
diff --git a/proxmox-rest-server/src/connection.rs b/proxmox-rest-server/src/connection.rs
index 9511b7cb..6ffaac73 100644
--- a/proxmox-rest-server/src/connection.rs
+++ b/proxmox-rest-server/src/connection.rs
@@ -24,7 +24,9 @@ use tokio_openssl::SslStream;
use tokio_stream::wrappers::ReceiverStream;
#[cfg(feature = "rate-limited-stream")]
-use proxmox_http::{RateLimitedStream, ShareableRateLimit};
+use proxmox_http::RateLimitedStream;
+#[cfg(feature = "rate-limited-stream")]
+use proxmox_rate_limiter::ShareableRateLimit;
#[cfg(feature = "rate-limited-stream")]
pub type SharedRateLimit = Arc<dyn ShareableRateLimit>;
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [pbs-devel] [PATCH proxmox v2 4/4] s3-client: add shared rate limiter via https connector
2025-09-16 12:41 [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Christian Ebner
` (2 preceding siblings ...)
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 3/4] rest-server: optionally depend on factored out shared rate limiter Christian Ebner
@ 2025-09-16 12:41 ` Christian Ebner
2025-11-11 10:41 ` Hannes Laimer
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 1/4] traffic control: use factored out shared rate limiter Christian Ebner
` (5 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Christian Ebner @ 2025-09-16 12:41 UTC (permalink / raw)
To: pbs-devel
Allows to configure a shared rate limiter for the s3 client to limit
upload and download bandwidth. This will help users which suffer from
issues due to network congestion by the unlimited s3 client data
transfers.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Changes since version 1:
- depend on proxmox-rate-limiter
proxmox-s3-client/Cargo.toml | 7 ++-
proxmox-s3-client/debian/control | 7 ++-
proxmox-s3-client/examples/s3_client.rs | 1 +
proxmox-s3-client/src/api_types.rs | 29 ++++++++++++
proxmox-s3-client/src/client.rs | 63 +++++++++++++++++++++++--
proxmox-s3-client/src/lib.rs | 4 +-
6 files changed, 105 insertions(+), 6 deletions(-)
diff --git a/proxmox-s3-client/Cargo.toml b/proxmox-s3-client/Cargo.toml
index 639ae26b..be24c4de 100644
--- a/proxmox-s3-client/Cargo.toml
+++ b/proxmox-s3-client/Cargo.toml
@@ -22,6 +22,7 @@ hyper-util = { workspace = true, features = [ "client-legacy", "tokio", "http1"
hyper = { workspace = true, optional = true }
iso8601 = { workspace = true, optional = true }
md5 = { workspace = true, optional = true }
+nix = { workspace = true, optional = true }
openssl = { workspace = true, optional = true }
quick-xml = { workspace = true, features = [ "async-tokio" ], optional = true }
regex.workspace = true
@@ -34,7 +35,9 @@ tracing = { workspace = true, optional = true }
url = {workspace = true, optional = true }
proxmox-base64 = { workspace = true, optional = true }
-proxmox-http = { workspace = true, features = [ "body", "client", "client-trait", "rate-limiter" ], optional = true }
+proxmox-http = { workspace = true, features = [ "body", "client", "client-trait" ], optional = true }
+proxmox-human-byte.workspace = true
+proxmox-rate-limiter = { workspace = true, features = [ "rate-limiter", "shared-rate-limiter" ], optional = true }
proxmox-schema = { workspace = true, features = [ "api-macro", "api-types" ] }
proxmox-serde.workspace = true
proxmox-time = {workspace = true, optional = true }
@@ -51,6 +54,7 @@ impl = [
"dep:hyper",
"dep:iso8601",
"dep:md5",
+ "dep:nix",
"dep:openssl",
"dep:quick-xml",
"dep:serde-xml-rs",
@@ -60,6 +64,7 @@ impl = [
"dep:url",
"dep:proxmox-base64",
"dep:proxmox-http",
+ "dep:proxmox-rate-limiter",
"dep:proxmox-time",
]
diff --git a/proxmox-s3-client/debian/control b/proxmox-s3-client/debian/control
index c9147749..24bcbeba 100644
--- a/proxmox-s3-client/debian/control
+++ b/proxmox-s3-client/debian/control
@@ -8,6 +8,7 @@ Build-Depends-Arch: cargo:native <!nocheck>,
libstd-rust-dev <!nocheck>,
librust-anyhow-1+default-dev <!nocheck>,
librust-const-format-0.2+default-dev <!nocheck>,
+ librust-proxmox-human-byte-1+default-dev <!nocheck>,
librust-proxmox-schema-5+api-macro-dev <!nocheck>,
librust-proxmox-schema-5+api-types-dev <!nocheck>,
librust-proxmox-schema-5+default-dev <!nocheck>,
@@ -31,6 +32,7 @@ Depends:
${misc:Depends},
librust-anyhow-1+default-dev,
librust-const-format-0.2+default-dev,
+ librust-proxmox-human-byte-1+default-dev,
librust-proxmox-schema-5+api-macro-dev,
librust-proxmox-schema-5+api-types-dev,
librust-proxmox-schema-5+default-dev,
@@ -74,13 +76,16 @@ Depends:
librust-hyper-util-0.1+tokio-dev (>= 0.1.12-~~),
librust-iso8601-0.6+default-dev (>= 0.6.1-~~),
librust-md5-0.7+default-dev,
+ librust-nix-0.29+default-dev,
librust-openssl-0.10+default-dev,
librust-proxmox-base64-1+default-dev,
librust-proxmox-http-1+body-dev (>= 1.0.3-~~),
librust-proxmox-http-1+client-dev (>= 1.0.3-~~),
librust-proxmox-http-1+client-trait-dev (>= 1.0.3-~~),
librust-proxmox-http-1+default-dev (>= 1.0.3-~~),
- librust-proxmox-http-1+rate-limiter-dev (>= 1.0.3-~~),
+ librust-proxmox-rate-limiter-1+default-dev,
+ librust-proxmox-rate-limiter-1+rate-limiter-dev,
+ librust-proxmox-rate-limiter-1+shared-rate-limiter-dev,
librust-proxmox-time-2+default-dev (>= 2.1.0-~~),
librust-quick-xml-0.36+async-tokio-dev (>= 0.36.1-~~),
librust-quick-xml-0.36+default-dev (>= 0.36.1-~~),
diff --git a/proxmox-s3-client/examples/s3_client.rs b/proxmox-s3-client/examples/s3_client.rs
index 67baf467..dd3885d7 100644
--- a/proxmox-s3-client/examples/s3_client.rs
+++ b/proxmox-s3-client/examples/s3_client.rs
@@ -39,6 +39,7 @@ async fn run() -> Result<(), anyhow::Error> {
fingerprint: Some("<s3-api-fingerprint>".to_string()),
put_rate_limit: None,
provider_quirks: Vec::new(),
+ rate_limiter_config: None,
};
// Creating a client instance and connect to api endpoint
diff --git a/proxmox-s3-client/src/api_types.rs b/proxmox-s3-client/src/api_types.rs
index 115b1d2d..9071868a 100644
--- a/proxmox-s3-client/src/api_types.rs
+++ b/proxmox-s3-client/src/api_types.rs
@@ -2,6 +2,7 @@ use anyhow::bail;
use const_format::concatcp;
use serde::{Deserialize, Serialize};
+use proxmox_human_byte::HumanByte;
use proxmox_schema::api_types::{
CERT_FINGERPRINT_SHA256_SCHEMA, DNS_LABEL_STR, IPRE_STR, SAFE_ID_FORMAT,
};
@@ -126,6 +127,22 @@ serde_plain::derive_fromstr_from_deserialize!(ProviderQuirks);
type: ProviderQuirks,
},
},
+ "rate-in": {
+ type: HumanByte,
+ optional: true,
+ },
+ "burst-in": {
+ type: HumanByte,
+ optional: true,
+ },
+ "rate-out": {
+ type: HumanByte,
+ optional: true,
+ },
+ "burst-out": {
+ type: HumanByte,
+ optional: true,
+ },
},
)]
#[derive(Serialize, Deserialize, Updater, Clone, PartialEq)]
@@ -154,6 +171,18 @@ pub struct S3ClientConfig {
/// List of provider specific feature implementation quirks.
#[serde(skip_serializing_if = "Option::is_none")]
pub provider_quirks: Option<Vec<ProviderQuirks>>,
+ /// Download rate limit.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub rate_in: Option<HumanByte>,
+ /// Download burst.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub burst_in: Option<HumanByte>,
+ /// Upload rate limit.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub rate_out: Option<HumanByte>,
+ /// Upload burst
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub burst_out: Option<HumanByte>,
}
impl S3ClientConfig {
diff --git a/proxmox-s3-client/src/client.rs b/proxmox-s3-client/src/client.rs
index 96a5878d..e50f6d1d 100644
--- a/proxmox-s3-client/src/client.rs
+++ b/proxmox-s3-client/src/client.rs
@@ -1,4 +1,4 @@
-use std::path::Path;
+use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::sync::{Arc, Mutex};
use std::time::{Duration, Instant};
@@ -12,6 +12,7 @@ use hyper::{Request, Response};
use hyper_util::client::legacy::connect::HttpConnector;
use hyper_util::client::legacy::Client;
use hyper_util::rt::TokioExecutor;
+use nix::unistd::User;
use openssl::hash::MessageDigest;
use openssl::sha::Sha256;
use openssl::ssl::{SslConnector, SslMethod, SslVerifyMode};
@@ -19,7 +20,8 @@ use openssl::x509::X509StoreContextRef;
use tracing::error;
use proxmox_http::client::HttpsConnector;
-use proxmox_http::{Body, RateLimit, RateLimiter};
+use proxmox_http::Body;
+use proxmox_rate_limiter::{RateLimit, RateLimiter, SharedRateLimiter};
use proxmox_schema::api_types::CERT_FINGERPRINT_SHA256_SCHEMA;
use crate::api_types::{ProviderQuirks, S3ClientConfig};
@@ -53,6 +55,25 @@ pub enum S3PathPrefix {
None,
}
+/// Options for the https connector's rate limiter
+pub struct S3RateLimiterOptions {
+ /// ID for the shared rate limiter.
+ pub id: String,
+ /// Base path for the shared memory mapped file
+ pub base_path: PathBuf,
+ /// User for the to be created shared memory mapped file and folders
+ pub user: User,
+}
+
+/// Configuration for the https connector's rate limiter
+pub struct S3RateLimiterConfig {
+ options: S3RateLimiterOptions,
+ rate_in: Option<u64>,
+ burst_in: Option<u64>,
+ rate_out: Option<u64>,
+ burst_out: Option<u64>,
+}
+
/// Configuration options for client
pub struct S3ClientOptions {
/// Endpoint to access S3 object store.
@@ -77,6 +98,8 @@ pub struct S3ClientOptions {
pub put_rate_limit: Option<u64>,
/// Provider implementation specific features and limitations
pub provider_quirks: Vec<ProviderQuirks>,
+ /// Configuration options for the shared rate limiter.
+ pub rate_limiter_config: Option<S3RateLimiterConfig>,
}
impl S3ClientOptions {
@@ -86,7 +109,15 @@ impl S3ClientOptions {
secret_key: String,
bucket: Option<String>,
common_prefix: String,
+ rate_limiter_options: Option<S3RateLimiterOptions>,
) -> Self {
+ let rate_limiter_config = rate_limiter_options.map(|options| S3RateLimiterConfig {
+ options,
+ rate_in: config.rate_in.map(|human_bytes| human_bytes.as_u64()),
+ burst_in: config.burst_in.map(|human_bytes| human_bytes.as_u64()),
+ rate_out: config.rate_out.map(|human_bytes| human_bytes.as_u64()),
+ burst_out: config.burst_out.map(|human_bytes| human_bytes.as_u64()),
+ });
Self {
endpoint: config.endpoint,
port: config.port,
@@ -99,6 +130,7 @@ impl S3ClientOptions {
secret_key,
put_rate_limit: config.put_rate_limit,
provider_quirks: config.provider_quirks.unwrap_or_default(),
+ rate_limiter_config,
}
}
}
@@ -151,11 +183,36 @@ impl S3Client {
// want communication to object store backend api to always use https
http_connector.enforce_http(false);
http_connector.set_connect_timeout(Some(S3_HTTP_CONNECT_TIMEOUT));
- let https_connector = HttpsConnector::with_connector(
+ let mut https_connector = HttpsConnector::with_connector(
http_connector,
ssl_connector_builder.build(),
S3_TCP_KEEPIDLE_TIME,
);
+
+ if let Some(limiter_config) = &options.rate_limiter_config {
+ if let Some(limit) = limiter_config.rate_in {
+ let limiter = SharedRateLimiter::mmap_shmem(
+ &format!("{}.in", limiter_config.options.id),
+ limit,
+ limiter_config.burst_in.unwrap_or(limit),
+ limiter_config.options.user.clone(),
+ limiter_config.options.base_path.clone(),
+ )?;
+ https_connector.set_read_limiter(Some(Arc::new(limiter)));
+ }
+
+ if let Some(limit) = limiter_config.rate_out {
+ let limiter = SharedRateLimiter::mmap_shmem(
+ &format!("{}.out", limiter_config.options.id),
+ limit,
+ limiter_config.burst_out.unwrap_or(limit),
+ limiter_config.options.user.clone(),
+ limiter_config.options.base_path.clone(),
+ )?;
+ https_connector.set_write_limiter(Some(Arc::new(limiter)));
+ }
+ }
+
let client = Client::builder(TokioExecutor::new()).build::<_, Body>(https_connector);
let authority_template = if let Some(port) = options.port {
diff --git a/proxmox-s3-client/src/lib.rs b/proxmox-s3-client/src/lib.rs
index 26e7032b..d02fd0dc 100644
--- a/proxmox-s3-client/src/lib.rs
+++ b/proxmox-s3-client/src/lib.rs
@@ -20,7 +20,9 @@ pub use aws_sign_v4::uri_decode;
#[cfg(feature = "impl")]
mod client;
#[cfg(feature = "impl")]
-pub use client::{S3Client, S3ClientOptions, S3PathPrefix, S3_HTTP_REQUEST_TIMEOUT};
+pub use client::{
+ S3Client, S3ClientOptions, S3PathPrefix, S3RateLimiterOptions, S3_HTTP_REQUEST_TIMEOUT,
+};
#[cfg(feature = "impl")]
mod timestamps;
#[cfg(feature = "impl")]
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v2 1/4] traffic control: use factored out shared rate limiter
2025-09-16 12:41 [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Christian Ebner
` (3 preceding siblings ...)
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 4/4] s3-client: add shared rate limiter via https connector Christian Ebner
@ 2025-09-16 12:41 ` Christian Ebner
2025-11-11 10:45 ` Hannes Laimer
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 2/4] api: config: update s3 endpoint rate limits in config Christian Ebner
` (4 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Christian Ebner @ 2025-09-16 12:41 UTC (permalink / raw)
To: pbs-devel
The shared rate limiter implementation was factored out into a
dedicated crate so it can be easily reused for other code paths, e.g.
the s3 client implementation.
Use that implementation and drop the now dead code.
No functional changes intended.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Changes since version 1:
- depend on proxmox-rate-limiter
Cargo.toml | 3 +
debian/control | 1 -
pbs-client/Cargo.toml | 3 +-
pbs-client/src/http_client.rs | 3 +-
src/tools/mod.rs | 4 -
src/tools/shared_rate_limiter.rs | 122 -------------------------------
src/traffic_control_cache.rs | 8 +-
7 files changed, 11 insertions(+), 133 deletions(-)
delete mode 100644 src/tools/shared_rate_limiter.rs
diff --git a/Cargo.toml b/Cargo.toml
index b3f55b4db..e276c11de 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -74,6 +74,7 @@ proxmox-network-api = "1"
proxmox-notify = "1"
proxmox-openid = "1"
proxmox-product-config = "1"
+proxmox-rate-limiter = "1.0.0"
proxmox-rest-server = { version = "1.0.1", features = [ "templates" ] }
# some use "cli", some use "cli" and "server", pbs-config uses nothing
proxmox-router = { version = "3.2.2", default-features = false }
@@ -227,6 +228,7 @@ proxmox-network-api = { workspace = true, features = [ "impl" ] }
proxmox-notify = { workspace = true, features = [ "pbs-context" ] }
proxmox-openid.workspace = true
proxmox-product-config.workspace = true
+proxmox-rate-limiter = { workspace = true, features = [ "shared-rate-limiter" ] }
proxmox-rest-server = { workspace = true, features = [ "rate-limited-stream" ] }
proxmox-router = { workspace = true, features = [ "cli", "server"] }
proxmox-s3-client.workspace = true
@@ -288,6 +290,7 @@ proxmox-rrd-api-types.workspace = true
#proxmox-notify = { path = "../proxmox/proxmox-notify" }
#proxmox-openid = { path = "../proxmox/proxmox-openid" }
#proxmox-product-config = { path = "../proxmox/proxmox-product-config" }
+#proxmox-rate-limiter = { path = "../proxmox/proxmox-rate-limiter" }
#proxmox-rest-server = { path = "../proxmox/proxmox-rest-server" }
#proxmox-router = { path = "../proxmox/proxmox-router" }
#proxmox-rrd = { path = "../proxmox/proxmox-rrd" }
diff --git a/debian/control b/debian/control
index 4346acd38..58ea000c6 100644
--- a/debian/control
+++ b/debian/control
@@ -81,7 +81,6 @@ Build-Depends: bash-completion,
librust-proxmox-http-1+http-helpers-dev (>= 1.0.2-~~),
librust-proxmox-http-1+proxmox-async-dev (>= 1.0.2-~~),
librust-proxmox-http-1+rate-limited-stream-dev (>= 1.0.2-~~),
- librust-proxmox-http-1+rate-limiter-dev (>= 1.0.2-~~),
librust-proxmox-http-1+websocket-dev (>= 1.0.2-~~),
librust-proxmox-human-byte-1+default-dev,
librust-proxmox-io-1+default-dev (>= 1.0.1-~~),
diff --git a/pbs-client/Cargo.toml b/pbs-client/Cargo.toml
index 4c6c80983..233969f65 100644
--- a/pbs-client/Cargo.toml
+++ b/pbs-client/Cargo.toml
@@ -36,10 +36,11 @@ pathpatterns.workspace = true
proxmox-async.workspace = true
proxmox-auth-api.workspace = true
proxmox-compression.workspace = true
-proxmox-http = { workspace = true, features = [ "body", "rate-limiter" ] }
+proxmox-http = { workspace = true, features = [ "body" ] }
proxmox-human-byte.workspace = true
proxmox-io = { workspace = true, features = [ "tokio" ] }
proxmox-log = { workspace = true }
+proxmox-rate-limiter = { workspace = true, features = [ "rate-limiter" ] }
proxmox-router = { workspace = true, features = [ "cli", "server" ] }
proxmox-schema.workspace = true
proxmox-sys.workspace = true
diff --git a/pbs-client/src/http_client.rs b/pbs-client/src/http_client.rs
index 74b076244..3cdae2c01 100644
--- a/pbs-client/src/http_client.rs
+++ b/pbs-client/src/http_client.rs
@@ -31,8 +31,9 @@ use proxmox_async::broadcast_future::BroadcastFuture;
use proxmox_http::client::HttpsConnector;
use proxmox_http::uri::{build_authority, json_object_to_query};
use proxmox_http::Body;
-use proxmox_http::{ProxyConfig, RateLimiter};
+use proxmox_http::ProxyConfig;
use proxmox_log::{error, info, warn};
+use proxmox_rate_limiter::RateLimiter;
use pbs_api_types::percent_encoding::DEFAULT_ENCODE_SET;
use pbs_api_types::{Authid, RateLimitConfig, Userid};
diff --git a/src/tools/mod.rs b/src/tools/mod.rs
index 6556effe3..a97aba5b6 100644
--- a/src/tools/mod.rs
+++ b/src/tools/mod.rs
@@ -16,10 +16,6 @@ use pbs_datastore::manifest::BackupManifest;
pub mod config;
pub mod disks;
pub mod fs;
-
-mod shared_rate_limiter;
-pub use shared_rate_limiter::SharedRateLimiter;
-
pub mod statistics;
pub mod systemd;
pub mod ticket;
diff --git a/src/tools/shared_rate_limiter.rs b/src/tools/shared_rate_limiter.rs
deleted file mode 100644
index db754728b..000000000
--- a/src/tools/shared_rate_limiter.rs
+++ /dev/null
@@ -1,122 +0,0 @@
-use std::mem::MaybeUninit;
-use std::path::PathBuf;
-use std::time::{Duration, Instant};
-
-use anyhow::{bail, Error};
-use nix::sys::stat::Mode;
-
-use proxmox_sys::fs::{create_path, CreateOptions};
-
-use proxmox_http::{RateLimit, RateLimiter, ShareableRateLimit};
-use proxmox_shared_memory::{check_subtype, initialize_subtype};
-use proxmox_shared_memory::{Init, SharedMemory, SharedMutex};
-
-// openssl::sha::sha256(b"Proxmox Backup SharedRateLimiter v1.0")[0..8];
-pub const PROXMOX_BACKUP_SHARED_RATE_LIMITER_MAGIC_1_0: [u8; 8] =
- [6, 58, 213, 96, 161, 122, 130, 117];
-
-const BASE_PATH: &str = pbs_buildcfg::rundir!("/shmem/tbf");
-
-// Wrap RateLimiter, so that we can provide an Init impl
-#[repr(C)]
-struct WrapLimiter(RateLimiter);
-
-impl Init for WrapLimiter {
- fn initialize(this: &mut MaybeUninit<Self>) {
- // default does not matter here, because we override later
- this.write(WrapLimiter(RateLimiter::new(1_000_000, 1_000_000)));
- }
-}
-
-#[repr(C)]
-struct SharedRateLimiterData {
- magic: [u8; 8],
- tbf: SharedMutex<WrapLimiter>,
- padding: [u8; 4096 - 104],
-}
-
-impl Init for SharedRateLimiterData {
- fn initialize(this: &mut MaybeUninit<Self>) {
- unsafe {
- let me = &mut *this.as_mut_ptr();
- me.magic = PROXMOX_BACKUP_SHARED_RATE_LIMITER_MAGIC_1_0;
- initialize_subtype(&mut me.tbf);
- }
- }
-
- fn check_type_magic(this: &MaybeUninit<Self>) -> Result<(), Error> {
- unsafe {
- let me = &*this.as_ptr();
- if me.magic != PROXMOX_BACKUP_SHARED_RATE_LIMITER_MAGIC_1_0 {
- bail!("SharedRateLimiterData: wrong magic number");
- }
- check_subtype(&me.tbf)?;
- Ok(())
- }
- }
-}
-
-/// Rate limiter designed for shared memory ([SharedMemory])
-///
-/// The actual [RateLimiter] is protected by a [SharedMutex] and
-/// implements [Init]. This way we can share the limiter between
-/// different processes.
-pub struct SharedRateLimiter {
- shmem: SharedMemory<SharedRateLimiterData>,
-}
-
-impl SharedRateLimiter {
- /// Creates a new mmap'ed instance.
- ///
- /// Data is mapped in `/var/run/proxmox-backup/shmem/tbf/<name>` using
- /// `TMPFS`.
- pub fn mmap_shmem(name: &str, rate: u64, burst: u64) -> Result<Self, Error> {
- let mut path = PathBuf::from(BASE_PATH);
-
- let user = pbs_config::backup_user()?;
-
- let dir_opts = CreateOptions::new()
- .perm(Mode::from_bits_truncate(0o770))
- .owner(user.uid)
- .group(user.gid);
-
- create_path(&path, Some(dir_opts), Some(dir_opts))?;
-
- path.push(name);
-
- let file_opts = CreateOptions::new()
- .perm(Mode::from_bits_truncate(0o660))
- .owner(user.uid)
- .group(user.gid);
-
- let shmem: SharedMemory<SharedRateLimiterData> = SharedMemory::open(&path, file_opts)?;
-
- shmem.data().tbf.lock().0.update_rate(rate, burst);
-
- Ok(Self { shmem })
- }
-}
-
-impl ShareableRateLimit for SharedRateLimiter {
- fn update_rate(&self, rate: u64, bucket_size: u64) {
- self.shmem
- .data()
- .tbf
- .lock()
- .0
- .update_rate(rate, bucket_size);
- }
-
- fn traffic(&self) -> u64 {
- self.shmem.data().tbf.lock().0.traffic()
- }
-
- fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration {
- self.shmem
- .data()
- .tbf
- .lock()
- .0
- .register_traffic(current_time, data_len)
- }
-}
diff --git a/src/traffic_control_cache.rs b/src/traffic_control_cache.rs
index 830a8c043..bc52d37ee 100644
--- a/src/traffic_control_cache.rs
+++ b/src/traffic_control_cache.rs
@@ -8,7 +8,7 @@ use std::time::Instant;
use anyhow::Error;
use cidr::IpInet;
-use proxmox_http::{RateLimiter, ShareableRateLimit};
+use proxmox_rate_limiter::{RateLimiter, ShareableRateLimit, SharedRateLimiter};
use proxmox_section_config::SectionConfigData;
use proxmox_time::{parse_daily_duration, DailyDuration, TmEditor};
@@ -17,8 +17,6 @@ use pbs_api_types::TrafficControlRule;
use pbs_config::ConfigVersionCache;
-use crate::tools::SharedRateLimiter;
-
pub type SharedRateLimit = Arc<dyn ShareableRateLimit>;
/// Shared traffic control cache singleton.
@@ -110,7 +108,9 @@ fn create_limiter(
burst: u64,
) -> Result<SharedRateLimit, Error> {
if use_shared_memory {
- let limiter = SharedRateLimiter::mmap_shmem(name, rate, burst)?;
+ let user = pbs_config::backup_user()?;
+ let base_path = pbs_buildcfg::rundir!("/shmem/tbf");
+ let limiter = SharedRateLimiter::mmap_shmem(name, rate, burst, user, base_path)?;
Ok(Arc::new(limiter))
} else {
Ok(Arc::new(Mutex::new(RateLimiter::new(rate, burst))))
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v2 2/4] api: config: update s3 endpoint rate limits in config
2025-09-16 12:41 [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Christian Ebner
` (4 preceding siblings ...)
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 1/4] traffic control: use factored out shared rate limiter Christian Ebner
@ 2025-09-16 12:41 ` Christian Ebner
2025-11-11 10:45 ` Hannes Laimer
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 3/4] datastore: s3: set rate limiter options for s3 client Christian Ebner
` (3 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Christian Ebner @ 2025-09-16 12:41 UTC (permalink / raw)
To: pbs-devel
Set the rate/burst limits in the s3 endpoint configuration whenever
the limits changed.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Changes since version 1:
- no changes
src/api2/config/s3.rs | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/src/api2/config/s3.rs b/src/api2/config/s3.rs
index 8270e8d52..1e421114c 100644
--- a/src/api2/config/s3.rs
+++ b/src/api2/config/s3.rs
@@ -139,6 +139,14 @@ pub enum DeletableProperty {
Fingerprint,
/// Delete the path-style property.
PathStyle,
+ /// Delete the rate-in property.
+ RateIn,
+ /// Delete the burst-in property.
+ BurstIn,
+ /// Delete the rate-out property.
+ RateOut,
+ /// Delete the burst-out property.
+ BurstOut,
/// Delete the provider quirks property.
ProviderQuirks,
}
@@ -213,6 +221,18 @@ pub fn update_s3_client_config(
DeletableProperty::PathStyle => {
data.config.path_style = None;
}
+ DeletableProperty::RateIn => {
+ data.config.rate_in = None;
+ }
+ DeletableProperty::BurstIn => {
+ data.config.burst_in = None;
+ }
+ DeletableProperty::RateOut => {
+ data.config.rate_out = None;
+ }
+ DeletableProperty::BurstOut => {
+ data.config.burst_out = None;
+ }
DeletableProperty::ProviderQuirks => {
data.config.provider_quirks = None;
}
@@ -238,6 +258,18 @@ pub fn update_s3_client_config(
if let Some(path_style) = update.path_style {
data.config.path_style = Some(path_style);
}
+ if let Some(rate_in) = update.rate_in {
+ data.config.rate_in = Some(rate_in);
+ }
+ if let Some(burst_in) = update.burst_in {
+ data.config.burst_in = Some(burst_in);
+ }
+ if let Some(rate_out) = update.rate_out {
+ data.config.rate_out = Some(rate_out);
+ }
+ if let Some(burst_out) = update.burst_out {
+ data.config.burst_out = Some(burst_out);
+ }
if let Some(provider_quirks) = update.provider_quirks {
data.config.provider_quirks = Some(provider_quirks);
}
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v2 3/4] datastore: s3: set rate limiter options for s3 client
2025-09-16 12:41 [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Christian Ebner
` (5 preceding siblings ...)
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 2/4] api: config: update s3 endpoint rate limits in config Christian Ebner
@ 2025-09-16 12:41 ` Christian Ebner
2025-11-11 10:46 ` Hannes Laimer
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 4/4] ui: expose rate and burst limits for s3 endpoints Christian Ebner
` (2 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Christian Ebner @ 2025-09-16 12:41 UTC (permalink / raw)
To: pbs-devel
Set the shared rate limiter for each client instance based on the
endpoint configuration. The same limits are shared for each s3
endpoint. To avoid possibly id clashing with rate limits set via
traffic control, use the base directory `<rundir>/s3/shmem/tbf`
instead of the traffic control's `<rundir>/shmem/tbf`.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Changes since version 1:
- no changes
pbs-datastore/src/datastore.rs | 18 +++++++++++++++++-
src/api2/admin/s3.rs | 9 +++++++--
src/api2/config/s3.rs | 2 +-
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 7cf020fc0..e7cc76dc7 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -14,7 +14,9 @@ use tokio::io::AsyncWriteExt;
use tracing::{info, warn};
use proxmox_human_byte::HumanByte;
-use proxmox_s3_client::{S3Client, S3ClientConf, S3ClientOptions, S3ObjectKey, S3PathPrefix};
+use proxmox_s3_client::{
+ S3Client, S3ClientConf, S3ClientOptions, S3ObjectKey, S3PathPrefix, S3RateLimiterOptions,
+};
use proxmox_schema::ApiType;
use proxmox_sys::error::SysError;
@@ -55,6 +57,7 @@ pub const GROUP_NOTES_FILE_NAME: &str = "notes";
pub const GROUP_OWNER_FILE_NAME: &str = "owner";
/// Filename for in-use marker stored on S3 object store backend
pub const S3_DATASTORE_IN_USE_MARKER: &str = ".in-use";
+const S3_CLIENT_RATE_LIMITER_BASE_PATH: &str = pbs_buildcfg::rundir!("/s3/shmem/tbf");
const NAMESPACE_MARKER_FILENAME: &str = ".namespace";
/// checks if auth_id is owner, or, if owner is a token, if
@@ -254,12 +257,18 @@ impl DataStore {
let (config, _config_digest) = pbs_config::s3::config()?;
let config: S3ClientConf = config.lookup(S3_CFG_TYPE_ID, s3_client_id)?;
+ let rate_limiter_options = S3RateLimiterOptions {
+ id: s3_client_id.to_string(),
+ user: pbs_config::backup_user()?,
+ base_path: S3_CLIENT_RATE_LIMITER_BASE_PATH.into(),
+ };
let options = S3ClientOptions::from_config(
config.config,
config.secret_key,
Some(bucket),
self.name().to_owned(),
+ Some(rate_limiter_options),
);
let s3_client = S3Client::new(options)?;
DatastoreBackend::S3(Arc::new(s3_client))
@@ -2432,11 +2441,18 @@ impl DataStore {
let client_config: S3ClientConf = config
.lookup(S3_CFG_TYPE_ID, s3_client_id)
.with_context(|| format!("no '{s3_client_id}' in config"))?;
+ let rate_limiter_options = S3RateLimiterOptions {
+ id: s3_client_id.to_string(),
+ user: pbs_config::backup_user()?,
+ base_path: S3_CLIENT_RATE_LIMITER_BASE_PATH.into(),
+ };
+
let options = S3ClientOptions::from_config(
client_config.config,
client_config.secret_key,
Some(bucket),
datastore_config.name.to_owned(),
+ Some(rate_limiter_options),
);
let s3_client = S3Client::new(options)
.context("failed to create s3 client")
diff --git a/src/api2/admin/s3.rs b/src/api2/admin/s3.rs
index 73f3779a5..73388281b 100644
--- a/src/api2/admin/s3.rs
+++ b/src/api2/admin/s3.rs
@@ -49,8 +49,13 @@ pub async fn check(
.context("config lookup failed")?;
let store_prefix = store_prefix.unwrap_or_default();
- let options =
- S3ClientOptions::from_config(config.config, config.secret_key, Some(bucket), store_prefix);
+ let options = S3ClientOptions::from_config(
+ config.config,
+ config.secret_key,
+ Some(bucket),
+ store_prefix,
+ None,
+ );
let test_object_key =
S3ObjectKey::try_from(".s3-client-test").context("failed to generate s3 object key")?;
diff --git a/src/api2/config/s3.rs b/src/api2/config/s3.rs
index 1e421114c..27b3c4cc2 100644
--- a/src/api2/config/s3.rs
+++ b/src/api2/config/s3.rs
@@ -351,7 +351,7 @@ pub async fn list_buckets(
let empty_prefix = String::new();
let options =
- S3ClientOptions::from_config(config.config, config.secret_key, None, empty_prefix);
+ S3ClientOptions::from_config(config.config, config.secret_key, None, empty_prefix, None);
let client = S3Client::new(options).context("client creation failed")?;
let list_buckets_response = client
.list_buckets()
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v2 4/4] ui: expose rate and burst limits for s3 endpoints
2025-09-16 12:41 [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Christian Ebner
` (6 preceding siblings ...)
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 3/4] datastore: s3: set rate limiter options for s3 client Christian Ebner
@ 2025-09-16 12:41 ` Christian Ebner
2025-11-11 10:46 ` Hannes Laimer
2025-11-11 10:49 ` [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Hannes Laimer
2025-11-12 11:55 ` [pbs-devel] superseded: " Christian Ebner
9 siblings, 1 reply; 21+ messages in thread
From: Christian Ebner @ 2025-09-16 12:41 UTC (permalink / raw)
To: pbs-devel
Allows to configure the s3 endpoints rate limits via the advanced
options.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Changes since version 1:
- no changes
www/window/S3ClientEdit.js | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/www/window/S3ClientEdit.js b/www/window/S3ClientEdit.js
index 269b35967..8862683b8 100644
--- a/www/window/S3ClientEdit.js
+++ b/www/window/S3ClientEdit.js
@@ -121,6 +121,20 @@ Ext.define('PBS.window.S3ClientEdit', {
],
advancedColumn1: [
+ {
+ xtype: 'pmxBandwidthField',
+ name: 'rate-in',
+ fieldLabel: gettext('Rate In'),
+ emptyText: gettext('Unlimited'),
+ submitAutoScaledSizeUnit: true,
+ },
+ {
+ xtype: 'pmxBandwidthField',
+ name: 'rate-out',
+ fieldLabel: gettext('Rate Out'),
+ emptyText: gettext('Unlimited'),
+ submitAutoScaledSizeUnit: true,
+ },
{
xtype: 'proxmoxKVComboBox',
name: 'provider-quirks',
@@ -136,6 +150,22 @@ Ext.define('PBS.window.S3ClientEdit', {
},
},
],
+ advancedColumn2: [
+ {
+ xtype: 'pmxBandwidthField',
+ name: 'burst-in',
+ fieldLabel: gettext('Burst In'),
+ emptyText: gettext('Same as Rate'),
+ submitAutoScaledSizeUnit: true,
+ },
+ {
+ xtype: 'pmxBandwidthField',
+ name: 'burst-out',
+ fieldLabel: gettext('Burst Out'),
+ emptyText: gettext('Same as Rate'),
+ submitAutoScaledSizeUnit: true,
+ },
+ ],
},
getValues: function () {
@@ -146,6 +176,10 @@ Ext.define('PBS.window.S3ClientEdit', {
values.delete = values.delete.split(',');
}
PBS.Utils.delete_if_default(values, 'path-style', false, me.isCreate);
+ PBS.Utils.delete_if_default(values, 'rate-in', undefined, me.isCreate);
+ PBS.Utils.delete_if_default(values, 'burst-in', undefined, me.isCreate);
+ PBS.Utils.delete_if_default(values, 'rate-out', undefined, me.isCreate);
+ PBS.Utils.delete_if_default(values, 'burst-out', undefined, me.isCreate);
let https_scheme_prefix = 'https://';
if (values.endpoint.startsWith(https_scheme_prefix)) {
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pbs-devel] [PATCH proxmox v2 1/4] rate-limiter: add crate for traffic rate limiter implementations
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 1/4] rate-limiter: add crate for traffic rate limiter implementations Christian Ebner
@ 2025-11-11 10:34 ` Hannes Laimer
2025-11-11 15:06 ` Christian Ebner
0 siblings, 1 reply; 21+ messages in thread
From: Hannes Laimer @ 2025-11-11 10:34 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Christian Ebner
comment inline, other than that consider this
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
On 9/16/25 14:42, Christian Ebner wrote:
> Factors out the traffic rate limiter implementations currently tied
> to the proxmox-backup and proxmox-http crates to make them
> independent and easily reusable, e.g. for the s3-client
> implementation.
>
> The shared rate limiter implementation from PBS relies on mmapping
> for state sharing, the file exposed having a predefined magic number.
> In order to be backwards compatible, leave the magic number as is and
> only adapt the constant name to be more generic, although the string
> the magic number is derived from is PBS specific.
>
> Further, the user for file ownership and base path of the mmapped
> file are now passed as parameters during shared rate limiter
> instantiation.
>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Changes since version 1:
> - move rate limiter implementations into dedicated crate instead of
> proxmox-http
> - Adapt shared state file magic constant name to be generic
>
> Cargo.toml | 1 +
> proxmox-rate-limiter/Cargo.toml | 29 +++
> proxmox-rate-limiter/debian/changelog | 5 +
> proxmox-rate-limiter/debian/control | 70 ++++++
> proxmox-rate-limiter/debian/copyright | 18 ++
> proxmox-rate-limiter/debian/debcargo.toml | 7 +
> proxmox-rate-limiter/src/lib.rs | 13 ++
> proxmox-rate-limiter/src/rate_limiter.rs | 214 ++++++++++++++++++
> .../src/shared_rate_limiter.rs | 130 +++++++++++
> 9 files changed, 487 insertions(+)
> create mode 100644 proxmox-rate-limiter/Cargo.toml
> create mode 100644 proxmox-rate-limiter/debian/changelog
> create mode 100644 proxmox-rate-limiter/debian/control
> create mode 100644 proxmox-rate-limiter/debian/copyright
> create mode 100644 proxmox-rate-limiter/debian/debcargo.toml
> create mode 100644 proxmox-rate-limiter/src/lib.rs
> create mode 100644 proxmox-rate-limiter/src/rate_limiter.rs
> create mode 100644 proxmox-rate-limiter/src/shared_rate_limiter.rs
>
> diff --git a/Cargo.toml b/Cargo.toml
> index f149af65..bde32b17 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -29,6 +29,7 @@ members = [
> "proxmox-notify",
> "proxmox-openid",
> "proxmox-product-config",
> + "proxmox-rate-limiter",
> "proxmox-resource-scheduling",
> "proxmox-rest-server",
> "proxmox-router",
> diff --git a/proxmox-rate-limiter/Cargo.toml b/proxmox-rate-limiter/Cargo.toml
> new file mode 100644
> index 00000000..6d8d96cc
> --- /dev/null
> +++ b/proxmox-rate-limiter/Cargo.toml
> @@ -0,0 +1,29 @@
> +[package]
> +name = "proxmox-rate-limiter"
> +description = "Token bucket based traffic rate limiter implementation"
> +version = "1.0.0"
> +
> +authors.workspace = true
> +edition.workspace = true
> +exclude.workspace = true
> +homepage.workspace = true
> +license.workspace = true
> +repository.workspace = true
> +rust-version.workspace = true
> +
> +[dependencies]
> +anyhow.workspace = true
> +hyper = { workspace = true, optional = true }
> +nix = { workspace = true, optional = true }
> +
> +proxmox-shared-memory = { workspace = true, optional = true }
> +proxmox-sys = { workspace = true, optional = true }
> +
> +[features]
> +default = []
> +rate-limiter = ["dep:hyper"]
> +shared-rate-limiter = [
> + "dep:nix",
> + "dep:proxmox-shared-memory",
> + "dep:proxmox-sys",
> +]
> diff --git a/proxmox-rate-limiter/debian/changelog b/proxmox-rate-limiter/debian/changelog
> new file mode 100644
> index 00000000..0bffa551
> --- /dev/null
> +++ b/proxmox-rate-limiter/debian/changelog
> @@ -0,0 +1,5 @@
> +rust-proxmox-rate-limiter (1.0.0-1) bookworm; urgency=medium
> +
> + * initial packaging
> +
> + -- Proxmox Support Team <support@proxmox.com> Tue, 16 Sep 2025 11:06:23 +0200
> diff --git a/proxmox-rate-limiter/debian/control b/proxmox-rate-limiter/debian/control
> new file mode 100644
> index 00000000..689fe02e
> --- /dev/null
> +++ b/proxmox-rate-limiter/debian/control
> @@ -0,0 +1,70 @@
> +Source: rust-proxmox-rate-limiter
> +Section: rust
> +Priority: optional
> +Build-Depends: debhelper-compat (= 13),
> + dh-sequence-cargo
> +Build-Depends-Arch: cargo:native <!nocheck>,
> + rustc:native (>= 1.82) <!nocheck>,
> + libstd-rust-dev <!nocheck>,
> + librust-anyhow-1+default-dev <!nocheck>
> +Maintainer: Proxmox Support Team <support@proxmox.com>
> +Standards-Version: 4.7.0
> +Vcs-Git: git://git.proxmox.com/git/proxmox.git
> +Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
> +Homepage: https://proxmox.com
> +X-Cargo-Crate: proxmox-rate-limiter
> +Rules-Requires-Root: no
> +
> +Package: librust-proxmox-rate-limiter-dev
> +Architecture: any
> +Multi-Arch: same
> +Depends:
> + ${misc:Depends},
> + librust-anyhow-1+default-dev
> +Suggests:
> + librust-proxmox-rate-limiter+rate-limiter-dev (= ${binary:Version}),
> + librust-proxmox-rate-limiter+shared-rate-limiter-dev (= ${binary:Version})
> +Provides:
> + librust-proxmox-rate-limiter+default-dev (= ${binary:Version}),
> + librust-proxmox-rate-limiter-1-dev (= ${binary:Version}),
> + librust-proxmox-rate-limiter-1+default-dev (= ${binary:Version}),
> + librust-proxmox-rate-limiter-1.0-dev (= ${binary:Version}),
> + librust-proxmox-rate-limiter-1.0+default-dev (= ${binary:Version}),
> + librust-proxmox-rate-limiter-1.0.0-dev (= ${binary:Version}),
> + librust-proxmox-rate-limiter-1.0.0+default-dev (= ${binary:Version})
> +Description: Token bucket based traffic rate limiter implementation - Rust source code
> + Source code for Debianized Rust crate "proxmox-rate-limiter"
> +
> +Package: librust-proxmox-rate-limiter+rate-limiter-dev
> +Architecture: any
> +Multi-Arch: same
> +Depends:
> + ${misc:Depends},
> + librust-proxmox-rate-limiter-dev (= ${binary:Version}),
> + librust-hyper-1+default-dev
> +Provides:
> + librust-proxmox-rate-limiter-1+rate-limiter-dev (= ${binary:Version}),
> + librust-proxmox-rate-limiter-1.0+rate-limiter-dev (= ${binary:Version}),
> + librust-proxmox-rate-limiter-1.0.0+rate-limiter-dev (= ${binary:Version})
> +Description: Token bucket based traffic rate limiter implementation - feature "rate-limiter"
> + This metapackage enables feature "rate-limiter" for the Rust proxmox-rate-
> + limiter crate, by pulling in any additional dependencies needed by that
> + feature.
> +
> +Package: librust-proxmox-rate-limiter+shared-rate-limiter-dev
> +Architecture: any
> +Multi-Arch: same
> +Depends:
> + ${misc:Depends},
> + librust-proxmox-rate-limiter-dev (= ${binary:Version}),
> + librust-nix-0.29+default-dev,
> + librust-proxmox-shared-memory-1+default-dev,
> + librust-proxmox-sys-1+default-dev
> +Provides:
> + librust-proxmox-rate-limiter-1+shared-rate-limiter-dev (= ${binary:Version}),
> + librust-proxmox-rate-limiter-1.0+shared-rate-limiter-dev (= ${binary:Version}),
> + librust-proxmox-rate-limiter-1.0.0+shared-rate-limiter-dev (= ${binary:Version})
> +Description: Token bucket based traffic rate limiter implementation - feature "shared-rate-limiter"
> + This metapackage enables feature "shared-rate-limiter" for the Rust proxmox-
> + rate-limiter crate, by pulling in any additional dependencies needed by that
> + feature.
> diff --git a/proxmox-rate-limiter/debian/copyright b/proxmox-rate-limiter/debian/copyright
> new file mode 100644
> index 00000000..d6e3c304
> --- /dev/null
> +++ b/proxmox-rate-limiter/debian/copyright
> @@ -0,0 +1,18 @@
> +Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
> +
> +Files:
> + *
> +Copyright: 2025 Proxmox Server Solutions GmbH <support@proxmox.com>
> +License: AGPL-3.0-or-later
> + This program is free software: you can redistribute it and/or modify it under
> + the terms of the GNU Affero General Public License as published by the Free
> + Software Foundation, either version 3 of the License, or (at your option) any
> + later version.
> + .
> + This program is distributed in the hope that it will be useful, but WITHOUT
> + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> + FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
> + details.
> + .
> + You should have received a copy of the GNU Affero General Public License along
> + with this program. If not, see <https://www.gnu.org/licenses/>.
> diff --git a/proxmox-rate-limiter/debian/debcargo.toml b/proxmox-rate-limiter/debian/debcargo.toml
> new file mode 100644
> index 00000000..b7864cdb
> --- /dev/null
> +++ b/proxmox-rate-limiter/debian/debcargo.toml
> @@ -0,0 +1,7 @@
> +overlay = "."
> +crate_src_path = ".."
> +maintainer = "Proxmox Support Team <support@proxmox.com>"
> +
> +[source]
> +vcs_git = "git://git.proxmox.com/git/proxmox.git"
> +vcs_browser = "https://git.proxmox.com/?p=proxmox.git"
> diff --git a/proxmox-rate-limiter/src/lib.rs b/proxmox-rate-limiter/src/lib.rs
> new file mode 100644
> index 00000000..a8d7cfdd
> --- /dev/null
> +++ b/proxmox-rate-limiter/src/lib.rs
> @@ -0,0 +1,13 @@
> +//! Token bucket based traffic rate limiter implementations.
> +
> +#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
> +
> +#[cfg(feature = "rate-limiter")]
> +mod rate_limiter;
> +#[cfg(feature = "rate-limiter")]
> +pub use rate_limiter::{RateLimit, RateLimiter, RateLimiterVec, ShareableRateLimit};
> +
> +#[cfg(feature = "shared-rate-limiter")]
> +mod shared_rate_limiter;
> +#[cfg(feature = "shared-rate-limiter")]
> +pub use shared_rate_limiter::SharedRateLimiter;
> diff --git a/proxmox-rate-limiter/src/rate_limiter.rs b/proxmox-rate-limiter/src/rate_limiter.rs
> new file mode 100644
> index 00000000..945c77a6
> --- /dev/null
> +++ b/proxmox-rate-limiter/src/rate_limiter.rs
> @@ -0,0 +1,214 @@
> +use std::convert::TryInto;
> +use std::time::{Duration, Instant};
> +
> +use anyhow::{bail, Error};
> +
> +/// Rate limiter interface.
> +pub trait RateLimit {
> + /// Update rate and bucket size
> + fn update_rate(&mut self, rate: u64, bucket_size: u64);
> +
> + /// Returns the overall traffic (since started)
> + fn traffic(&self) -> u64;
> +
> + /// Register traffic, returning a proposed delay to reach the
> + /// expected rate.
> + fn register_traffic(&mut self, current_time: Instant, data_len: u64) -> Duration;
> +}
> +
> +/// Like [`RateLimit`], but does not require self to be mutable.
> +///
> +/// This is useful for types providing internal mutability (Mutex).
> +pub trait ShareableRateLimit: Send + Sync {
> + fn update_rate(&self, rate: u64, bucket_size: u64);
> + fn traffic(&self) -> u64;
> + fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration;
> +}
> +
> +/// IMPORTANT: We use this struct in shared memory, so please do not
> +/// change/modify the layout (do not add fields)
> +#[derive(Clone)]
> +#[repr(C)]
> +struct TbfState {
> + traffic: u64, // overall traffic
> + last_update: Instant,
> + consumed_tokens: u64,
> +}
> +
> +impl TbfState {
> + const NO_DELAY: Duration = Duration::from_millis(0);
> +
> + fn refill_bucket(&mut self, rate: u64, current_time: Instant) {
> + let time_diff = match current_time.checked_duration_since(self.last_update) {
> + Some(duration) => duration.as_nanos(),
> + None => return,
> + };
> +
> + if time_diff == 0 {
> + return;
> + }
> +
> + self.last_update = current_time;
> +
> + let allowed_traffic = ((time_diff.saturating_mul(rate as u128)) / 1_000_000_000)
> + .try_into()
> + .unwrap_or(u64::MAX);
> +
> + self.consumed_tokens = self.consumed_tokens.saturating_sub(allowed_traffic);
> + }
> +
> + fn register_traffic(
> + &mut self,
> + rate: u64,
> + bucket_size: u64,
> + current_time: Instant,
> + data_len: u64,
> + ) -> Duration {
> + self.refill_bucket(rate, current_time);
> +
> + self.traffic += data_len;
> + self.consumed_tokens += data_len;
> +
> + if self.consumed_tokens <= bucket_size {
> + return Self::NO_DELAY;
> + }
> + Duration::from_nanos(
> + (self.consumed_tokens - bucket_size).saturating_mul(1_000_000_000) / rate,
the UI doesn't let you set 0, but if one edits the config directly the
`... / rate` is a problem. The scenario is a little far fetched, but
someone might quickly want to test something and just assume 0 means
unlimited and just set it to that instead of removing the line.
Given that it is not super easy to run into this `.min(1)` is probably
enough here. Not sure if checking for 0 in the endpoint when setting the
limit makes sense.
> + )
> + }
> +}
> +
> +/// Token bucket based rate limiter
> +///
> +/// IMPORTANT: We use this struct in shared memory, so please do not
> +/// change/modify the layout (do not add fields)
> +#[repr(C)]
> +pub struct RateLimiter {
> + rate: u64, // tokens/second
> + bucket_size: u64, // TBF bucket size
> + state: TbfState,
> +}
> +
> +impl RateLimiter {
> + /// Creates a new instance, using [Instant::now] as start time.
> + pub fn new(rate: u64, bucket_size: u64) -> Self {
> + let start_time = Instant::now();
> + Self::with_start_time(rate, bucket_size, start_time)
> + }
> +
> + /// Creates a new instance with specified `rate`, `bucket_size` and `start_time`.
> + pub fn with_start_time(rate: u64, bucket_size: u64, start_time: Instant) -> Self {
> + Self {
> + rate,
> + bucket_size,
> + state: TbfState {
> + traffic: 0,
> + last_update: start_time,
> + // start with empty bucket (all tokens consumed)
> + consumed_tokens: bucket_size,
> + },
> + }
> + }
> +}
> +
> +impl RateLimit for RateLimiter {
> + fn update_rate(&mut self, rate: u64, bucket_size: u64) {
> + self.rate = rate;
> +
> + if bucket_size < self.bucket_size && self.state.consumed_tokens > bucket_size {
> + self.state.consumed_tokens = bucket_size; // start again
> + }
> +
> + self.bucket_size = bucket_size;
> + }
> +
> + fn traffic(&self) -> u64 {
> + self.state.traffic
> + }
> +
> + fn register_traffic(&mut self, current_time: Instant, data_len: u64) -> Duration {
> + self.state
> + .register_traffic(self.rate, self.bucket_size, current_time, data_len)
> + }
> +}
> +
> +impl<R: RateLimit + Send> ShareableRateLimit for std::sync::Mutex<R> {
> + fn update_rate(&self, rate: u64, bucket_size: u64) {
> + self.lock().unwrap().update_rate(rate, bucket_size);
> + }
> +
> + fn traffic(&self) -> u64 {
> + self.lock().unwrap().traffic()
> + }
> +
> + fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration {
> + self.lock()
> + .unwrap()
> + .register_traffic(current_time, data_len)
> + }
> +}
> +
> +/// Array of rate limiters.
> +///
> +/// A group of rate limiters with same configuration.
> +pub struct RateLimiterVec {
> + rate: u64, // tokens/second
> + bucket_size: u64, // TBF bucket size
> + state: Vec<TbfState>,
> +}
> +
> +impl RateLimiterVec {
> + /// Creates a new instance, using [Instant::now] as start time.
> + pub fn new(group_size: usize, rate: u64, bucket_size: u64) -> Self {
> + let start_time = Instant::now();
> + Self::with_start_time(group_size, rate, bucket_size, start_time)
> + }
> +
> + /// Creates a new instance with specified `rate`, `bucket_size` and `start_time`.
> + pub fn with_start_time(
> + group_size: usize,
> + rate: u64,
> + bucket_size: u64,
> + start_time: Instant,
> + ) -> Self {
> + let state = TbfState {
> + traffic: 0,
> + last_update: start_time,
> + // start with empty bucket (all tokens consumed)
> + consumed_tokens: bucket_size,
> + };
> + Self {
> + rate,
> + bucket_size,
> + state: vec![state; group_size],
> + }
> + }
> +
> + #[allow(clippy::len_without_is_empty)]
> + /// Return the number of TBF entries (group_size)
> + pub fn len(&self) -> usize {
> + self.state.len()
> + }
> +
> + /// Traffic for the specified index
> + pub fn traffic(&self, index: usize) -> Result<u64, Error> {
> + if index >= self.state.len() {
> + bail!("RateLimiterVec::traffic - index out of range");
> + }
> + Ok(self.state[index].traffic)
> + }
> +
> + /// Register traffic at the specified index
> + pub fn register_traffic(
> + &mut self,
> + index: usize,
> + current_time: Instant,
> + data_len: u64,
> + ) -> Result<Duration, Error> {
> + if index >= self.state.len() {
> + bail!("RateLimiterVec::register_traffic - index out of range");
> + }
> +
> + Ok(self.state[index].register_traffic(self.rate, self.bucket_size, current_time, data_len))
> + }
> +}
> diff --git a/proxmox-rate-limiter/src/shared_rate_limiter.rs b/proxmox-rate-limiter/src/shared_rate_limiter.rs
> new file mode 100644
> index 00000000..2822e7ea
> --- /dev/null
> +++ b/proxmox-rate-limiter/src/shared_rate_limiter.rs
> @@ -0,0 +1,130 @@
> +//! Rate limiter designed for shared memory
> +
> +use std::mem::MaybeUninit;
> +use std::path::Path;
> +use std::time::{Duration, Instant};
> +
> +use anyhow::{bail, Error};
> +use nix::sys::stat::Mode;
> +use nix::unistd::User;
> +
> +use proxmox_shared_memory::{check_subtype, initialize_subtype};
> +use proxmox_shared_memory::{Init, SharedMemory, SharedMutex};
> +use proxmox_sys::fs::{create_path, CreateOptions};
> +
> +use crate::{RateLimit, RateLimiter, ShareableRateLimit};
> +
> +/// Magic number for shared rate limiter exposed file mappings
> +///
> +/// Generated by `openssl::sha::sha256(b"Proxmox Backup SharedRateLimiter v1.0")[0..8];`
> +/// Original magic number kept when factored out from the initial
> +/// PBS implementation for full backwards compatibility.
> +pub const PROXMOX_SHARED_RATE_LIMITER_MAGIC_1_0: [u8; 8] = [6, 58, 213, 96, 161, 122, 130, 117];
> +
> +// Wrap RateLimiter, so that we can provide an Init impl
> +#[repr(C)]
> +struct WrapLimiter(RateLimiter);
> +
> +impl Init for WrapLimiter {
> + fn initialize(this: &mut MaybeUninit<Self>) {
> + // default does not matter here, because we override later
> + this.write(WrapLimiter(RateLimiter::new(1_000_000, 1_000_000)));
> + }
> +}
> +
> +#[repr(C)]
> +struct SharedRateLimiterData {
> + magic: [u8; 8],
> + tbf: SharedMutex<WrapLimiter>,
> + padding: [u8; 4096 - 104],
> +}
> +
> +impl Init for SharedRateLimiterData {
> + fn initialize(this: &mut MaybeUninit<Self>) {
> + unsafe {
> + let me = &mut *this.as_mut_ptr();
> + me.magic = PROXMOX_SHARED_RATE_LIMITER_MAGIC_1_0;
> + initialize_subtype(&mut me.tbf);
> + }
> + }
> +
> + fn check_type_magic(this: &MaybeUninit<Self>) -> Result<(), Error> {
> + unsafe {
> + let me = &*this.as_ptr();
> + if me.magic != PROXMOX_SHARED_RATE_LIMITER_MAGIC_1_0 {
> + bail!("SharedRateLimiterData: wrong magic number");
> + }
> + check_subtype(&me.tbf)?;
> + Ok(())
> + }
> + }
> +}
> +
> +/// Rate limiter designed for shared memory ([SharedMemory])
> +///
> +/// The actual [RateLimiter] is protected by a [SharedMutex] and
> +/// implements [Init]. This way we can share the limiter between
> +/// different processes.
> +pub struct SharedRateLimiter {
> + shmem: SharedMemory<SharedRateLimiterData>,
> +}
> +
> +impl SharedRateLimiter {
> + /// Creates a new mmap'ed instance.
> + ///
> + /// Data is mapped in `<base_path>/<name>` using
> + /// `TMPFS`.
> + pub fn mmap_shmem<P: AsRef<Path>>(
> + name: &str,
> + rate: u64,
> + burst: u64,
> + user: User,
> + base_path: P,
> + ) -> Result<Self, Error> {
> + let mut path = base_path.as_ref().to_path_buf();
> +
> + let dir_opts = CreateOptions::new()
> + .perm(Mode::from_bits_truncate(0o770))
> + .owner(user.uid)
> + .group(user.gid);
> +
> + create_path(&path, Some(dir_opts), Some(dir_opts))?;
> +
> + path.push(name);
> +
> + let file_opts = CreateOptions::new()
> + .perm(Mode::from_bits_truncate(0o660))
> + .owner(user.uid)
> + .group(user.gid);
> +
> + let shmem: SharedMemory<SharedRateLimiterData> = SharedMemory::open(&path, file_opts)?;
> +
> + shmem.data().tbf.lock().0.update_rate(rate, burst);
> +
> + Ok(Self { shmem })
> + }
> +}
> +
> +impl ShareableRateLimit for SharedRateLimiter {
> + fn update_rate(&self, rate: u64, bucket_size: u64) {
> + self.shmem
> + .data()
> + .tbf
> + .lock()
> + .0
> + .update_rate(rate, bucket_size);
> + }
> +
> + fn traffic(&self) -> u64 {
> + self.shmem.data().tbf.lock().0.traffic()
> + }
> +
> + fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration {
> + self.shmem
> + .data()
> + .tbf
> + .lock()
> + .0
> + .register_traffic(current_time, data_len)
> + }
> +}
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pbs-devel] [PATCH proxmox v2 2/4] http: drop factored out rate limiter implementation
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 2/4] http: drop factored out rate limiter implementation Christian Ebner
@ 2025-11-11 10:36 ` Hannes Laimer
0 siblings, 0 replies; 21+ messages in thread
From: Hannes Laimer @ 2025-11-11 10:36 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Christian Ebner
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
On 9/16/25 14:42, Christian Ebner wrote:
> The rate limiter implementation has been moved together with the shared
> rate limiter into a dedicated crate. Depend on that and drop the now
> dead code from the proxmox-http crate.
>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Changes since version 1:
> - not present in previous version
>
> Cargo.toml | 1 +
> proxmox-http/Cargo.toml | 4 +-
> proxmox-http/debian/control | 19 +--
> proxmox-http/src/client/connector.rs | 3 +-
> proxmox-http/src/lib.rs | 5 -
> proxmox-http/src/rate_limited_stream.rs | 2 +-
> proxmox-http/src/rate_limiter.rs | 214 ------------------------
> 7 files changed, 8 insertions(+), 240 deletions(-)
> delete mode 100644 proxmox-http/src/rate_limiter.rs
>
> diff --git a/Cargo.toml b/Cargo.toml
> index bde32b17..f62b9882 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -153,6 +153,7 @@ proxmox-login = { version = "1.0.0", path = "proxmox-login" }
> proxmox-network-types = { version = "0.1.0", path = "proxmox-network-types" }
> proxmox-product-config = { version = "1.0.0", path = "proxmox-product-config" }
> proxmox-config-digest = { version = "1.0.0", path = "proxmox-config-digest" }
> +proxmox-rate-limiter = { version = "1.0.0", path = "proxmox-rate-limiter" }
> proxmox-rest-server = { version = "1.0.0", path = "proxmox-rest-server" }
> proxmox-router = { version = "3.2.2", path = "proxmox-router" }
> proxmox-s3-client = { version = "1.2", path = "proxmox-s3-client" }
> diff --git a/proxmox-http/Cargo.toml b/proxmox-http/Cargo.toml
> index 2f83cf5f..0b362bef 100644
> --- a/proxmox-http/Cargo.toml
> +++ b/proxmox-http/Cargo.toml
> @@ -32,6 +32,7 @@ url = { workspace = true, optional = true }
>
> proxmox-async = { workspace = true, optional = true }
> proxmox-base64 = { workspace = true, optional = true }
> +proxmox-rate-limiter = { workspace = true, optional = true, features = [ "rate-limiter" ] }
> proxmox-sys = { workspace = true, optional = true }
> proxmox-io = { workspace = true, optional = true }
> proxmox-lang = { workspace = true, optional = true }
> @@ -53,7 +54,6 @@ body = [
> "dep:sync_wrapper",
> "sync_wrapper?/futures",
> ]
> -rate-limiter = ["dep:hyper"]
> rate-limited-stream = [
> "dep:tokio",
> "dep:hyper-util",
> @@ -61,7 +61,7 @@ rate-limited-stream = [
> "hyper-util?/client-legacy",
> "hyper-util?/http1",
> "tokio?/time",
> - "rate-limiter",
> + "dep:proxmox-rate-limiter",
> ]
> client = [
> "dep:bytes",
> diff --git a/proxmox-http/debian/control b/proxmox-http/debian/control
> index f86e58d1..ae3af111 100644
> --- a/proxmox-http/debian/control
> +++ b/proxmox-http/debian/control
> @@ -29,7 +29,6 @@ Suggests:
> librust-proxmox-http+http-helpers-dev (= ${binary:Version}),
> librust-proxmox-http+proxmox-async-dev (= ${binary:Version}),
> librust-proxmox-http+rate-limited-stream-dev (= ${binary:Version}),
> - librust-proxmox-http+rate-limiter-dev (= ${binary:Version}),
> librust-proxmox-http+websocket-dev (= ${binary:Version})
> Provides:
> librust-proxmox-http+default-dev (= ${binary:Version}),
> @@ -172,12 +171,13 @@ Multi-Arch: same
> Depends:
> ${misc:Depends},
> librust-proxmox-http-dev (= ${binary:Version}),
> - librust-proxmox-http+rate-limiter-dev (= ${binary:Version}),
> librust-hyper-util-0.1+client-dev (>= 0.1.12-~~),
> librust-hyper-util-0.1+client-legacy-dev (>= 0.1.12-~~),
> librust-hyper-util-0.1+default-dev (>= 0.1.12-~~),
> librust-hyper-util-0.1+http1-dev (>= 0.1.12-~~),
> librust-hyper-util-0.1+http2-dev (>= 0.1.12-~~),
> + librust-proxmox-rate-limiter-1+default-dev,
> + librust-proxmox-rate-limiter-1+rate-limiter-dev,
> librust-tokio-1+default-dev (>= 1.6-~~),
> librust-tokio-1+time-dev (>= 1.6-~~)
> Provides:
> @@ -188,21 +188,6 @@ Description: Proxmox HTTP library - feature "rate-limited-stream"
> This metapackage enables feature "rate-limited-stream" for the Rust proxmox-
> http crate, by pulling in any additional dependencies needed by that feature.
>
> -Package: librust-proxmox-http+rate-limiter-dev
> -Architecture: any
> -Multi-Arch: same
> -Depends:
> - ${misc:Depends},
> - librust-proxmox-http-dev (= ${binary:Version}),
> - librust-hyper-1+default-dev
> -Provides:
> - librust-proxmox-http-1+rate-limiter-dev (= ${binary:Version}),
> - librust-proxmox-http-1.0+rate-limiter-dev (= ${binary:Version}),
> - librust-proxmox-http-1.0.3+rate-limiter-dev (= ${binary:Version})
> -Description: Proxmox HTTP library - feature "rate-limiter"
> - This metapackage enables feature "rate-limiter" for the Rust proxmox-http
> - crate, by pulling in any additional dependencies needed by that feature.
> -
> Package: librust-proxmox-http+websocket-dev
> Architecture: any
> Multi-Arch: same
> diff --git a/proxmox-http/src/client/connector.rs b/proxmox-http/src/client/connector.rs
> index 1600d47c..d5d85cb9 100644
> --- a/proxmox-http/src/client/connector.rs
> +++ b/proxmox-http/src/client/connector.rs
> @@ -13,13 +13,14 @@ use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
> use tokio::net::TcpStream;
> use tokio_openssl::SslStream;
>
> +use proxmox_rate_limiter::ShareableRateLimit;
> use proxmox_sys::linux::socket::set_tcp_keepalive;
>
> use crate::proxy_config::ProxyConfig;
> use crate::uri::build_authority;
>
> use super::tls::MaybeTlsStream;
> -use crate::{RateLimitedStream, ShareableRateLimit};
> +use crate::RateLimitedStream;
>
> type SharedRateLimit = Arc<dyn ShareableRateLimit>;
>
> diff --git a/proxmox-http/src/lib.rs b/proxmox-http/src/lib.rs
> index 8b6953b0..7f6f67f8 100644
> --- a/proxmox-http/src/lib.rs
> +++ b/proxmox-http/src/lib.rs
> @@ -26,11 +26,6 @@ mod client_trait;
> #[cfg(feature = "client-trait")]
> pub use client_trait::HttpClient;
>
> -#[cfg(feature = "rate-limiter")]
> -mod rate_limiter;
> -#[cfg(feature = "rate-limiter")]
> -pub use rate_limiter::{RateLimit, RateLimiter, RateLimiterVec, ShareableRateLimit};
> -
> #[cfg(feature = "rate-limited-stream")]
> mod rate_limited_stream;
> #[cfg(feature = "rate-limited-stream")]
> diff --git a/proxmox-http/src/rate_limited_stream.rs b/proxmox-http/src/rate_limited_stream.rs
> index e9308a47..2253bef8 100644
> --- a/proxmox-http/src/rate_limited_stream.rs
> +++ b/proxmox-http/src/rate_limited_stream.rs
> @@ -11,7 +11,7 @@ use tokio::time::Sleep;
>
> use std::task::{Context, Poll};
>
> -use super::{RateLimiter, ShareableRateLimit};
> +use proxmox_rate_limiter::{RateLimiter, ShareableRateLimit};
>
> type SharedRateLimit = Arc<dyn ShareableRateLimit>;
>
> diff --git a/proxmox-http/src/rate_limiter.rs b/proxmox-http/src/rate_limiter.rs
> deleted file mode 100644
> index 945c77a6..00000000
> --- a/proxmox-http/src/rate_limiter.rs
> +++ /dev/null
> @@ -1,214 +0,0 @@
> -use std::convert::TryInto;
> -use std::time::{Duration, Instant};
> -
> -use anyhow::{bail, Error};
> -
> -/// Rate limiter interface.
> -pub trait RateLimit {
> - /// Update rate and bucket size
> - fn update_rate(&mut self, rate: u64, bucket_size: u64);
> -
> - /// Returns the overall traffic (since started)
> - fn traffic(&self) -> u64;
> -
> - /// Register traffic, returning a proposed delay to reach the
> - /// expected rate.
> - fn register_traffic(&mut self, current_time: Instant, data_len: u64) -> Duration;
> -}
> -
> -/// Like [`RateLimit`], but does not require self to be mutable.
> -///
> -/// This is useful for types providing internal mutability (Mutex).
> -pub trait ShareableRateLimit: Send + Sync {
> - fn update_rate(&self, rate: u64, bucket_size: u64);
> - fn traffic(&self) -> u64;
> - fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration;
> -}
> -
> -/// IMPORTANT: We use this struct in shared memory, so please do not
> -/// change/modify the layout (do not add fields)
> -#[derive(Clone)]
> -#[repr(C)]
> -struct TbfState {
> - traffic: u64, // overall traffic
> - last_update: Instant,
> - consumed_tokens: u64,
> -}
> -
> -impl TbfState {
> - const NO_DELAY: Duration = Duration::from_millis(0);
> -
> - fn refill_bucket(&mut self, rate: u64, current_time: Instant) {
> - let time_diff = match current_time.checked_duration_since(self.last_update) {
> - Some(duration) => duration.as_nanos(),
> - None => return,
> - };
> -
> - if time_diff == 0 {
> - return;
> - }
> -
> - self.last_update = current_time;
> -
> - let allowed_traffic = ((time_diff.saturating_mul(rate as u128)) / 1_000_000_000)
> - .try_into()
> - .unwrap_or(u64::MAX);
> -
> - self.consumed_tokens = self.consumed_tokens.saturating_sub(allowed_traffic);
> - }
> -
> - fn register_traffic(
> - &mut self,
> - rate: u64,
> - bucket_size: u64,
> - current_time: Instant,
> - data_len: u64,
> - ) -> Duration {
> - self.refill_bucket(rate, current_time);
> -
> - self.traffic += data_len;
> - self.consumed_tokens += data_len;
> -
> - if self.consumed_tokens <= bucket_size {
> - return Self::NO_DELAY;
> - }
> - Duration::from_nanos(
> - (self.consumed_tokens - bucket_size).saturating_mul(1_000_000_000) / rate,
> - )
> - }
> -}
> -
> -/// Token bucket based rate limiter
> -///
> -/// IMPORTANT: We use this struct in shared memory, so please do not
> -/// change/modify the layout (do not add fields)
> -#[repr(C)]
> -pub struct RateLimiter {
> - rate: u64, // tokens/second
> - bucket_size: u64, // TBF bucket size
> - state: TbfState,
> -}
> -
> -impl RateLimiter {
> - /// Creates a new instance, using [Instant::now] as start time.
> - pub fn new(rate: u64, bucket_size: u64) -> Self {
> - let start_time = Instant::now();
> - Self::with_start_time(rate, bucket_size, start_time)
> - }
> -
> - /// Creates a new instance with specified `rate`, `bucket_size` and `start_time`.
> - pub fn with_start_time(rate: u64, bucket_size: u64, start_time: Instant) -> Self {
> - Self {
> - rate,
> - bucket_size,
> - state: TbfState {
> - traffic: 0,
> - last_update: start_time,
> - // start with empty bucket (all tokens consumed)
> - consumed_tokens: bucket_size,
> - },
> - }
> - }
> -}
> -
> -impl RateLimit for RateLimiter {
> - fn update_rate(&mut self, rate: u64, bucket_size: u64) {
> - self.rate = rate;
> -
> - if bucket_size < self.bucket_size && self.state.consumed_tokens > bucket_size {
> - self.state.consumed_tokens = bucket_size; // start again
> - }
> -
> - self.bucket_size = bucket_size;
> - }
> -
> - fn traffic(&self) -> u64 {
> - self.state.traffic
> - }
> -
> - fn register_traffic(&mut self, current_time: Instant, data_len: u64) -> Duration {
> - self.state
> - .register_traffic(self.rate, self.bucket_size, current_time, data_len)
> - }
> -}
> -
> -impl<R: RateLimit + Send> ShareableRateLimit for std::sync::Mutex<R> {
> - fn update_rate(&self, rate: u64, bucket_size: u64) {
> - self.lock().unwrap().update_rate(rate, bucket_size);
> - }
> -
> - fn traffic(&self) -> u64 {
> - self.lock().unwrap().traffic()
> - }
> -
> - fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration {
> - self.lock()
> - .unwrap()
> - .register_traffic(current_time, data_len)
> - }
> -}
> -
> -/// Array of rate limiters.
> -///
> -/// A group of rate limiters with same configuration.
> -pub struct RateLimiterVec {
> - rate: u64, // tokens/second
> - bucket_size: u64, // TBF bucket size
> - state: Vec<TbfState>,
> -}
> -
> -impl RateLimiterVec {
> - /// Creates a new instance, using [Instant::now] as start time.
> - pub fn new(group_size: usize, rate: u64, bucket_size: u64) -> Self {
> - let start_time = Instant::now();
> - Self::with_start_time(group_size, rate, bucket_size, start_time)
> - }
> -
> - /// Creates a new instance with specified `rate`, `bucket_size` and `start_time`.
> - pub fn with_start_time(
> - group_size: usize,
> - rate: u64,
> - bucket_size: u64,
> - start_time: Instant,
> - ) -> Self {
> - let state = TbfState {
> - traffic: 0,
> - last_update: start_time,
> - // start with empty bucket (all tokens consumed)
> - consumed_tokens: bucket_size,
> - };
> - Self {
> - rate,
> - bucket_size,
> - state: vec![state; group_size],
> - }
> - }
> -
> - #[allow(clippy::len_without_is_empty)]
> - /// Return the number of TBF entries (group_size)
> - pub fn len(&self) -> usize {
> - self.state.len()
> - }
> -
> - /// Traffic for the specified index
> - pub fn traffic(&self, index: usize) -> Result<u64, Error> {
> - if index >= self.state.len() {
> - bail!("RateLimiterVec::traffic - index out of range");
> - }
> - Ok(self.state[index].traffic)
> - }
> -
> - /// Register traffic at the specified index
> - pub fn register_traffic(
> - &mut self,
> - index: usize,
> - current_time: Instant,
> - data_len: u64,
> - ) -> Result<Duration, Error> {
> - if index >= self.state.len() {
> - bail!("RateLimiterVec::register_traffic - index out of range");
> - }
> -
> - Ok(self.state[index].register_traffic(self.rate, self.bucket_size, current_time, data_len))
> - }
> -}
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pbs-devel] [PATCH proxmox v2 4/4] s3-client: add shared rate limiter via https connector
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 4/4] s3-client: add shared rate limiter via https connector Christian Ebner
@ 2025-11-11 10:41 ` Hannes Laimer
0 siblings, 0 replies; 21+ messages in thread
From: Hannes Laimer @ 2025-11-11 10:41 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Christian Ebner
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
On 9/16/25 14:42, Christian Ebner wrote:
> Allows to configure a shared rate limiter for the s3 client to limit
> upload and download bandwidth. This will help users which suffer from
> issues due to network congestion by the unlimited s3 client data
> transfers.
>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Changes since version 1:
> - depend on proxmox-rate-limiter
>
> proxmox-s3-client/Cargo.toml | 7 ++-
> proxmox-s3-client/debian/control | 7 ++-
> proxmox-s3-client/examples/s3_client.rs | 1 +
> proxmox-s3-client/src/api_types.rs | 29 ++++++++++++
> proxmox-s3-client/src/client.rs | 63 +++++++++++++++++++++++--
> proxmox-s3-client/src/lib.rs | 4 +-
> 6 files changed, 105 insertions(+), 6 deletions(-)
>
> diff --git a/proxmox-s3-client/Cargo.toml b/proxmox-s3-client/Cargo.toml
> index 639ae26b..be24c4de 100644
> --- a/proxmox-s3-client/Cargo.toml
> +++ b/proxmox-s3-client/Cargo.toml
> @@ -22,6 +22,7 @@ hyper-util = { workspace = true, features = [ "client-legacy", "tokio", "http1"
> hyper = { workspace = true, optional = true }
> iso8601 = { workspace = true, optional = true }
> md5 = { workspace = true, optional = true }
> +nix = { workspace = true, optional = true }
> openssl = { workspace = true, optional = true }
> quick-xml = { workspace = true, features = [ "async-tokio" ], optional = true }
> regex.workspace = true
> @@ -34,7 +35,9 @@ tracing = { workspace = true, optional = true }
> url = {workspace = true, optional = true }
>
> proxmox-base64 = { workspace = true, optional = true }
> -proxmox-http = { workspace = true, features = [ "body", "client", "client-trait", "rate-limiter" ], optional = true }
> +proxmox-http = { workspace = true, features = [ "body", "client", "client-trait" ], optional = true }
> +proxmox-human-byte.workspace = true
> +proxmox-rate-limiter = { workspace = true, features = [ "rate-limiter", "shared-rate-limiter" ], optional = true }
> proxmox-schema = { workspace = true, features = [ "api-macro", "api-types" ] }
> proxmox-serde.workspace = true
> proxmox-time = {workspace = true, optional = true }
> @@ -51,6 +54,7 @@ impl = [
> "dep:hyper",
> "dep:iso8601",
> "dep:md5",
> + "dep:nix",
> "dep:openssl",
> "dep:quick-xml",
> "dep:serde-xml-rs",
> @@ -60,6 +64,7 @@ impl = [
> "dep:url",
> "dep:proxmox-base64",
> "dep:proxmox-http",
> + "dep:proxmox-rate-limiter",
> "dep:proxmox-time",
> ]
>
> diff --git a/proxmox-s3-client/debian/control b/proxmox-s3-client/debian/control
> index c9147749..24bcbeba 100644
> --- a/proxmox-s3-client/debian/control
> +++ b/proxmox-s3-client/debian/control
> @@ -8,6 +8,7 @@ Build-Depends-Arch: cargo:native <!nocheck>,
> libstd-rust-dev <!nocheck>,
> librust-anyhow-1+default-dev <!nocheck>,
> librust-const-format-0.2+default-dev <!nocheck>,
> + librust-proxmox-human-byte-1+default-dev <!nocheck>,
> librust-proxmox-schema-5+api-macro-dev <!nocheck>,
> librust-proxmox-schema-5+api-types-dev <!nocheck>,
> librust-proxmox-schema-5+default-dev <!nocheck>,
> @@ -31,6 +32,7 @@ Depends:
> ${misc:Depends},
> librust-anyhow-1+default-dev,
> librust-const-format-0.2+default-dev,
> + librust-proxmox-human-byte-1+default-dev,
> librust-proxmox-schema-5+api-macro-dev,
> librust-proxmox-schema-5+api-types-dev,
> librust-proxmox-schema-5+default-dev,
> @@ -74,13 +76,16 @@ Depends:
> librust-hyper-util-0.1+tokio-dev (>= 0.1.12-~~),
> librust-iso8601-0.6+default-dev (>= 0.6.1-~~),
> librust-md5-0.7+default-dev,
> + librust-nix-0.29+default-dev,
> librust-openssl-0.10+default-dev,
> librust-proxmox-base64-1+default-dev,
> librust-proxmox-http-1+body-dev (>= 1.0.3-~~),
> librust-proxmox-http-1+client-dev (>= 1.0.3-~~),
> librust-proxmox-http-1+client-trait-dev (>= 1.0.3-~~),
> librust-proxmox-http-1+default-dev (>= 1.0.3-~~),
> - librust-proxmox-http-1+rate-limiter-dev (>= 1.0.3-~~),
> + librust-proxmox-rate-limiter-1+default-dev,
> + librust-proxmox-rate-limiter-1+rate-limiter-dev,
> + librust-proxmox-rate-limiter-1+shared-rate-limiter-dev,
> librust-proxmox-time-2+default-dev (>= 2.1.0-~~),
> librust-quick-xml-0.36+async-tokio-dev (>= 0.36.1-~~),
> librust-quick-xml-0.36+default-dev (>= 0.36.1-~~),
> diff --git a/proxmox-s3-client/examples/s3_client.rs b/proxmox-s3-client/examples/s3_client.rs
> index 67baf467..dd3885d7 100644
> --- a/proxmox-s3-client/examples/s3_client.rs
> +++ b/proxmox-s3-client/examples/s3_client.rs
> @@ -39,6 +39,7 @@ async fn run() -> Result<(), anyhow::Error> {
> fingerprint: Some("<s3-api-fingerprint>".to_string()),
> put_rate_limit: None,
> provider_quirks: Vec::new(),
> + rate_limiter_config: None,
> };
>
> // Creating a client instance and connect to api endpoint
> diff --git a/proxmox-s3-client/src/api_types.rs b/proxmox-s3-client/src/api_types.rs
> index 115b1d2d..9071868a 100644
> --- a/proxmox-s3-client/src/api_types.rs
> +++ b/proxmox-s3-client/src/api_types.rs
> @@ -2,6 +2,7 @@ use anyhow::bail;
> use const_format::concatcp;
> use serde::{Deserialize, Serialize};
>
> +use proxmox_human_byte::HumanByte;
> use proxmox_schema::api_types::{
> CERT_FINGERPRINT_SHA256_SCHEMA, DNS_LABEL_STR, IPRE_STR, SAFE_ID_FORMAT,
> };
> @@ -126,6 +127,22 @@ serde_plain::derive_fromstr_from_deserialize!(ProviderQuirks);
> type: ProviderQuirks,
> },
> },
> + "rate-in": {
> + type: HumanByte,
> + optional: true,
> + },
> + "burst-in": {
> + type: HumanByte,
> + optional: true,
> + },
> + "rate-out": {
> + type: HumanByte,
> + optional: true,
> + },
> + "burst-out": {
> + type: HumanByte,
> + optional: true,
> + },
> },
> )]
> #[derive(Serialize, Deserialize, Updater, Clone, PartialEq)]
> @@ -154,6 +171,18 @@ pub struct S3ClientConfig {
> /// List of provider specific feature implementation quirks.
> #[serde(skip_serializing_if = "Option::is_none")]
> pub provider_quirks: Option<Vec<ProviderQuirks>>,
> + /// Download rate limit.
> + #[serde(skip_serializing_if = "Option::is_none")]
> + pub rate_in: Option<HumanByte>,
> + /// Download burst.
> + #[serde(skip_serializing_if = "Option::is_none")]
> + pub burst_in: Option<HumanByte>,
> + /// Upload rate limit.
> + #[serde(skip_serializing_if = "Option::is_none")]
> + pub rate_out: Option<HumanByte>,
> + /// Upload burst
> + #[serde(skip_serializing_if = "Option::is_none")]
> + pub burst_out: Option<HumanByte>,
> }
>
> impl S3ClientConfig {
> diff --git a/proxmox-s3-client/src/client.rs b/proxmox-s3-client/src/client.rs
> index 96a5878d..e50f6d1d 100644
> --- a/proxmox-s3-client/src/client.rs
> +++ b/proxmox-s3-client/src/client.rs
> @@ -1,4 +1,4 @@
> -use std::path::Path;
> +use std::path::{Path, PathBuf};
> use std::str::FromStr;
> use std::sync::{Arc, Mutex};
> use std::time::{Duration, Instant};
> @@ -12,6 +12,7 @@ use hyper::{Request, Response};
> use hyper_util::client::legacy::connect::HttpConnector;
> use hyper_util::client::legacy::Client;
> use hyper_util::rt::TokioExecutor;
> +use nix::unistd::User;
> use openssl::hash::MessageDigest;
> use openssl::sha::Sha256;
> use openssl::ssl::{SslConnector, SslMethod, SslVerifyMode};
> @@ -19,7 +20,8 @@ use openssl::x509::X509StoreContextRef;
> use tracing::error;
>
> use proxmox_http::client::HttpsConnector;
> -use proxmox_http::{Body, RateLimit, RateLimiter};
> +use proxmox_http::Body;
> +use proxmox_rate_limiter::{RateLimit, RateLimiter, SharedRateLimiter};
> use proxmox_schema::api_types::CERT_FINGERPRINT_SHA256_SCHEMA;
>
> use crate::api_types::{ProviderQuirks, S3ClientConfig};
> @@ -53,6 +55,25 @@ pub enum S3PathPrefix {
> None,
> }
>
> +/// Options for the https connector's rate limiter
> +pub struct S3RateLimiterOptions {
> + /// ID for the shared rate limiter.
> + pub id: String,
> + /// Base path for the shared memory mapped file
> + pub base_path: PathBuf,
> + /// User for the to be created shared memory mapped file and folders
> + pub user: User,
> +}
> +
> +/// Configuration for the https connector's rate limiter
> +pub struct S3RateLimiterConfig {
> + options: S3RateLimiterOptions,
> + rate_in: Option<u64>,
> + burst_in: Option<u64>,
> + rate_out: Option<u64>,
> + burst_out: Option<u64>,
> +}
> +
> /// Configuration options for client
> pub struct S3ClientOptions {
> /// Endpoint to access S3 object store.
> @@ -77,6 +98,8 @@ pub struct S3ClientOptions {
> pub put_rate_limit: Option<u64>,
> /// Provider implementation specific features and limitations
> pub provider_quirks: Vec<ProviderQuirks>,
> + /// Configuration options for the shared rate limiter.
> + pub rate_limiter_config: Option<S3RateLimiterConfig>,
> }
>
> impl S3ClientOptions {
> @@ -86,7 +109,15 @@ impl S3ClientOptions {
> secret_key: String,
> bucket: Option<String>,
> common_prefix: String,
> + rate_limiter_options: Option<S3RateLimiterOptions>,
> ) -> Self {
> + let rate_limiter_config = rate_limiter_options.map(|options| S3RateLimiterConfig {
> + options,
> + rate_in: config.rate_in.map(|human_bytes| human_bytes.as_u64()),
> + burst_in: config.burst_in.map(|human_bytes| human_bytes.as_u64()),
> + rate_out: config.rate_out.map(|human_bytes| human_bytes.as_u64()),
> + burst_out: config.burst_out.map(|human_bytes| human_bytes.as_u64()),
> + });
> Self {
> endpoint: config.endpoint,
> port: config.port,
> @@ -99,6 +130,7 @@ impl S3ClientOptions {
> secret_key,
> put_rate_limit: config.put_rate_limit,
> provider_quirks: config.provider_quirks.unwrap_or_default(),
> + rate_limiter_config,
> }
> }
> }
> @@ -151,11 +183,36 @@ impl S3Client {
> // want communication to object store backend api to always use https
> http_connector.enforce_http(false);
> http_connector.set_connect_timeout(Some(S3_HTTP_CONNECT_TIMEOUT));
> - let https_connector = HttpsConnector::with_connector(
> + let mut https_connector = HttpsConnector::with_connector(
> http_connector,
> ssl_connector_builder.build(),
> S3_TCP_KEEPIDLE_TIME,
> );
> +
> + if let Some(limiter_config) = &options.rate_limiter_config {
> + if let Some(limit) = limiter_config.rate_in {
> + let limiter = SharedRateLimiter::mmap_shmem(
> + &format!("{}.in", limiter_config.options.id),
> + limit,
> + limiter_config.burst_in.unwrap_or(limit),
> + limiter_config.options.user.clone(),
> + limiter_config.options.base_path.clone(),
> + )?;
> + https_connector.set_read_limiter(Some(Arc::new(limiter)));
> + }
> +
> + if let Some(limit) = limiter_config.rate_out {
> + let limiter = SharedRateLimiter::mmap_shmem(
> + &format!("{}.out", limiter_config.options.id),
> + limit,
> + limiter_config.burst_out.unwrap_or(limit),
> + limiter_config.options.user.clone(),
> + limiter_config.options.base_path.clone(),
> + )?;
> + https_connector.set_write_limiter(Some(Arc::new(limiter)));
> + }
> + }
> +
> let client = Client::builder(TokioExecutor::new()).build::<_, Body>(https_connector);
>
> let authority_template = if let Some(port) = options.port {
> diff --git a/proxmox-s3-client/src/lib.rs b/proxmox-s3-client/src/lib.rs
> index 26e7032b..d02fd0dc 100644
> --- a/proxmox-s3-client/src/lib.rs
> +++ b/proxmox-s3-client/src/lib.rs
> @@ -20,7 +20,9 @@ pub use aws_sign_v4::uri_decode;
> #[cfg(feature = "impl")]
> mod client;
> #[cfg(feature = "impl")]
> -pub use client::{S3Client, S3ClientOptions, S3PathPrefix, S3_HTTP_REQUEST_TIMEOUT};
> +pub use client::{
> + S3Client, S3ClientOptions, S3PathPrefix, S3RateLimiterOptions, S3_HTTP_REQUEST_TIMEOUT,
> +};
> #[cfg(feature = "impl")]
> mod timestamps;
> #[cfg(feature = "impl")]
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pbs-devel] [PATCH proxmox v2 3/4] rest-server: optionally depend on factored out shared rate limiter
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 3/4] rest-server: optionally depend on factored out shared rate limiter Christian Ebner
@ 2025-11-11 10:42 ` Hannes Laimer
0 siblings, 0 replies; 21+ messages in thread
From: Hannes Laimer @ 2025-11-11 10:42 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Christian Ebner
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
On 9/16/25 14:42, Christian Ebner wrote:
> The rate limiter implementation has been moved to a dedicated crate,
> so depend on it for the rest servers rate limited stream connection.
>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Changes since version 1:
> - not present in previous version
>
> proxmox-rest-server/Cargo.toml | 2 ++
> proxmox-rest-server/debian/control | 14 ++++++++------
> proxmox-rest-server/src/connection.rs | 4 +++-
> 3 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/proxmox-rest-server/Cargo.toml b/proxmox-rest-server/Cargo.toml
> index 81d93ca1..7478f6a8 100644
> --- a/proxmox-rest-server/Cargo.toml
> +++ b/proxmox-rest-server/Cargo.toml
> @@ -44,6 +44,7 @@ proxmox-daemon.workspace = true
> proxmox-http = { workspace = true, features = ["body"] }
> proxmox-lang.workspace = true
> proxmox-log.workspace = true
> +proxmox-rate-limiter = { workspace = true, optional = true, features = [ "shared-rate-limiter" ] }
> proxmox-router.workspace = true
> proxmox-schema = { workspace = true, features = [ "api-macro", "upid-api-impl" ] }
> proxmox-sys = { workspace = true, features = [ "logrotate", "timer" ] }
> @@ -54,5 +55,6 @@ proxmox-worker-task.workspace = true
> default = []
> templates = ["dep:handlebars"]
> rate-limited-stream = [
> + "dep:proxmox-rate-limiter",
> "proxmox-http/rate-limited-stream",
> ]
> diff --git a/proxmox-rest-server/debian/control b/proxmox-rest-server/debian/control
> index 263ca7ea..ee04bd3f 100644
> --- a/proxmox-rest-server/debian/control
> +++ b/proxmox-rest-server/debian/control
> @@ -29,8 +29,8 @@ Build-Depends-Arch: cargo:native <!nocheck>,
> librust-proxmox-async-0.5+default-dev <!nocheck>,
> librust-proxmox-compression-1+default-dev <!nocheck>,
> librust-proxmox-daemon-1+default-dev <!nocheck>,
> - librust-proxmox-http-1+body-dev (>= 1.0.2-~~) <!nocheck>,
> - librust-proxmox-http-1+default-dev (>= 1.0.2-~~) <!nocheck>,
> + librust-proxmox-http-1+body-dev (>= 1.0.3-~~) <!nocheck>,
> + librust-proxmox-http-1+default-dev (>= 1.0.3-~~) <!nocheck>,
> librust-proxmox-lang-1+default-dev (>= 1.5-~~) <!nocheck>,
> librust-proxmox-log-1+default-dev <!nocheck>,
> librust-proxmox-router-3+default-dev (>= 3.2.2-~~) <!nocheck>,
> @@ -91,8 +91,8 @@ Depends:
> librust-proxmox-async-0.5+default-dev,
> librust-proxmox-compression-1+default-dev,
> librust-proxmox-daemon-1+default-dev,
> - librust-proxmox-http-1+body-dev (>= 1.0.2-~~),
> - librust-proxmox-http-1+default-dev (>= 1.0.2-~~),
> + librust-proxmox-http-1+body-dev (>= 1.0.3-~~),
> + librust-proxmox-http-1+default-dev (>= 1.0.3-~~),
> librust-proxmox-lang-1+default-dev (>= 1.5-~~),
> librust-proxmox-log-1+default-dev,
> librust-proxmox-router-3+default-dev (>= 3.2.2-~~),
> @@ -137,8 +137,10 @@ Multi-Arch: same
> Depends:
> ${misc:Depends},
> librust-proxmox-rest-server-dev (= ${binary:Version}),
> - librust-proxmox-http-1+body-dev (>= 1.0.2-~~),
> - librust-proxmox-http-1+rate-limited-stream-dev (>= 1.0.2-~~)
> + librust-proxmox-http-1+body-dev (>= 1.0.3-~~),
> + librust-proxmox-http-1+rate-limited-stream-dev (>= 1.0.3-~~),
> + librust-proxmox-rate-limiter-1+default-dev,
> + librust-proxmox-rate-limiter-1+shared-rate-limiter-dev
> Provides:
> librust-proxmox-rest-server-1+rate-limited-stream-dev (= ${binary:Version}),
> librust-proxmox-rest-server-1.0+rate-limited-stream-dev (= ${binary:Version}),
> diff --git a/proxmox-rest-server/src/connection.rs b/proxmox-rest-server/src/connection.rs
> index 9511b7cb..6ffaac73 100644
> --- a/proxmox-rest-server/src/connection.rs
> +++ b/proxmox-rest-server/src/connection.rs
> @@ -24,7 +24,9 @@ use tokio_openssl::SslStream;
> use tokio_stream::wrappers::ReceiverStream;
>
> #[cfg(feature = "rate-limited-stream")]
> -use proxmox_http::{RateLimitedStream, ShareableRateLimit};
> +use proxmox_http::RateLimitedStream;
> +#[cfg(feature = "rate-limited-stream")]
> +use proxmox_rate_limiter::ShareableRateLimit;
>
> #[cfg(feature = "rate-limited-stream")]
> pub type SharedRateLimit = Arc<dyn ShareableRateLimit>;
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v2 1/4] traffic control: use factored out shared rate limiter
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 1/4] traffic control: use factored out shared rate limiter Christian Ebner
@ 2025-11-11 10:45 ` Hannes Laimer
0 siblings, 0 replies; 21+ messages in thread
From: Hannes Laimer @ 2025-11-11 10:45 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Christian Ebner
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
On 9/16/25 14:42, Christian Ebner wrote:
> The shared rate limiter implementation was factored out into a
> dedicated crate so it can be easily reused for other code paths, e.g.
> the s3 client implementation.
>
> Use that implementation and drop the now dead code.
>
> No functional changes intended.
>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Changes since version 1:
> - depend on proxmox-rate-limiter
>
> Cargo.toml | 3 +
> debian/control | 1 -
> pbs-client/Cargo.toml | 3 +-
> pbs-client/src/http_client.rs | 3 +-
> src/tools/mod.rs | 4 -
> src/tools/shared_rate_limiter.rs | 122 -------------------------------
> src/traffic_control_cache.rs | 8 +-
> 7 files changed, 11 insertions(+), 133 deletions(-)
> delete mode 100644 src/tools/shared_rate_limiter.rs
>
> diff --git a/Cargo.toml b/Cargo.toml
> index b3f55b4db..e276c11de 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -74,6 +74,7 @@ proxmox-network-api = "1"
> proxmox-notify = "1"
> proxmox-openid = "1"
> proxmox-product-config = "1"
> +proxmox-rate-limiter = "1.0.0"
> proxmox-rest-server = { version = "1.0.1", features = [ "templates" ] }
> # some use "cli", some use "cli" and "server", pbs-config uses nothing
> proxmox-router = { version = "3.2.2", default-features = false }
> @@ -227,6 +228,7 @@ proxmox-network-api = { workspace = true, features = [ "impl" ] }
> proxmox-notify = { workspace = true, features = [ "pbs-context" ] }
> proxmox-openid.workspace = true
> proxmox-product-config.workspace = true
> +proxmox-rate-limiter = { workspace = true, features = [ "shared-rate-limiter" ] }
> proxmox-rest-server = { workspace = true, features = [ "rate-limited-stream" ] }
> proxmox-router = { workspace = true, features = [ "cli", "server"] }
> proxmox-s3-client.workspace = true
> @@ -288,6 +290,7 @@ proxmox-rrd-api-types.workspace = true
> #proxmox-notify = { path = "../proxmox/proxmox-notify" }
> #proxmox-openid = { path = "../proxmox/proxmox-openid" }
> #proxmox-product-config = { path = "../proxmox/proxmox-product-config" }
> +#proxmox-rate-limiter = { path = "../proxmox/proxmox-rate-limiter" }
> #proxmox-rest-server = { path = "../proxmox/proxmox-rest-server" }
> #proxmox-router = { path = "../proxmox/proxmox-router" }
> #proxmox-rrd = { path = "../proxmox/proxmox-rrd" }
> diff --git a/debian/control b/debian/control
> index 4346acd38..58ea000c6 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -81,7 +81,6 @@ Build-Depends: bash-completion,
> librust-proxmox-http-1+http-helpers-dev (>= 1.0.2-~~),
> librust-proxmox-http-1+proxmox-async-dev (>= 1.0.2-~~),
> librust-proxmox-http-1+rate-limited-stream-dev (>= 1.0.2-~~),
> - librust-proxmox-http-1+rate-limiter-dev (>= 1.0.2-~~),
> librust-proxmox-http-1+websocket-dev (>= 1.0.2-~~),
> librust-proxmox-human-byte-1+default-dev,
> librust-proxmox-io-1+default-dev (>= 1.0.1-~~),
> diff --git a/pbs-client/Cargo.toml b/pbs-client/Cargo.toml
> index 4c6c80983..233969f65 100644
> --- a/pbs-client/Cargo.toml
> +++ b/pbs-client/Cargo.toml
> @@ -36,10 +36,11 @@ pathpatterns.workspace = true
> proxmox-async.workspace = true
> proxmox-auth-api.workspace = true
> proxmox-compression.workspace = true
> -proxmox-http = { workspace = true, features = [ "body", "rate-limiter" ] }
> +proxmox-http = { workspace = true, features = [ "body" ] }
> proxmox-human-byte.workspace = true
> proxmox-io = { workspace = true, features = [ "tokio" ] }
> proxmox-log = { workspace = true }
> +proxmox-rate-limiter = { workspace = true, features = [ "rate-limiter" ] }
> proxmox-router = { workspace = true, features = [ "cli", "server" ] }
> proxmox-schema.workspace = true
> proxmox-sys.workspace = true
> diff --git a/pbs-client/src/http_client.rs b/pbs-client/src/http_client.rs
> index 74b076244..3cdae2c01 100644
> --- a/pbs-client/src/http_client.rs
> +++ b/pbs-client/src/http_client.rs
> @@ -31,8 +31,9 @@ use proxmox_async::broadcast_future::BroadcastFuture;
> use proxmox_http::client::HttpsConnector;
> use proxmox_http::uri::{build_authority, json_object_to_query};
> use proxmox_http::Body;
> -use proxmox_http::{ProxyConfig, RateLimiter};
> +use proxmox_http::ProxyConfig;
> use proxmox_log::{error, info, warn};
> +use proxmox_rate_limiter::RateLimiter;
>
> use pbs_api_types::percent_encoding::DEFAULT_ENCODE_SET;
> use pbs_api_types::{Authid, RateLimitConfig, Userid};
> diff --git a/src/tools/mod.rs b/src/tools/mod.rs
> index 6556effe3..a97aba5b6 100644
> --- a/src/tools/mod.rs
> +++ b/src/tools/mod.rs
> @@ -16,10 +16,6 @@ use pbs_datastore::manifest::BackupManifest;
> pub mod config;
> pub mod disks;
> pub mod fs;
> -
> -mod shared_rate_limiter;
> -pub use shared_rate_limiter::SharedRateLimiter;
> -
> pub mod statistics;
> pub mod systemd;
> pub mod ticket;
> diff --git a/src/tools/shared_rate_limiter.rs b/src/tools/shared_rate_limiter.rs
> deleted file mode 100644
> index db754728b..000000000
> --- a/src/tools/shared_rate_limiter.rs
> +++ /dev/null
> @@ -1,122 +0,0 @@
> -use std::mem::MaybeUninit;
> -use std::path::PathBuf;
> -use std::time::{Duration, Instant};
> -
> -use anyhow::{bail, Error};
> -use nix::sys::stat::Mode;
> -
> -use proxmox_sys::fs::{create_path, CreateOptions};
> -
> -use proxmox_http::{RateLimit, RateLimiter, ShareableRateLimit};
> -use proxmox_shared_memory::{check_subtype, initialize_subtype};
> -use proxmox_shared_memory::{Init, SharedMemory, SharedMutex};
> -
> -// openssl::sha::sha256(b"Proxmox Backup SharedRateLimiter v1.0")[0..8];
> -pub const PROXMOX_BACKUP_SHARED_RATE_LIMITER_MAGIC_1_0: [u8; 8] =
> - [6, 58, 213, 96, 161, 122, 130, 117];
> -
> -const BASE_PATH: &str = pbs_buildcfg::rundir!("/shmem/tbf");
> -
> -// Wrap RateLimiter, so that we can provide an Init impl
> -#[repr(C)]
> -struct WrapLimiter(RateLimiter);
> -
> -impl Init for WrapLimiter {
> - fn initialize(this: &mut MaybeUninit<Self>) {
> - // default does not matter here, because we override later
> - this.write(WrapLimiter(RateLimiter::new(1_000_000, 1_000_000)));
> - }
> -}
> -
> -#[repr(C)]
> -struct SharedRateLimiterData {
> - magic: [u8; 8],
> - tbf: SharedMutex<WrapLimiter>,
> - padding: [u8; 4096 - 104],
> -}
> -
> -impl Init for SharedRateLimiterData {
> - fn initialize(this: &mut MaybeUninit<Self>) {
> - unsafe {
> - let me = &mut *this.as_mut_ptr();
> - me.magic = PROXMOX_BACKUP_SHARED_RATE_LIMITER_MAGIC_1_0;
> - initialize_subtype(&mut me.tbf);
> - }
> - }
> -
> - fn check_type_magic(this: &MaybeUninit<Self>) -> Result<(), Error> {
> - unsafe {
> - let me = &*this.as_ptr();
> - if me.magic != PROXMOX_BACKUP_SHARED_RATE_LIMITER_MAGIC_1_0 {
> - bail!("SharedRateLimiterData: wrong magic number");
> - }
> - check_subtype(&me.tbf)?;
> - Ok(())
> - }
> - }
> -}
> -
> -/// Rate limiter designed for shared memory ([SharedMemory])
> -///
> -/// The actual [RateLimiter] is protected by a [SharedMutex] and
> -/// implements [Init]. This way we can share the limiter between
> -/// different processes.
> -pub struct SharedRateLimiter {
> - shmem: SharedMemory<SharedRateLimiterData>,
> -}
> -
> -impl SharedRateLimiter {
> - /// Creates a new mmap'ed instance.
> - ///
> - /// Data is mapped in `/var/run/proxmox-backup/shmem/tbf/<name>` using
> - /// `TMPFS`.
> - pub fn mmap_shmem(name: &str, rate: u64, burst: u64) -> Result<Self, Error> {
> - let mut path = PathBuf::from(BASE_PATH);
> -
> - let user = pbs_config::backup_user()?;
> -
> - let dir_opts = CreateOptions::new()
> - .perm(Mode::from_bits_truncate(0o770))
> - .owner(user.uid)
> - .group(user.gid);
> -
> - create_path(&path, Some(dir_opts), Some(dir_opts))?;
> -
> - path.push(name);
> -
> - let file_opts = CreateOptions::new()
> - .perm(Mode::from_bits_truncate(0o660))
> - .owner(user.uid)
> - .group(user.gid);
> -
> - let shmem: SharedMemory<SharedRateLimiterData> = SharedMemory::open(&path, file_opts)?;
> -
> - shmem.data().tbf.lock().0.update_rate(rate, burst);
> -
> - Ok(Self { shmem })
> - }
> -}
> -
> -impl ShareableRateLimit for SharedRateLimiter {
> - fn update_rate(&self, rate: u64, bucket_size: u64) {
> - self.shmem
> - .data()
> - .tbf
> - .lock()
> - .0
> - .update_rate(rate, bucket_size);
> - }
> -
> - fn traffic(&self) -> u64 {
> - self.shmem.data().tbf.lock().0.traffic()
> - }
> -
> - fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration {
> - self.shmem
> - .data()
> - .tbf
> - .lock()
> - .0
> - .register_traffic(current_time, data_len)
> - }
> -}
> diff --git a/src/traffic_control_cache.rs b/src/traffic_control_cache.rs
> index 830a8c043..bc52d37ee 100644
> --- a/src/traffic_control_cache.rs
> +++ b/src/traffic_control_cache.rs
> @@ -8,7 +8,7 @@ use std::time::Instant;
> use anyhow::Error;
> use cidr::IpInet;
>
> -use proxmox_http::{RateLimiter, ShareableRateLimit};
> +use proxmox_rate_limiter::{RateLimiter, ShareableRateLimit, SharedRateLimiter};
> use proxmox_section_config::SectionConfigData;
>
> use proxmox_time::{parse_daily_duration, DailyDuration, TmEditor};
> @@ -17,8 +17,6 @@ use pbs_api_types::TrafficControlRule;
>
> use pbs_config::ConfigVersionCache;
>
> -use crate::tools::SharedRateLimiter;
> -
> pub type SharedRateLimit = Arc<dyn ShareableRateLimit>;
>
> /// Shared traffic control cache singleton.
> @@ -110,7 +108,9 @@ fn create_limiter(
> burst: u64,
> ) -> Result<SharedRateLimit, Error> {
> if use_shared_memory {
> - let limiter = SharedRateLimiter::mmap_shmem(name, rate, burst)?;
> + let user = pbs_config::backup_user()?;
> + let base_path = pbs_buildcfg::rundir!("/shmem/tbf");
> + let limiter = SharedRateLimiter::mmap_shmem(name, rate, burst, user, base_path)?;
> Ok(Arc::new(limiter))
> } else {
> Ok(Arc::new(Mutex::new(RateLimiter::new(rate, burst))))
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v2 2/4] api: config: update s3 endpoint rate limits in config
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 2/4] api: config: update s3 endpoint rate limits in config Christian Ebner
@ 2025-11-11 10:45 ` Hannes Laimer
0 siblings, 0 replies; 21+ messages in thread
From: Hannes Laimer @ 2025-11-11 10:45 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Christian Ebner
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
On 9/16/25 14:42, Christian Ebner wrote:
> Set the rate/burst limits in the s3 endpoint configuration whenever
> the limits changed.
>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Changes since version 1:
> - no changes
>
> src/api2/config/s3.rs | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/src/api2/config/s3.rs b/src/api2/config/s3.rs
> index 8270e8d52..1e421114c 100644
> --- a/src/api2/config/s3.rs
> +++ b/src/api2/config/s3.rs
> @@ -139,6 +139,14 @@ pub enum DeletableProperty {
> Fingerprint,
> /// Delete the path-style property.
> PathStyle,
> + /// Delete the rate-in property.
> + RateIn,
> + /// Delete the burst-in property.
> + BurstIn,
> + /// Delete the rate-out property.
> + RateOut,
> + /// Delete the burst-out property.
> + BurstOut,
> /// Delete the provider quirks property.
> ProviderQuirks,
> }
> @@ -213,6 +221,18 @@ pub fn update_s3_client_config(
> DeletableProperty::PathStyle => {
> data.config.path_style = None;
> }
> + DeletableProperty::RateIn => {
> + data.config.rate_in = None;
> + }
> + DeletableProperty::BurstIn => {
> + data.config.burst_in = None;
> + }
> + DeletableProperty::RateOut => {
> + data.config.rate_out = None;
> + }
> + DeletableProperty::BurstOut => {
> + data.config.burst_out = None;
> + }
> DeletableProperty::ProviderQuirks => {
> data.config.provider_quirks = None;
> }
> @@ -238,6 +258,18 @@ pub fn update_s3_client_config(
> if let Some(path_style) = update.path_style {
> data.config.path_style = Some(path_style);
> }
> + if let Some(rate_in) = update.rate_in {
> + data.config.rate_in = Some(rate_in);
> + }
> + if let Some(burst_in) = update.burst_in {
> + data.config.burst_in = Some(burst_in);
> + }
> + if let Some(rate_out) = update.rate_out {
> + data.config.rate_out = Some(rate_out);
> + }
> + if let Some(burst_out) = update.burst_out {
> + data.config.burst_out = Some(burst_out);
> + }
> if let Some(provider_quirks) = update.provider_quirks {
> data.config.provider_quirks = Some(provider_quirks);
> }
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v2 3/4] datastore: s3: set rate limiter options for s3 client
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 3/4] datastore: s3: set rate limiter options for s3 client Christian Ebner
@ 2025-11-11 10:46 ` Hannes Laimer
0 siblings, 0 replies; 21+ messages in thread
From: Hannes Laimer @ 2025-11-11 10:46 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Christian Ebner
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
On 9/16/25 14:42, Christian Ebner wrote:
> Set the shared rate limiter for each client instance based on the
> endpoint configuration. The same limits are shared for each s3
> endpoint. To avoid possibly id clashing with rate limits set via
> traffic control, use the base directory `<rundir>/s3/shmem/tbf`
> instead of the traffic control's `<rundir>/shmem/tbf`.
>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Changes since version 1:
> - no changes
>
> pbs-datastore/src/datastore.rs | 18 +++++++++++++++++-
> src/api2/admin/s3.rs | 9 +++++++--
> src/api2/config/s3.rs | 2 +-
> 3 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index 7cf020fc0..e7cc76dc7 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -14,7 +14,9 @@ use tokio::io::AsyncWriteExt;
> use tracing::{info, warn};
>
> use proxmox_human_byte::HumanByte;
> -use proxmox_s3_client::{S3Client, S3ClientConf, S3ClientOptions, S3ObjectKey, S3PathPrefix};
> +use proxmox_s3_client::{
> + S3Client, S3ClientConf, S3ClientOptions, S3ObjectKey, S3PathPrefix, S3RateLimiterOptions,
> +};
> use proxmox_schema::ApiType;
>
> use proxmox_sys::error::SysError;
> @@ -55,6 +57,7 @@ pub const GROUP_NOTES_FILE_NAME: &str = "notes";
> pub const GROUP_OWNER_FILE_NAME: &str = "owner";
> /// Filename for in-use marker stored on S3 object store backend
> pub const S3_DATASTORE_IN_USE_MARKER: &str = ".in-use";
> +const S3_CLIENT_RATE_LIMITER_BASE_PATH: &str = pbs_buildcfg::rundir!("/s3/shmem/tbf");
> const NAMESPACE_MARKER_FILENAME: &str = ".namespace";
>
> /// checks if auth_id is owner, or, if owner is a token, if
> @@ -254,12 +257,18 @@ impl DataStore {
>
> let (config, _config_digest) = pbs_config::s3::config()?;
> let config: S3ClientConf = config.lookup(S3_CFG_TYPE_ID, s3_client_id)?;
> + let rate_limiter_options = S3RateLimiterOptions {
> + id: s3_client_id.to_string(),
> + user: pbs_config::backup_user()?,
> + base_path: S3_CLIENT_RATE_LIMITER_BASE_PATH.into(),
> + };
>
> let options = S3ClientOptions::from_config(
> config.config,
> config.secret_key,
> Some(bucket),
> self.name().to_owned(),
> + Some(rate_limiter_options),
> );
> let s3_client = S3Client::new(options)?;
> DatastoreBackend::S3(Arc::new(s3_client))
> @@ -2432,11 +2441,18 @@ impl DataStore {
> let client_config: S3ClientConf = config
> .lookup(S3_CFG_TYPE_ID, s3_client_id)
> .with_context(|| format!("no '{s3_client_id}' in config"))?;
> + let rate_limiter_options = S3RateLimiterOptions {
> + id: s3_client_id.to_string(),
> + user: pbs_config::backup_user()?,
> + base_path: S3_CLIENT_RATE_LIMITER_BASE_PATH.into(),
> + };
> +
> let options = S3ClientOptions::from_config(
> client_config.config,
> client_config.secret_key,
> Some(bucket),
> datastore_config.name.to_owned(),
> + Some(rate_limiter_options),
> );
> let s3_client = S3Client::new(options)
> .context("failed to create s3 client")
> diff --git a/src/api2/admin/s3.rs b/src/api2/admin/s3.rs
> index 73f3779a5..73388281b 100644
> --- a/src/api2/admin/s3.rs
> +++ b/src/api2/admin/s3.rs
> @@ -49,8 +49,13 @@ pub async fn check(
> .context("config lookup failed")?;
>
> let store_prefix = store_prefix.unwrap_or_default();
> - let options =
> - S3ClientOptions::from_config(config.config, config.secret_key, Some(bucket), store_prefix);
> + let options = S3ClientOptions::from_config(
> + config.config,
> + config.secret_key,
> + Some(bucket),
> + store_prefix,
> + None,
> + );
>
> let test_object_key =
> S3ObjectKey::try_from(".s3-client-test").context("failed to generate s3 object key")?;
> diff --git a/src/api2/config/s3.rs b/src/api2/config/s3.rs
> index 1e421114c..27b3c4cc2 100644
> --- a/src/api2/config/s3.rs
> +++ b/src/api2/config/s3.rs
> @@ -351,7 +351,7 @@ pub async fn list_buckets(
>
> let empty_prefix = String::new();
> let options =
> - S3ClientOptions::from_config(config.config, config.secret_key, None, empty_prefix);
> + S3ClientOptions::from_config(config.config, config.secret_key, None, empty_prefix, None);
> let client = S3Client::new(options).context("client creation failed")?;
> let list_buckets_response = client
> .list_buckets()
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v2 4/4] ui: expose rate and burst limits for s3 endpoints
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 4/4] ui: expose rate and burst limits for s3 endpoints Christian Ebner
@ 2025-11-11 10:46 ` Hannes Laimer
0 siblings, 0 replies; 21+ messages in thread
From: Hannes Laimer @ 2025-11-11 10:46 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Christian Ebner
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
On 9/16/25 14:42, Christian Ebner wrote:
> Allows to configure the s3 endpoints rate limits via the advanced
> options.
>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Changes since version 1:
> - no changes
>
> www/window/S3ClientEdit.js | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/www/window/S3ClientEdit.js b/www/window/S3ClientEdit.js
> index 269b35967..8862683b8 100644
> --- a/www/window/S3ClientEdit.js
> +++ b/www/window/S3ClientEdit.js
> @@ -121,6 +121,20 @@ Ext.define('PBS.window.S3ClientEdit', {
> ],
>
> advancedColumn1: [
> + {
> + xtype: 'pmxBandwidthField',
> + name: 'rate-in',
> + fieldLabel: gettext('Rate In'),
> + emptyText: gettext('Unlimited'),
> + submitAutoScaledSizeUnit: true,
> + },
> + {
> + xtype: 'pmxBandwidthField',
> + name: 'rate-out',
> + fieldLabel: gettext('Rate Out'),
> + emptyText: gettext('Unlimited'),
> + submitAutoScaledSizeUnit: true,
> + },
> {
> xtype: 'proxmoxKVComboBox',
> name: 'provider-quirks',
> @@ -136,6 +150,22 @@ Ext.define('PBS.window.S3ClientEdit', {
> },
> },
> ],
> + advancedColumn2: [
> + {
> + xtype: 'pmxBandwidthField',
> + name: 'burst-in',
> + fieldLabel: gettext('Burst In'),
> + emptyText: gettext('Same as Rate'),
> + submitAutoScaledSizeUnit: true,
> + },
> + {
> + xtype: 'pmxBandwidthField',
> + name: 'burst-out',
> + fieldLabel: gettext('Burst Out'),
> + emptyText: gettext('Same as Rate'),
> + submitAutoScaledSizeUnit: true,
> + },
> + ],
> },
>
> getValues: function () {
> @@ -146,6 +176,10 @@ Ext.define('PBS.window.S3ClientEdit', {
> values.delete = values.delete.split(',');
> }
> PBS.Utils.delete_if_default(values, 'path-style', false, me.isCreate);
> + PBS.Utils.delete_if_default(values, 'rate-in', undefined, me.isCreate);
> + PBS.Utils.delete_if_default(values, 'burst-in', undefined, me.isCreate);
> + PBS.Utils.delete_if_default(values, 'rate-out', undefined, me.isCreate);
> + PBS.Utils.delete_if_default(values, 'burst-out', undefined, me.isCreate);
>
> let https_scheme_prefix = 'https://';
> if (values.endpoint.startsWith(https_scheme_prefix)) {
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances
2025-09-16 12:41 [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Christian Ebner
` (7 preceding siblings ...)
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 4/4] ui: expose rate and burst limits for s3 endpoints Christian Ebner
@ 2025-11-11 10:49 ` Hannes Laimer
2025-11-11 15:10 ` Christian Ebner
2025-11-12 11:55 ` [pbs-devel] superseded: " Christian Ebner
9 siblings, 1 reply; 21+ messages in thread
From: Hannes Laimer @ 2025-11-11 10:49 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Christian Ebner
I did run into some conflicts when applying, mostly control files, other
than that and a small div by zero problem this L(very)GTM!
I did test this with a local S3 storage, worked as advertised.
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
On 9/16/25 14:42, Christian Ebner wrote:
> This patches implement a shared bandwidth rate limiter for the s3
> client instances, with the goal to allow users to avoid network
> congestion by traffic to the s3 backend.
>
> The limiter is shared accorss all the clients using the same s3
> endpoint id, setting the limits on client instantiation. To utilize
> the pre-existing shared rate limiter implementation from PBS, factor
> it out into a dedicated crate, together with the non-shared rate limiter
> implementation from proxmox-http.
>
> Expose the rate limit configuration in the s3 endpoint configuration
> and provide it to the client's https connector. Expose the settings
> also in the advanced options of the s3 endpoint edit/crate window.
>
> Changes since version 1 (thanks @Thomas for feedback):
> - Move shared/non-shared rate limiter into dedicated crate
> - Use a more generic name for the magic number constant of the mmapped
> state file
>
> proxmox:
>
> Christian Ebner (4):
> rate-limiter: add crate for traffic rate limiter implementations
> http: drop factored out rate limiter implementation
> rest-server: optionally depend on factored out shared rate limiter
> s3-client: add shared rate limiter via https connector
>
> Cargo.toml | 2 +
> proxmox-http/Cargo.toml | 4 +-
> proxmox-http/debian/control | 19 +--
> proxmox-http/src/client/connector.rs | 3 +-
> proxmox-http/src/lib.rs | 5 -
> proxmox-http/src/rate_limited_stream.rs | 2 +-
> proxmox-rate-limiter/Cargo.toml | 29 ++++
> proxmox-rate-limiter/debian/changelog | 5 +
> proxmox-rate-limiter/debian/control | 70 ++++++++++
> proxmox-rate-limiter/debian/copyright | 18 +++
> proxmox-rate-limiter/debian/debcargo.toml | 7 +
> proxmox-rate-limiter/src/lib.rs | 13 ++
> .../src/rate_limiter.rs | 0
> .../src/shared_rate_limiter.rs | 130 ++++++++++++++++++
> proxmox-rest-server/Cargo.toml | 2 +
> proxmox-rest-server/debian/control | 14 +-
> proxmox-rest-server/src/connection.rs | 4 +-
> proxmox-s3-client/Cargo.toml | 7 +-
> proxmox-s3-client/debian/control | 7 +-
> proxmox-s3-client/examples/s3_client.rs | 1 +
> proxmox-s3-client/src/api_types.rs | 29 ++++
> proxmox-s3-client/src/client.rs | 63 ++++++++-
> proxmox-s3-client/src/lib.rs | 4 +-
> 23 files changed, 399 insertions(+), 39 deletions(-)
> create mode 100644 proxmox-rate-limiter/Cargo.toml
> create mode 100644 proxmox-rate-limiter/debian/changelog
> create mode 100644 proxmox-rate-limiter/debian/control
> create mode 100644 proxmox-rate-limiter/debian/copyright
> create mode 100644 proxmox-rate-limiter/debian/debcargo.toml
> create mode 100644 proxmox-rate-limiter/src/lib.rs
> rename {proxmox-http => proxmox-rate-limiter}/src/rate_limiter.rs (100%)
> create mode 100644 proxmox-rate-limiter/src/shared_rate_limiter.rs
>
>
> proxmox-backup:
>
> Christian Ebner (4):
> traffic control: use factored out shared rate limiter
> api: config: update s3 endpoint rate limits in config
> datastore: s3: set rate limiter options for s3 client
> ui: expose rate and burst limits for s3 endpoints
>
> Cargo.toml | 3 +
> debian/control | 1 -
> pbs-client/Cargo.toml | 3 +-
> pbs-client/src/http_client.rs | 3 +-
> pbs-datastore/src/datastore.rs | 18 ++++-
> src/api2/admin/s3.rs | 9 ++-
> src/api2/config/s3.rs | 34 ++++++++-
> src/tools/mod.rs | 4 -
> src/tools/shared_rate_limiter.rs | 122 -------------------------------
> src/traffic_control_cache.rs | 8 +-
> www/window/S3ClientEdit.js | 34 +++++++++
> 11 files changed, 102 insertions(+), 137 deletions(-)
> delete mode 100644 src/tools/shared_rate_limiter.rs
>
>
> Summary over all repositories:
> 34 files changed, 501 insertions(+), 176 deletions(-)
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pbs-devel] [PATCH proxmox v2 1/4] rate-limiter: add crate for traffic rate limiter implementations
2025-11-11 10:34 ` Hannes Laimer
@ 2025-11-11 15:06 ` Christian Ebner
0 siblings, 0 replies; 21+ messages in thread
From: Christian Ebner @ 2025-11-11 15:06 UTC (permalink / raw)
To: Hannes Laimer, Proxmox Backup Server development discussion
On 11/11/25 11:34 AM, Hannes Laimer wrote:
> comment inline, other than that consider this
>
> Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
> Tested-by: Hannes Laimer <h.laimer@proxmox.com>
>
> On 9/16/25 14:42, Christian Ebner wrote:
>> Factors out the traffic rate limiter implementations currently tied
>> to the proxmox-backup and proxmox-http crates to make them
>> independent and easily reusable, e.g. for the s3-client
>> implementation.
>>
>> The shared rate limiter implementation from PBS relies on mmapping
>> for state sharing, the file exposed having a predefined magic number.
>> In order to be backwards compatible, leave the magic number as is and
>> only adapt the constant name to be more generic, although the string
>> the magic number is derived from is PBS specific.
>>
>> Further, the user for file ownership and base path of the mmapped
>> file are now passed as parameters during shared rate limiter
>> instantiation.
>>
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>> Changes since version 1:
>> - move rate limiter implementations into dedicated crate instead of
>> proxmox-http
>> - Adapt shared state file magic constant name to be generic
>>
>> Cargo.toml | 1 +
>> proxmox-rate-limiter/Cargo.toml | 29 +++
>> proxmox-rate-limiter/debian/changelog | 5 +
>> proxmox-rate-limiter/debian/control | 70 ++++++
>> proxmox-rate-limiter/debian/copyright | 18 ++
>> proxmox-rate-limiter/debian/debcargo.toml | 7 +
>> proxmox-rate-limiter/src/lib.rs | 13 ++
>> proxmox-rate-limiter/src/rate_limiter.rs | 214 ++++++++++++++++++
>> .../src/shared_rate_limiter.rs | 130 +++++++++++
>> 9 files changed, 487 insertions(+)
>> create mode 100644 proxmox-rate-limiter/Cargo.toml
>> create mode 100644 proxmox-rate-limiter/debian/changelog
>> create mode 100644 proxmox-rate-limiter/debian/control
>> create mode 100644 proxmox-rate-limiter/debian/copyright
>> create mode 100644 proxmox-rate-limiter/debian/debcargo.toml
>> create mode 100644 proxmox-rate-limiter/src/lib.rs
>> create mode 100644 proxmox-rate-limiter/src/rate_limiter.rs
>> create mode 100644 proxmox-rate-limiter/src/shared_rate_limiter.rs
>>
>> diff --git a/Cargo.toml b/Cargo.toml
>> index f149af65..bde32b17 100644
>> --- a/Cargo.toml
>> +++ b/Cargo.toml
>> @@ -29,6 +29,7 @@ members = [
>> "proxmox-notify",
>> "proxmox-openid",
>> "proxmox-product-config",
>> + "proxmox-rate-limiter",
>> "proxmox-resource-scheduling",
>> "proxmox-rest-server",
>> "proxmox-router",
>> diff --git a/proxmox-rate-limiter/Cargo.toml b/proxmox-rate-limiter/
>> Cargo.toml
>> new file mode 100644
>> index 00000000..6d8d96cc
>> --- /dev/null
>> +++ b/proxmox-rate-limiter/Cargo.toml
>> @@ -0,0 +1,29 @@
>> +[package]
>> +name = "proxmox-rate-limiter"
>> +description = "Token bucket based traffic rate limiter implementation"
>> +version = "1.0.0"
>> +
>> +authors.workspace = true
>> +edition.workspace = true
>> +exclude.workspace = true
>> +homepage.workspace = true
>> +license.workspace = true
>> +repository.workspace = true
>> +rust-version.workspace = true
>> +
>> +[dependencies]
>> +anyhow.workspace = true
>> +hyper = { workspace = true, optional = true }
>> +nix = { workspace = true, optional = true }
>> +
>> +proxmox-shared-memory = { workspace = true, optional = true }
>> +proxmox-sys = { workspace = true, optional = true }
>> +
>> +[features]
>> +default = []
>> +rate-limiter = ["dep:hyper"]
>> +shared-rate-limiter = [
>> + "dep:nix",
>> + "dep:proxmox-shared-memory",
>> + "dep:proxmox-sys",
>> +]
>> diff --git a/proxmox-rate-limiter/debian/changelog b/proxmox-rate-
>> limiter/debian/changelog
>> new file mode 100644
>> index 00000000..0bffa551
>> --- /dev/null
>> +++ b/proxmox-rate-limiter/debian/changelog
>> @@ -0,0 +1,5 @@
>> +rust-proxmox-rate-limiter (1.0.0-1) bookworm; urgency=medium
>> +
>> + * initial packaging
>> +
>> + -- Proxmox Support Team <support@proxmox.com> Tue, 16 Sep 2025
>> 11:06:23 +0200
>> diff --git a/proxmox-rate-limiter/debian/control b/proxmox-rate-
>> limiter/debian/control
>> new file mode 100644
>> index 00000000..689fe02e
>> --- /dev/null
>> +++ b/proxmox-rate-limiter/debian/control
>> @@ -0,0 +1,70 @@
>> +Source: rust-proxmox-rate-limiter
>> +Section: rust
>> +Priority: optional
>> +Build-Depends: debhelper-compat (= 13),
>> + dh-sequence-cargo
>> +Build-Depends-Arch: cargo:native <!nocheck>,
>> + rustc:native (>= 1.82) <!nocheck>,
>> + libstd-rust-dev <!nocheck>,
>> + librust-anyhow-1+default-dev <!nocheck>
>> +Maintainer: Proxmox Support Team <support@proxmox.com>
>> +Standards-Version: 4.7.0
>> +Vcs-Git: git://git.proxmox.com/git/proxmox.git
>> +Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
>> +Homepage: https://proxmox.com
>> +X-Cargo-Crate: proxmox-rate-limiter
>> +Rules-Requires-Root: no
>> +
>> +Package: librust-proxmox-rate-limiter-dev
>> +Architecture: any
>> +Multi-Arch: same
>> +Depends:
>> + ${misc:Depends},
>> + librust-anyhow-1+default-dev
>> +Suggests:
>> + librust-proxmox-rate-limiter+rate-limiter-dev (= ${binary:Version}),
>> + librust-proxmox-rate-limiter+shared-rate-limiter-dev (=
>> ${binary:Version})
>> +Provides:
>> + librust-proxmox-rate-limiter+default-dev (= ${binary:Version}),
>> + librust-proxmox-rate-limiter-1-dev (= ${binary:Version}),
>> + librust-proxmox-rate-limiter-1+default-dev (= ${binary:Version}),
>> + librust-proxmox-rate-limiter-1.0-dev (= ${binary:Version}),
>> + librust-proxmox-rate-limiter-1.0+default-dev (= ${binary:Version}),
>> + librust-proxmox-rate-limiter-1.0.0-dev (= ${binary:Version}),
>> + librust-proxmox-rate-limiter-1.0.0+default-dev (= ${binary:Version})
>> +Description: Token bucket based traffic rate limiter implementation -
>> Rust source code
>> + Source code for Debianized Rust crate "proxmox-rate-limiter"
>> +
>> +Package: librust-proxmox-rate-limiter+rate-limiter-dev
>> +Architecture: any
>> +Multi-Arch: same
>> +Depends:
>> + ${misc:Depends},
>> + librust-proxmox-rate-limiter-dev (= ${binary:Version}),
>> + librust-hyper-1+default-dev
>> +Provides:
>> + librust-proxmox-rate-limiter-1+rate-limiter-dev (= ${binary:Version}),
>> + librust-proxmox-rate-limiter-1.0+rate-limiter-dev (=
>> ${binary:Version}),
>> + librust-proxmox-rate-limiter-1.0.0+rate-limiter-dev (=
>> ${binary:Version})
>> +Description: Token bucket based traffic rate limiter implementation -
>> feature "rate-limiter"
>> + This metapackage enables feature "rate-limiter" for the Rust
>> proxmox-rate-
>> + limiter crate, by pulling in any additional dependencies needed by that
>> + feature.
>> +
>> +Package: librust-proxmox-rate-limiter+shared-rate-limiter-dev
>> +Architecture: any
>> +Multi-Arch: same
>> +Depends:
>> + ${misc:Depends},
>> + librust-proxmox-rate-limiter-dev (= ${binary:Version}),
>> + librust-nix-0.29+default-dev,
>> + librust-proxmox-shared-memory-1+default-dev,
>> + librust-proxmox-sys-1+default-dev
>> +Provides:
>> + librust-proxmox-rate-limiter-1+shared-rate-limiter-dev (=
>> ${binary:Version}),
>> + librust-proxmox-rate-limiter-1.0+shared-rate-limiter-dev (=
>> ${binary:Version}),
>> + librust-proxmox-rate-limiter-1.0.0+shared-rate-limiter-dev (=
>> ${binary:Version})
>> +Description: Token bucket based traffic rate limiter implementation -
>> feature "shared-rate-limiter"
>> + This metapackage enables feature "shared-rate-limiter" for the Rust
>> proxmox-
>> + rate-limiter crate, by pulling in any additional dependencies needed
>> by that
>> + feature.
>> diff --git a/proxmox-rate-limiter/debian/copyright b/proxmox-rate-
>> limiter/debian/copyright
>> new file mode 100644
>> index 00000000..d6e3c304
>> --- /dev/null
>> +++ b/proxmox-rate-limiter/debian/copyright
>> @@ -0,0 +1,18 @@
>> +Format: https://www.debian.org/doc/packaging-manuals/copyright-
>> format/1.0/
>> +
>> +Files:
>> + *
>> +Copyright: 2025 Proxmox Server Solutions GmbH <support@proxmox.com>
>> +License: AGPL-3.0-or-later
>> + This program is free software: you can redistribute it and/or modify
>> it under
>> + the terms of the GNU Affero General Public License as published by
>> the Free
>> + Software Foundation, either version 3 of the License, or (at your
>> option) any
>> + later version.
>> + .
>> + This program is distributed in the hope that it will be useful, but
>> WITHOUT
>> + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
>> or FITNESS
>> + FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License
>> for more
>> + details.
>> + .
>> + You should have received a copy of the GNU Affero General Public
>> License along
>> + with this program. If not, see <https://www.gnu.org/licenses/>.
>> diff --git a/proxmox-rate-limiter/debian/debcargo.toml b/proxmox-rate-
>> limiter/debian/debcargo.toml
>> new file mode 100644
>> index 00000000..b7864cdb
>> --- /dev/null
>> +++ b/proxmox-rate-limiter/debian/debcargo.toml
>> @@ -0,0 +1,7 @@
>> +overlay = "."
>> +crate_src_path = ".."
>> +maintainer = "Proxmox Support Team <support@proxmox.com>"
>> +
>> +[source]
>> +vcs_git = "git://git.proxmox.com/git/proxmox.git"
>> +vcs_browser = "https://git.proxmox.com/?p=proxmox.git"
>> diff --git a/proxmox-rate-limiter/src/lib.rs b/proxmox-rate-limiter/
>> src/lib.rs
>> new file mode 100644
>> index 00000000..a8d7cfdd
>> --- /dev/null
>> +++ b/proxmox-rate-limiter/src/lib.rs
>> @@ -0,0 +1,13 @@
>> +//! Token bucket based traffic rate limiter implementations.
>> +
>> +#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
>> +
>> +#[cfg(feature = "rate-limiter")]
>> +mod rate_limiter;
>> +#[cfg(feature = "rate-limiter")]
>> +pub use rate_limiter::{RateLimit, RateLimiter, RateLimiterVec,
>> ShareableRateLimit};
>> +
>> +#[cfg(feature = "shared-rate-limiter")]
>> +mod shared_rate_limiter;
>> +#[cfg(feature = "shared-rate-limiter")]
>> +pub use shared_rate_limiter::SharedRateLimiter;
>> diff --git a/proxmox-rate-limiter/src/rate_limiter.rs b/proxmox-rate-
>> limiter/src/rate_limiter.rs
>> new file mode 100644
>> index 00000000..945c77a6
>> --- /dev/null
>> +++ b/proxmox-rate-limiter/src/rate_limiter.rs
>> @@ -0,0 +1,214 @@
>> +use std::convert::TryInto;
>> +use std::time::{Duration, Instant};
>> +
>> +use anyhow::{bail, Error};
>> +
>> +/// Rate limiter interface.
>> +pub trait RateLimit {
>> + /// Update rate and bucket size
>> + fn update_rate(&mut self, rate: u64, bucket_size: u64);
>> +
>> + /// Returns the overall traffic (since started)
>> + fn traffic(&self) -> u64;
>> +
>> + /// Register traffic, returning a proposed delay to reach the
>> + /// expected rate.
>> + fn register_traffic(&mut self, current_time: Instant, data_len:
>> u64) -> Duration;
>> +}
>> +
>> +/// Like [`RateLimit`], but does not require self to be mutable.
>> +///
>> +/// This is useful for types providing internal mutability (Mutex).
>> +pub trait ShareableRateLimit: Send + Sync {
>> + fn update_rate(&self, rate: u64, bucket_size: u64);
>> + fn traffic(&self) -> u64;
>> + fn register_traffic(&self, current_time: Instant, data_len: u64)
>> -> Duration;
>> +}
>> +
>> +/// IMPORTANT: We use this struct in shared memory, so please do not
>> +/// change/modify the layout (do not add fields)
>> +#[derive(Clone)]
>> +#[repr(C)]
>> +struct TbfState {
>> + traffic: u64, // overall traffic
>> + last_update: Instant,
>> + consumed_tokens: u64,
>> +}
>> +
>> +impl TbfState {
>> + const NO_DELAY: Duration = Duration::from_millis(0);
>> +
>> + fn refill_bucket(&mut self, rate: u64, current_time: Instant) {
>> + let time_diff = match
>> current_time.checked_duration_since(self.last_update) {
>> + Some(duration) => duration.as_nanos(),
>> + None => return,
>> + };
>> +
>> + if time_diff == 0 {
>> + return;
>> + }
>> +
>> + self.last_update = current_time;
>> +
>> + let allowed_traffic = ((time_diff.saturating_mul(rate as
>> u128)) / 1_000_000_000)
>> + .try_into()
>> + .unwrap_or(u64::MAX);
>> +
>> + self.consumed_tokens =
>> self.consumed_tokens.saturating_sub(allowed_traffic);
>> + }
>> +
>> + fn register_traffic(
>> + &mut self,
>> + rate: u64,
>> + bucket_size: u64,
>> + current_time: Instant,
>> + data_len: u64,
>> + ) -> Duration {
>> + self.refill_bucket(rate, current_time);
>> +
>> + self.traffic += data_len;
>> + self.consumed_tokens += data_len;
>> +
>> + if self.consumed_tokens <= bucket_size {
>> + return Self::NO_DELAY;
>> + }
>> + Duration::from_nanos(
>> + (self.consumed_tokens -
>> bucket_size).saturating_mul(1_000_000_000) / rate,
>
> the UI doesn't let you set 0, but if one edits the config directly the
> `... / rate` is a problem. The scenario is a little far fetched, but
> someone might quickly want to test something and just assume 0 means
> unlimited and just set it to that instead of removing the line.
Nice catch! This is wrong indeed (also in the pre-existing code). Since
this is however a private method of the TbfState, I'd rather fix it on
the call sides.
> Given that it is not super easy to run into this `.min(1)` is probably
> enough here. Not sure if checking for 0 in the endpoint when setting the
> limit makes sense.
Yes, will check that as well for version 2 of the patches, thanks!
>> + )
>> + }
>> +}
>> +
>> +/// Token bucket based rate limiter
>> +///
>> +/// IMPORTANT: We use this struct in shared memory, so please do not
>> +/// change/modify the layout (do not add fields)
>> +#[repr(C)]
>> +pub struct RateLimiter {
>> + rate: u64, // tokens/second
>> + bucket_size: u64, // TBF bucket size
>> + state: TbfState,
>> +}
>> +
>> +impl RateLimiter {
>> + /// Creates a new instance, using [Instant::now] as start time.
>> + pub fn new(rate: u64, bucket_size: u64) -> Self {
>> + let start_time = Instant::now();
>> + Self::with_start_time(rate, bucket_size, start_time)
>> + }
>> +
>> + /// Creates a new instance with specified `rate`, `bucket_size`
>> and `start_time`.
>> + pub fn with_start_time(rate: u64, bucket_size: u64, start_time:
>> Instant) -> Self {
>> + Self {
>> + rate,
>> + bucket_size,
>> + state: TbfState {
>> + traffic: 0,
>> + last_update: start_time,
>> + // start with empty bucket (all tokens consumed)
>> + consumed_tokens: bucket_size,
>> + },
>> + }
>> + }
>> +}
>> +
>> +impl RateLimit for RateLimiter {
>> + fn update_rate(&mut self, rate: u64, bucket_size: u64) {
>> + self.rate = rate;
>> +
>> + if bucket_size < self.bucket_size &&
>> self.state.consumed_tokens > bucket_size {
>> + self.state.consumed_tokens = bucket_size; // start again
>> + }
>> +
>> + self.bucket_size = bucket_size;
>> + }
>> +
>> + fn traffic(&self) -> u64 {
>> + self.state.traffic
>> + }
>> +
>> + fn register_traffic(&mut self, current_time: Instant, data_len:
>> u64) -> Duration {
>> + self.state
>> + .register_traffic(self.rate, self.bucket_size,
>> current_time, data_len)
>> + }
>> +}
>> +
>> +impl<R: RateLimit + Send> ShareableRateLimit for std::sync::Mutex<R> {
>> + fn update_rate(&self, rate: u64, bucket_size: u64) {
>> + self.lock().unwrap().update_rate(rate, bucket_size);
>> + }
>> +
>> + fn traffic(&self) -> u64 {
>> + self.lock().unwrap().traffic()
>> + }
>> +
>> + fn register_traffic(&self, current_time: Instant, data_len: u64)
>> -> Duration {
>> + self.lock()
>> + .unwrap()
>> + .register_traffic(current_time, data_len)
>> + }
>> +}
>> +
>> +/// Array of rate limiters.
>> +///
>> +/// A group of rate limiters with same configuration.
>> +pub struct RateLimiterVec {
>> + rate: u64, // tokens/second
>> + bucket_size: u64, // TBF bucket size
>> + state: Vec<TbfState>,
>> +}
>> +
>> +impl RateLimiterVec {
>> + /// Creates a new instance, using [Instant::now] as start time.
>> + pub fn new(group_size: usize, rate: u64, bucket_size: u64) -> Self {
>> + let start_time = Instant::now();
>> + Self::with_start_time(group_size, rate, bucket_size, start_time)
>> + }
>> +
>> + /// Creates a new instance with specified `rate`, `bucket_size`
>> and `start_time`.
>> + pub fn with_start_time(
>> + group_size: usize,
>> + rate: u64,
>> + bucket_size: u64,
>> + start_time: Instant,
>> + ) -> Self {
>> + let state = TbfState {
>> + traffic: 0,
>> + last_update: start_time,
>> + // start with empty bucket (all tokens consumed)
>> + consumed_tokens: bucket_size,
>> + };
>> + Self {
>> + rate,
>> + bucket_size,
>> + state: vec![state; group_size],
>> + }
>> + }
>> +
>> + #[allow(clippy::len_without_is_empty)]
>> + /// Return the number of TBF entries (group_size)
>> + pub fn len(&self) -> usize {
>> + self.state.len()
>> + }
>> +
>> + /// Traffic for the specified index
>> + pub fn traffic(&self, index: usize) -> Result<u64, Error> {
>> + if index >= self.state.len() {
>> + bail!("RateLimiterVec::traffic - index out of range");
>> + }
>> + Ok(self.state[index].traffic)
>> + }
>> +
>> + /// Register traffic at the specified index
>> + pub fn register_traffic(
>> + &mut self,
>> + index: usize,
>> + current_time: Instant,
>> + data_len: u64,
>> + ) -> Result<Duration, Error> {
>> + if index >= self.state.len() {
>> + bail!("RateLimiterVec::register_traffic - index out of
>> range");
>> + }
>> +
>> + Ok(self.state[index].register_traffic(self.rate,
>> self.bucket_size, current_time, data_len))
>> + }
>> +}
>> diff --git a/proxmox-rate-limiter/src/shared_rate_limiter.rs b/
>> proxmox-rate-limiter/src/shared_rate_limiter.rs
>> new file mode 100644
>> index 00000000..2822e7ea
>> --- /dev/null
>> +++ b/proxmox-rate-limiter/src/shared_rate_limiter.rs
>> @@ -0,0 +1,130 @@
>> +//! Rate limiter designed for shared memory
>> +
>> +use std::mem::MaybeUninit;
>> +use std::path::Path;
>> +use std::time::{Duration, Instant};
>> +
>> +use anyhow::{bail, Error};
>> +use nix::sys::stat::Mode;
>> +use nix::unistd::User;
>> +
>> +use proxmox_shared_memory::{check_subtype, initialize_subtype};
>> +use proxmox_shared_memory::{Init, SharedMemory, SharedMutex};
>> +use proxmox_sys::fs::{create_path, CreateOptions};
>> +
>> +use crate::{RateLimit, RateLimiter, ShareableRateLimit};
>> +
>> +/// Magic number for shared rate limiter exposed file mappings
>> +///
>> +/// Generated by `openssl::sha::sha256(b"Proxmox Backup
>> SharedRateLimiter v1.0")[0..8];`
>> +/// Original magic number kept when factored out from the initial
>> +/// PBS implementation for full backwards compatibility.
>> +pub const PROXMOX_SHARED_RATE_LIMITER_MAGIC_1_0: [u8; 8] = [6, 58,
>> 213, 96, 161, 122, 130, 117];
>> +
>> +// Wrap RateLimiter, so that we can provide an Init impl
>> +#[repr(C)]
>> +struct WrapLimiter(RateLimiter);
>> +
>> +impl Init for WrapLimiter {
>> + fn initialize(this: &mut MaybeUninit<Self>) {
>> + // default does not matter here, because we override later
>> + this.write(WrapLimiter(RateLimiter::new(1_000_000, 1_000_000)));
>> + }
>> +}
>> +
>> +#[repr(C)]
>> +struct SharedRateLimiterData {
>> + magic: [u8; 8],
>> + tbf: SharedMutex<WrapLimiter>,
>> + padding: [u8; 4096 - 104],
>> +}
>> +
>> +impl Init for SharedRateLimiterData {
>> + fn initialize(this: &mut MaybeUninit<Self>) {
>> + unsafe {
>> + let me = &mut *this.as_mut_ptr();
>> + me.magic = PROXMOX_SHARED_RATE_LIMITER_MAGIC_1_0;
>> + initialize_subtype(&mut me.tbf);
>> + }
>> + }
>> +
>> + fn check_type_magic(this: &MaybeUninit<Self>) -> Result<(), Error> {
>> + unsafe {
>> + let me = &*this.as_ptr();
>> + if me.magic != PROXMOX_SHARED_RATE_LIMITER_MAGIC_1_0 {
>> + bail!("SharedRateLimiterData: wrong magic number");
>> + }
>> + check_subtype(&me.tbf)?;
>> + Ok(())
>> + }
>> + }
>> +}
>> +
>> +/// Rate limiter designed for shared memory ([SharedMemory])
>> +///
>> +/// The actual [RateLimiter] is protected by a [SharedMutex] and
>> +/// implements [Init]. This way we can share the limiter between
>> +/// different processes.
>> +pub struct SharedRateLimiter {
>> + shmem: SharedMemory<SharedRateLimiterData>,
>> +}
>> +
>> +impl SharedRateLimiter {
>> + /// Creates a new mmap'ed instance.
>> + ///
>> + /// Data is mapped in `<base_path>/<name>` using
>> + /// `TMPFS`.
>> + pub fn mmap_shmem<P: AsRef<Path>>(
>> + name: &str,
>> + rate: u64,
>> + burst: u64,
>> + user: User,
>> + base_path: P,
>> + ) -> Result<Self, Error> {
>> + let mut path = base_path.as_ref().to_path_buf();
>> +
>> + let dir_opts = CreateOptions::new()
>> + .perm(Mode::from_bits_truncate(0o770))
>> + .owner(user.uid)
>> + .group(user.gid);
>> +
>> + create_path(&path, Some(dir_opts), Some(dir_opts))?;
>> +
>> + path.push(name);
>> +
>> + let file_opts = CreateOptions::new()
>> + .perm(Mode::from_bits_truncate(0o660))
>> + .owner(user.uid)
>> + .group(user.gid);
>> +
>> + let shmem: SharedMemory<SharedRateLimiterData> =
>> SharedMemory::open(&path, file_opts)?;
>> +
>> + shmem.data().tbf.lock().0.update_rate(rate, burst);
>> +
>> + Ok(Self { shmem })
>> + }
>> +}
>> +
>> +impl ShareableRateLimit for SharedRateLimiter {
>> + fn update_rate(&self, rate: u64, bucket_size: u64) {
>> + self.shmem
>> + .data()
>> + .tbf
>> + .lock()
>> + .0
>> + .update_rate(rate, bucket_size);
>> + }
>> +
>> + fn traffic(&self) -> u64 {
>> + self.shmem.data().tbf.lock().0.traffic()
>> + }
>> +
>> + fn register_traffic(&self, current_time: Instant, data_len: u64)
>> -> Duration {
>> + self.shmem
>> + .data()
>> + .tbf
>> + .lock()
>> + .0
>> + .register_traffic(current_time, data_len)
>> + }
>> +}
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances
2025-11-11 10:49 ` [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Hannes Laimer
@ 2025-11-11 15:10 ` Christian Ebner
0 siblings, 0 replies; 21+ messages in thread
From: Christian Ebner @ 2025-11-11 15:10 UTC (permalink / raw)
To: Hannes Laimer, Proxmox Backup Server development discussion
On 11/11/25 11:49 AM, Hannes Laimer wrote:
> I did run into some conflicts when applying, mostly control files, other
> than that and a small div by zero problem this L(very)GTM!
>
> I did test this with a local S3 storage, worked as advertised.
>
>
> Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
> Tested-by: Hannes Laimer <h.laimer@proxmox.com>
Thanks a lot for review and testing, will address the issues and rebase
for version 2.
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [pbs-devel] superseded: [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances
2025-09-16 12:41 [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Christian Ebner
` (8 preceding siblings ...)
2025-11-11 10:49 ` [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Hannes Laimer
@ 2025-11-12 11:55 ` Christian Ebner
9 siblings, 0 replies; 21+ messages in thread
From: Christian Ebner @ 2025-11-12 11:55 UTC (permalink / raw)
To: pbs-devel
superseded-by version 3:
https://lore.proxmox.com/pbs-devel/20251112115353.277514-1-c.ebner@proxmox.com/T/
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-11-12 11:55 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-16 12:41 [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Christian Ebner
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 1/4] rate-limiter: add crate for traffic rate limiter implementations Christian Ebner
2025-11-11 10:34 ` Hannes Laimer
2025-11-11 15:06 ` Christian Ebner
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 2/4] http: drop factored out rate limiter implementation Christian Ebner
2025-11-11 10:36 ` Hannes Laimer
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 3/4] rest-server: optionally depend on factored out shared rate limiter Christian Ebner
2025-11-11 10:42 ` Hannes Laimer
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox v2 4/4] s3-client: add shared rate limiter via https connector Christian Ebner
2025-11-11 10:41 ` Hannes Laimer
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 1/4] traffic control: use factored out shared rate limiter Christian Ebner
2025-11-11 10:45 ` Hannes Laimer
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 2/4] api: config: update s3 endpoint rate limits in config Christian Ebner
2025-11-11 10:45 ` Hannes Laimer
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 3/4] datastore: s3: set rate limiter options for s3 client Christian Ebner
2025-11-11 10:46 ` Hannes Laimer
2025-09-16 12:41 ` [pbs-devel] [PATCH proxmox-backup v2 4/4] ui: expose rate and burst limits for s3 endpoints Christian Ebner
2025-11-11 10:46 ` Hannes Laimer
2025-11-11 10:49 ` [pbs-devel] [PATCH proxmox{, -backup} v2 0/8] shared rate limiter for s3 client instances Hannes Laimer
2025-11-11 15:10 ` Christian Ebner
2025-11-12 11:55 ` [pbs-devel] superseded: " Christian Ebner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.