liaoxin01 opened a new pull request, #64554:
URL: https://github.com/apache/doris/pull/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.
   
   ## Further comments
   
   The unit test does not require a real S3 backend, so it runs in normal CI.
   


-- 
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]

Reply via email to