Adding support for an Allow method

[alexgokhale]

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?

[c-harlotte]

Great idea

[rabbbit]

The feature on its own sounds reasonable, however but before any PRs we should discuss what exactly Allow would do.

• does it just peak or also pop the available limit? Should it take some arguments like Allow(count int) Or should it perhaps be Allow(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.

[alexgokhale]

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 😄

[rabbbit]

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 (?).

[rabbbit]

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.

[alexgokhale]

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!

[rabbbit]

Out of curiosity, what's the functionality you're looking for?

[alexgokhale]

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'.

[rabbbit]

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.

[alexgokhale]

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!