uber-go
Adding support for an `Allow` method
This package initially seemed ideal for my use case - It supports refilling at a certain rate (so limits such as 10 per 2 minutes are possible) however only exposing Take means that it can't be used in a context where you want to return a 429 HTTP response (this would block the response which is not the behaviour I'm looking for).
I have seen mentioned on other issues that no other features are planned, but I wanted to ask - if I made a pull request which supported an Allow method which simply returns whether a call to Take would block or not, would a merge be considered for this?
The feature on its own sounds reasonable, however but before any PRs we should discuss what exactly Allow would do.
- does it just
peakor alsopopthe available limit? Should it take some arguments likeAllow(count int)Or should it perhaps beAllow(count int, at time.Time)? - how does it work with
Take? Do we allow both to be called, or do they work exclusively? - I'm not entirely sure whether you can squeeze it into the code easily, the implementation assumes "moving time"
Most important, have you considered just using https://pkg.go.dev/golang.org/x/time/rate#Limiter.Allow? This sounds like exactly what you need, and I don't see value in copying the the same functionality/API here.
In regards to how the function would work, I envisaged it peeking the limit, and providing both a count and time as paramters would be ideal. It shouldn't affect the functionality of take, and both should be able to work in harmony.
I have looked into using the Allow method from time/rate, however the issue I experienced was that there is no Per option like in this package. This makes rates such as '5 per 5 minutes' impossible, because the bucket refills at a rate proportional to the limit.
If you think that implementing this functionality here wouldn't be worthwhile/it would be more suitable to request an implementation of Per in time/rate I would be happy to go down that route instead 😄
I have looked into using the Allow method from time/rate, however the issue I experienced was that there is no Per option like in this package. This makes rates such as '5 per 5 minutes' impossible, because the bucket refills at a rate proportional to the limit.
Hm, I'm now not sure if I understand x/time/rate correctly. Per https://pkg.go.dev/golang.org/x/time/rate#Limit we see
Limit defines the maximum frequency of some events. Limit is represented as number of events per second. A zero Limit allows no events.
If I read this correctly, you could provide a very small float (0.016), that would effectively be the same as our Per option. As far as I understand, buckets refill in the same way here and in x/time/rate (?).
Perhaps it's our documentation that's unclear - if you do 5 per 5 minutes here you will effectively get a token every minute, rather than 5 every 5 minutes. Per is just a syntactic sugar to make the division simpler.
Perhaps it's our documentation that's unclear - if you do 5 per 5 minutes here you will effectively get a token every minute, rather than 5 every 5 minutes. Per is just a syntactic sugar to make the division simpler.
Ah I see, I was under the impression that your package differed from x/time/rate in this manner. I'll look into making my own solution, sorry for the bother!
A token bucket system where the refill rate can be entirely customised - for example you could create a new limiter with NewLimiter(5, 5 * time.Minute) and this would refill the bucket to 5 after the 5 minute interval.
The initial requirement came from limiting an endpoint to 5 requests every 5 minutes, which doesn't seem possible with any current offering because this duration is simplified into '1 per minute up to a maximum of 5'.
Yeah, but could you bubble up a bit on the stack? What use-case requires this kind of rate-limiting?
The initial requirement came from limiting an endpoint to 5 requests every 5 minutes, which doesn't seem possible with any current offering because this duration is simplified into '1 per minute up to a maximum of 5'.
So that's not entirely precise. With Slack, if you don't make requests for 5 minutes, the system will allow you to make 5 calls immediately. So you get the bustiness - if that's the requirement. After that it will be 1 per minute though, yeah.
What use-case requires this kind of rate-limiting?
The requirement came from the desire to limit file uploads to a strict limit per 5-minute period. With slack/burst they'd be able to upload 5 files immediately, followed by another 5 across the next 5 minute period (because the bucket would refill at a rate of 1 per minute), which was not ideal.
As it seems most packages act like this, I'll probably end up just using this functionality. Thanks for the help!