uber-go / ratelimit

A Go blocking leaky-bucket rate limit implementation
MIT License
4.32k stars 300 forks source link

Take always return without block if it enters the case branch #119

Closed smallnest closed 10 months ago

smallnest commented 10 months ago

https://github.com/uber-go/ratelimit/blob/main/limiter_atomic_int64.go#L72

        case t.maxSlack > 0 && now-timeOfNextPermissionIssue > int64(t.maxSlack):
            // a lot of nanoseconds passed since the last Take call
            // we will limit max accumulated time to maxSlack
            newTimeOfNextPermissionIssue = now - int64(t.maxSlack)

you will find the case always (almost) returns true.

The below code is a test for reproduce:

func TestLimiter(t *testing.T) {
    limiter := ratelimit.New(1, ratelimit.Per(time.Second), ratelimit.WithSlack(1))

    for i := 0; i < 25; i++ {
        if i == 1 {
            time.Sleep(2 * time.Second)
        }
        limiter.Take()

        fmt.Println(time.Now().Unix(), i) // burst

    }
}

ratelimit is v0.3.0

storozhukBM commented 10 months ago

@smallnest can you please try the version from #120 and confirm if it works for you

smallnest commented 10 months ago

yes. Fixed. thanks

spencercjh commented 10 months ago

The fixed PR isn't merged. @smallnest Please re-open the issue to notify the maintainer and other users to pay attention to it. It's a bug that leads to an incident in my prod service. 😢 😭

This issue is also associated with #106 .