[RateLimiter] CompoundLimiter was accepting requests even when some limiters already consumed all tokens #51666
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.
CompoundLimiter is accepting requests when the limit was reached previously.
When processing the limiters and the first one consumes exactly all the remaining tokens (remaining=0, accepted=true) and the next one already reached the limit previously (remaining=0, accepted=0) the $minimalRateLimit is considered the first one that will accept the request (even if it's not the most restrictive).
For example:
CompoundLimiter includes 2 limiters:
After consuming 2 tokens each each limiter generates to limits:
limiter1
->consume(2), generates a limit indicating0
remaining tokens, accepts the request (it was last permitted)limiter2
->consume(2), generates a limit indicating0
remaining tokens, did not accept the request (it did not have 2 tokens to satisfy the request)Because both of them have at this moment
0
remaining tokens, the minimum limit that is returned will be the limit from thelimiter1
. This means that the CompundLimiter will accept the request, even if thelimiter2
should be more restrictive.If we switch the order in the constructor, the request will be denied. The order should not matter.