Add DedupingBucketRateLimiter #6025
Merged
Conversation
This file contains 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6025 +/- ##
==========================================
- Coverage 37.14% 37.05% -0.09%
==========================================
Files 1313 1316 +3
Lines 131619 132100 +481
==========================================
+ Hits 48891 48951 +60
- Misses 78481 78896 +415
- Partials 4247 4253 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Our use of a `BucketRateLimiter` with `DelayingQueue` is problematic when repeatedly inserting the same items. The former continually moves out the rate limit token time with each new item (no deduping), while the latter dedupes insertions. The net effect is that under load newly inserted items end up with a token time way in the future while the queue happily drains to empty with items processed at a much lower than expected rate. This PR adds a new `DedupingBucketRateLimiter` that maintains historic reservations per item and skips taking out a new reservation when a duplicate item is found with an outstanding reservation. Reservations are dropped from the map when `Forget` is called, which happens after the item is fully processed. This is similar to how the exponential backoff rate limiter works. See [here](https://github.com/kubernetes/client-go/blob/master/util/workqueue/default_rate_limiters.go#L82-L149). - [x] Run new rate limiter under high execution load and verify that queue is kept full but not overflown. Signed-off-by: Andrew Dye <[email protected]>
hamersaw
approved these changes
Nov 18, 2024
andrewwdye
force-pushed
the
union/upstream-2f18ffe8f26e9c1e9a403945e6746225ef2fc1e4
branch
from
b3dd9d3 to
224c732
Compare
andrewwdye
deleted the
union/upstream-2f18ffe8f26e9c1e9a403945e6746225ef2fc1e4
branch
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.
Why are the changes needed?
Our use of a
BucketRateLimiterwithDelayingQueueis problematic when repeatedly inserting the same items. The former continually moves out the rate limit token time with each new item (no deduping), while the latter dedupes insertions. The net effect is that under load newly inserted items end up with a token time way in the future while the queue happily drains to empty with items processed at a much lower than expected rate.What changes were proposed in this pull request?
This PR adds a new
DedupingBucketRateLimiterthat maintains historic reservations per item and skips taking out a new reservation when a duplicate item is found with an outstanding reservation. Reservations are dropped from the map whenForgetis called, which happens after the item is fully processed. This is similar to how the exponential backoff rate limiter works. See here.How was this patch tested?
Ran new rate limiter under high execution load and verify that queue is kept full but not overflown.
Check all the applicable boxes