Obvs
Obvs copied to clipboard
christopherread
Burst limit
Is there any interest in adding support for burst? No worries if not but if so my company is using hammer in production and would be happy to contribute a PR adding this feature.
This would be very similar to how nginx works https://www.nginx.com/blog/rate-limiting-nginx/ but the idea is in practice requests are not uniformly distributed so with just a rate limit you have to set the limit to a high level to avoid false positives. With bursts you can set it at a more realistic level but use the burst to catch the false positives.
# Proposed
case Hammer.check_rate("upload_video:#{user_id}", 60_000, 5, 20) do
{:allow, _count} ->
# upload the video, somehow
{:deny, _limit} ->
# deny the request
end
Edit: Actually I was mistaken earlier this cannot be done with two rate limit checks.
👍
@jhchen can you expand on why this can't be done with 2 rate limit checks? We've been using that exact approach and it seems to be working for us. I wonder if there's any edge cases we should be aware of. Thanks!
What I was thinking is if you use two nested checks the request gets rejected if either rate limiter denies it. The burst is meant to only deny if both the rate and the burst is exceeded. But now that you mention it I suppose you could put the 2nd limiter in the deny branch. Is that what you are doing?
@jhchen Thanks, I see. We're doing the opposite, but for our use case it works as for us preventing bursts is maybe more than the rate limit itself. I suppose for most applications bursts are OK and even expected, though.
@jhchen , yup, Totally willing to merge a decent PR that implements this feature :+1: , and thanks for the input!
What I was thinking is if you use two nested checks the request gets rejected if either rate limiter denies it. The burst is meant to only deny if both the rate and the burst is exceeded.
If you want your users to consume a maximum of 1000 resources per day, and allow the burst of 100 in one minute, I guess it still works to have two nested checks for that.
If you do not exceed the 100/minute check but already have consumed 1000 resources today then you should not be allowed to execute burst request, am I right?
