github-actions[bot] commented on code in PR #64038:
URL: https://github.com/apache/doris/pull/64038#discussion_r3356989298
##########
common/cpp/token_bucket_rate_limiter.cpp:
##########
@@ -154,4 +167,53 @@ S3RateLimitType
string_to_s3_rate_limit_type(std::string_view value) {
}
return S3RateLimitType::UNKNOWN;
}
+
+std::function<void(int64_t)> s3_rate_limiter_metric_func(S3RateLimitType type)
{
+ switch (type) {
+ case S3RateLimitType::GET:
+ return metric_func_factory(s3_get_rate_limit_sleep_ns,
s3_get_rate_limit_sleep_count,
+ &s3_get_rate_limit_rejected_count);
+ case S3RateLimitType::PUT:
+ return metric_func_factory(s3_put_rate_limit_sleep_ns,
s3_put_rate_limit_sleep_count,
+ &s3_put_rate_limit_rejected_count);
+ default:
+ return [](int64_t) {};
+ }
+}
+
+int64_t apply_s3_rate_limit(S3RateLimitType type, S3RateLimiterHolder*
rate_limiter,
+ int64_t log_interval) {
+ auto sleep_duration = rate_limiter->add(1);
+ if (log_interval <= 0 || sleep_duration == 0) {
+ return sleep_duration;
+ }
+
+ auto is_get = type == S3RateLimitType::GET;
+ auto* sleep_log_count =
+ is_get ? &s3_get_rate_limit_sleep_log_count :
&s3_put_rate_limit_sleep_log_count;
+ auto* rejected_log_count =
+ is_get ? &s3_get_rate_limit_rejected_log_count :
&s3_put_rate_limit_rejected_log_count;
+
+ if (sleep_duration > 0) {
+ int64_t count = sleep_log_count->fetch_add(1,
std::memory_order_relaxed) + 1;
+ if (count == 1 || count % log_interval == 0) {
+ LOG(INFO) << "S3 " << to_string(type) << " request is throttled by
local rate limiter"
+ << ", sleep_ms=" << sleep_duration / 1000000 << ",
sleep_count=" << count
+ << ", token_per_second=" << rate_limiter->get_max_speed()
+ << ", bucket_tokens=" << rate_limiter->get_max_burst()
+ << ", token_limit=" << rate_limiter->get_limit();
Review Comment:
This log path now reads `rate_limiter->get_max_speed()` / `get_max_burst()`
/ `get_limit()` after `TokenBucketRateLimiterHolder::add()` has returned and
released its `shared_lock`. Both BE and Cloud expose `reset_s3_rate_limiter()`
paths that take the holder write lock and replace the underlying `unique_ptr`
(`rate_limiter = std::make_unique<...>`). A request thread that reaches this
log branch can therefore race with a dynamic reset and dereference the
replaced/freed limiter through these unlocked getters. Please keep the logged
configuration snapshot under the same holder lock as `add()` (or
return/snapshot the values from `add()` while locked) before logging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
