Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RateLimiting] Handle Timer jitter #74360

Merged
merged 6 commits into from
Sep 2, 2022

Conversation

BrennanConroy
Copy link
Member

@BrennanConroy BrennanConroy commented Aug 22, 2022

Fixes #74056

Two approaches are used here, we "trust the timer" when auto replenish is enabled so FixedWindow and SlidingWindow will always move the window when the timer fires. And in the token bucket case we change to use a fill rate internally and update with partial tokens when replenish is called, no matter how frequently.

Also added validation for not passing TimeSpan.Zero to the limiters, but we could revert that if we just want TimeSpan.Zero to mean never replenish (Timer already uses TimeSpan.Zero as never run the timer).

@ghost
Copy link

ghost commented Aug 22, 2022

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #74056

Two approaches are used here, we "trust the timer" when auto replenish is enabled so FixedWindow and SlidingWindow will always move the window when the timer fires. And in the token bucket case we change to use a fill rate internally and update with partial tokens when replenish is called, no matter how frequently.

Also added validation for not passing TimeSpan.Zero to the limiters, but we could revert that if we just want TimeSpan.Zero to mean never replenish (Timer already uses TimeSpan.Zero as never run the timer).

Author: BrennanConroy
Assignees: -
Labels:

area-System.Threading

Milestone: -

@BrennanConroy
Copy link
Member Author

/backport to release/7.0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2022

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/2975631117

@BrennanConroy
Copy link
Member Author

Failures are tracked by #74952 unrelated to this change. Merging.

@BrennanConroy BrennanConroy merged commit 72a82fe into dotnet:main Sep 2, 2022
@BrennanConroy BrennanConroy deleted the brecon/time branch September 2, 2022 16:14
@BrennanConroy BrennanConroy added this to the 7.0.0 milestone Sep 2, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Oct 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System.Threading.Ratelimiting does not effectively maximize requests for its parameters
3 participants