This is an automated email from the ASF dual-hosted git repository.
liaoxin01 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new afc3c35e18c [fix](cloud) make S3 rate limiter config take effect
dynamically in cloud mode (#64554)
afc3c35e18c is described below
commit afc3c35e18c8248d93331b55aba703857c29de18
Author: Xin Liao <[email protected]>
AuthorDate: Thu Jun 25 16:04:53 2026 +0800
[fix](cloud) make S3 rate limiter config take effect dynamically in cloud
mode (#64554)
## Proposed changes
### Problem
In cloud mode, dynamically modifying the S3 rate limiter configs
(`s3_get_token_per_second` / `s3_put_token_per_second` and the related
`*_bucket_tokens` / `*_token_limit`) does not take effect.
The running rate limiter only reads these configs when the
`TokenBucketRateLimiter` object is rebuilt via
`S3RateLimiterHolder::reset()`, and the only trigger is
`S3ClientFactory::create()` -> `check_s3_rate_limiter_config_changed()`.
In a steady cloud cluster, the periodic vault refresh
(`_refresh_storage_vault_info_thread_callback`, every
`refresh_s3_info_interval_s`) goes through `ObjClientHolder::reset()`
for existing vaults, which short-circuits and returns early when the
conf hash is unchanged. So `S3ClientFactory::create()` is never called,
and the modified configs are never applied — until a vault is added, its
conf changes, or the BE restarts.
### Fix
Call `check_s3_rate_limiter_config_changed()` in the vault refresh
thread so the rate limiter picks up config changes within
`refresh_s3_info_interval_s` (default 60s) regardless of whether any
vault is created or its conf changes. The check is a cheap no-op when
nothing changed.
The call is gated behind `config::enable_s3_rate_limiter` so that
clusters with rate limiting disabled (or HDFS-only vaults) do not
force-initialize `S3ClientFactory` / the AWS SDK on the first tick (the
`last_*` trackers start at 0 while the configs default to non-zero). The
rate limiter enforcement path is already gated by the same config, so
this is consistent.
### Changes
- `be/src/util/s3_util.h`: declare
`check_s3_rate_limiter_config_changed()` so it can be called externally.
- `be/src/cloud/cloud_storage_engine.cpp`: invoke it each tick in
`_refresh_storage_vault_info_thread_callback()`, gated by
`enable_s3_rate_limiter`.
- `be/test/util/s3_util_test.cpp`: add a unit test verifying the limiter
is rebuilt when the configs change.
---
be/src/cloud/cloud_storage_engine.cpp | 9 +++++++
be/src/util/s3_util.h | 3 +++
be/test/util/s3_util_test.cpp | 45 +++++++++++++++++++++++++++++++++++
3 files changed, 57 insertions(+)
diff --git a/be/src/cloud/cloud_storage_engine.cpp
b/be/src/cloud/cloud_storage_engine.cpp
index 169486bf256..db5c4576160 100644
--- a/be/src/cloud/cloud_storage_engine.cpp
+++ b/be/src/cloud/cloud_storage_engine.cpp
@@ -445,6 +445,15 @@ void
CloudStorageEngine::_refresh_storage_vault_info_thread_callback() {
while (!_stop_background_threads_latch.wait_for(
std::chrono::seconds(config::refresh_s3_info_interval_s))) {
sync_storage_vault();
+ // The other place that rebuilds the S3 rate limiter is
S3ClientFactory::create(), which
+ // is not called when an existing vault's conf is unchanged. Trigger
the check here as well
+ // so that dynamically modified s3_{get,put}_* rate limiter configs
take effect within
+ // refresh_s3_info_interval_s even when no vault is created or its
conf does not change.
+ // Gate it behind enable_s3_rate_limiter so that clusters with rate
limiting disabled
+ // (e.g. HDFS-only vaults) do not force-initialize S3ClientFactory /
the AWS SDK here.
+ if (config::enable_s3_rate_limiter) {
+ check_s3_rate_limiter_config_changed();
+ }
}
}
diff --git a/be/src/util/s3_util.h b/be/src/util/s3_util.h
index 636c78f1b65..5546db857c9 100644
--- a/be/src/util/s3_util.h
+++ b/be/src/util/s3_util.h
@@ -64,6 +64,9 @@ extern bvar::LatencyRecorder s3_copy_object_latency;
std::string hide_access_key(const std::string& ak);
int reset_s3_rate_limiter(S3RateLimitType type, size_t max_speed, size_t
max_burst, size_t limit);
+// Rebuild the S3 GET/PUT rate limiters if the related configs have changed.
+// Safe to call periodically; it is a no-op when nothing changed.
+void check_s3_rate_limiter_config_changed();
class S3URI;
struct S3ClientConf {
diff --git a/be/test/util/s3_util_test.cpp b/be/test/util/s3_util_test.cpp
index cd8eeb5cb68..a87d0e9a2d2 100644
--- a/be/test/util/s3_util_test.cpp
+++ b/be/test/util/s3_util_test.cpp
@@ -21,6 +21,7 @@
#include <string>
+#include "common/config.h"
#include "gtest/gtest_pred_impl.h"
#include "util/s3_uri.h"
@@ -76,4 +77,48 @@ TEST_F(S3UTILTest, hide_access_key_typical_aws_key) {
EXPECT_EQ("xxxxxxxFODNN7xxxxxxx", result);
}
+// Verifies that check_s3_rate_limiter_config_changed() rebuilds the global
GET rate
+// limiter when the related configs change. This is the behavior the cloud
vault refresh
+// thread relies on to apply dynamically modified s3_get_* rate limiter
configs without
+// having to (re)create an S3 client.
+TEST_F(S3UTILTest, check_s3_rate_limiter_config_changed_rebuilds_limiter) {
+ auto* get_limiter =
S3ClientFactory::instance().rate_limiter(S3RateLimitType::GET);
+ ASSERT_NE(get_limiter, nullptr);
+
+ // Save originals so other tests are not affected.
+ const int64_t orig_tps = config::s3_get_token_per_second;
+ const int64_t orig_bucket = config::s3_get_bucket_tokens;
+ const int64_t orig_limit = config::s3_get_token_limit;
+
+ // Establish a known baseline (no count limit, no throttling).
+ config::s3_get_token_per_second = 1000000000;
+ config::s3_get_bucket_tokens = 1000000000;
+ config::s3_get_token_limit = 0;
+ check_s3_rate_limiter_config_changed();
+
+ // Impose a hard request-count limit of 3. Since the limit value changes
(0 -> 3),
+ // the limiter is rebuilt with a fresh counter.
+ config::s3_get_token_limit = 3;
+ check_s3_rate_limiter_config_changed();
+
+ // The bucket/speed are huge so add() never throttles (returns 0); only
the count
+ // limit takes effect: the first 3 requests pass, the 4th is rejected (-1).
+ EXPECT_GE(get_limiter->add(1), 0);
+ EXPECT_GE(get_limiter->add(1), 0);
+ EXPECT_GE(get_limiter->add(1), 0);
+ EXPECT_LT(get_limiter->add(1), 0);
+
+ // Raise the limit. The checker must rebuild the limiter so the exhausted
counter is
+ // reset; otherwise the next request would still be rejected.
+ config::s3_get_token_limit = 100;
+ check_s3_rate_limiter_config_changed();
+ EXPECT_GE(get_limiter->add(1), 0);
+
+ // Restore original configs and apply them back to the limiter.
+ config::s3_get_token_per_second = orig_tps;
+ config::s3_get_bucket_tokens = orig_bucket;
+ config::s3_get_token_limit = orig_limit;
+ check_s3_rate_limiter_config_changed();
+}
+
} // end namespace doris
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
