Surprising behaviour in x/time/rate

[James Lees]

Hi there,

I've never posted here, so apologies if I'm breaching any etiquette, but the contribution guide suggests posting here before creating an issue so I thought I'd try.

I was debugging an issue recently and saw some strange behaviour in the x/time/rate package.

The behaviour is exhibited in the following test

func TestRateIssue(t *testing.T) {
l := NewLimiter(0, 1)
fmt.Println(l.Allow()) // should be true
fmt.Println(l.Allow()) // should be false

l.SetLimit(10)

time.Sleep(1 * time.Second)
fmt.Println(l.Allow()) // should be true but is false.

}

This code seems very strange to me. If the burst is 1, decrementing it here means that the limiter becomes unusable even if the limit is subsequently increased.

This code appeared here but the conversation doesn't really reflect the code: the comment says:

> The opposite in fact needs to happen: lim.tokens must be reduced by the n consumed tokens?

Which makes sense to me, but I'm not sure how we ended up decrementing the burst. To me the solution to the reported problem (limiters not actually being full) would be to set lim.tokens on the constructor, but I haven't thought about that too deeply.

I'm happy to propose a change/create an issue but hopefully you folks can help me understand whether I'm missing something obvious!

Thanks

James

--

This email is confidential and protected by copyright, and might contain privileged information. The same goes for any attachments.

If we’ve sent it to you by mistake (sorry), please don’t copy it or show it to anyone. You also shouldn’t use it to make a decision, and you shouldn’t rely on the contents.  Let the sender know as soon as you can, and then delete the email. Thank you!

Monzo Bank Limited is a company registered in England and Wales (No. 09446231) registered at Broadwalk House, 5 Appold St, London, EC2A 2AG. Monzo Bank Ltd is authorised by the Prudential Regulation Authority (PRA) and regulated by the Financial Conduct Authority and the PRA. Our Financial Services Register number is 730427.

[Ian Lance Taylor]

On Fri, Jul 19, 2024 at 1:52 PM 'James Lees' via golang-nuts
<golan...@googlegroups.com> wrote:
>
> Hi there,
> I've never posted here, so apologies if I'm breaching any etiquette, but the contribution guide suggests posting here before creating an issue so I thought I'd try.
>
> I was debugging an issue recently and saw some strange behaviour in the x/time/rate package.
>
> The behaviour is exhibited in the following test
>
> func TestRateIssue(t *testing.T) {
> l := NewLimiter(0, 1)
> fmt.Println(l.Allow()) // should be true
> fmt.Println(l.Allow()) // should be false
>
> l.SetLimit(10)
>
> time.Sleep(1 * time.Second)
> fmt.Println(l.Allow()) // should be true but is false.
> }
>
> This code seems very strange to me. If the burst is 1, decrementing it here means that the limiter becomes unusable even if the limit is subsequently increased.
>
> This code appeared here but the conversation doesn't really reflect the code: the comment says:
>
> > The opposite in fact needs to happen: lim.tokens must be reduced by the n consumed tokens?
>
> Which makes sense to me, but I'm not sure how we ended up decrementing the burst. To me the solution to the reported problem (limiters not actually being full) would be to set lim.tokens on the constructor, but I haven't thought about that too deeply.
>
> I'm happy to propose a change/create an issue but hopefully you folks can help me understand whether I'm missing something obvious!

I agree that this seems wrong. Want to open a bug report or send a patch?

Ian