feat(config): make adaptive_rate_limit optional in server config#1734
Merged
feat(config): make adaptive_rate_limit optional in server config#1734
Conversation
Signed-off-by: Gaius <gaius.qi@gmail.com>
CormickKneey
approved these changes
Mar 18, 2026
EvanCley
approved these changes
Mar 18, 2026
Member
|
LGTM |
There was a problem hiding this comment.
Pull request overview
This PR makes the dfdaemon gRPC servers’ BBR-based adaptive rate limiting configurable/optional by changing the server config from a mandatory BBRConfig to Option<BBRConfig>, and conditionally applying the BBR middleware layer only when configured.
Changes:
- Change
Server.adaptive_rate_limittoOption<BBRConfig>with a default ofNone(disabled by default). - Update download/upload gRPC server structs to carry
Option<Arc<BBR>>and applyBBRLayerviatower::util::option_layer. - Update dfdaemon startup to only initialize
BBRwhenadaptive_rate_limitis configured.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
dragonfly-client-config/src/dfdaemon.rs |
Makes adaptive rate limit config optional and disables it by default. |
dragonfly-client/src/bin/dfdaemon/main.rs |
Conditionally initializes the BBR limiter based on optional config. |
dragonfly-client/src/grpc/dfdaemon_download.rs |
Makes BBR middleware optional and conditionally layers it into the server. |
dragonfly-client/src/grpc/dfdaemon_upload.rs |
Makes BBR middleware optional and conditionally layers it into the server. |
Comment on lines
405
to
412
| /// BBR-inspired adaptive rate limiter configuration for gRPC servers (download & upload). | ||
| /// | ||
| /// When system CPU or memory usage exceeds the configured thresholds, the limiter | ||
| /// estimates capacity via `max_pass × min_rt × bucket_count / 1000` and sheds | ||
| /// incoming requests whose in-flight count exceeds this estimate. A cooldown | ||
| /// period prevents rapid oscillation between shedding and accepting. | ||
| pub adaptive_rate_limit: BBRConfig, | ||
| pub adaptive_rate_limit: Option<BBRConfig>, | ||
| } |
Comment on lines
414
to
421
| @@ -417,7 +417,7 @@ | |||
| Server { | |||
| plugin_dir: default_dfdaemon_plugin_dir(), | |||
| cache_dir: default_dfdaemon_cache_dir(), | |||
| adaptive_rate_limit: BBRConfig::default(), | |||
| adaptive_rate_limit: None, | |||
| } | |||
| @@ -113,7 +114,7 @@ pub struct DfdaemonDownloadServer { | |||
| persistent_cache_task: Arc<persistent_cache_task::PersistentCacheTask>, | |||
|
|
|||
| /// BBR rate limiter middleware for adaptive rate limiting based on system load. | |||
| @@ -108,7 +109,7 @@ pub struct DfdaemonUploadServer { | |||
| system_monitor: Arc<SystemMonitor>, | |||
|
|
|||
| /// BBR rate limiter middleware for adaptive rate limiting based on system load. | |||
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1734 +/- ##
==========================================
- Coverage 46.89% 46.86% -0.04%
==========================================
Files 87 87
Lines 24811 24815 +4
==========================================
- Hits 11636 11629 -7
- Misses 13175 13186 +11
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request makes the adaptive BBR (Bottleneck Bandwidth and Round-trip propagation time) rate limiting feature optional in the Dragonfly client configuration and gRPC server implementations. The main goal is to allow the servers to run without BBR-based rate limiting when it is not configured, improving flexibility and configurability.
Configuration changes:
adaptive_rate_limitfield in theServerstruct to be anOption<BBRConfig>, and updated its default value toNone, making adaptive rate limiting optional in the configuration (dragonfly-client-config/src/dfdaemon.rs). [1] [2]gRPC server changes (Download/Upload):
DfdaemonDownloadServerandDfdaemonUploadServerstructs to make thebbrfield optional (Option<Arc<BBR>>), and adjusted their constructors accordingly (dragonfly-client/src/grpc/dfdaemon_download.rs,dragonfly-client/src/grpc/dfdaemon_upload.rs). [1] [2] [3] [4]option_layer, so the BBR rate limiting middleware is only applied if configured (dragonfly-client/src/grpc/dfdaemon_download.rs,dragonfly-client/src/grpc/dfdaemon_upload.rs). [1] [2]Dependency and import updates:
tower::util::option_layerwhere necessary to support conditional middleware application. [1] [2]Related Issue
Motivation and Context
Screenshots (if appropriate)