[Question] Why sleepFor should >= maxSlack in limiter_atomic.go

[jiekun]

Hi maintainers.

I am new to Go. And would like to raise this issue for the question about:
In Take() method in limiter_atomic.go, sleepFor attr represents how much time we should sleep:

newState.sleepFor += t.perRequest - now.Sub(oldState.last)

So for example we set atelimit.New(1), and last Take() triggered 20 seconds ago. Now newState.sleepFor is calculated as -19.

This attr is used later in t.clock.Sleep(newState.sleepFor). Other than that, it's useless ( Not sure if I am correct). So why sleepFor should >= maxSlack ? I think Sleep(-19) is the same as Sleep(-10) ?

Thank you for your reply in advance :)

[rabbbit]

The key is that sleepFor can be used not in just this iteration, but in the following ones - we keep it for future iterations, to keep track how many "slack" requests we have available.

Technically, you're right - if sleepFor is negative, we indeed don't sleep at all - so from that perspective it's the same to have Sleep(-19) and Sleep(-10).

This could have been just written as

if newState.sleepFor > 0 {
    t.clock.Sleep(newState.sleepFor)
}

and would be equivalent.

Hope this helps - if not, please ask again :)

[jiekun]

Yes I considered this could be (but currently not) used in other cases.

Thank you very much. clear enough

[rabbbit]

It is already used, and it's a critical part of having "slack".

…

On Sun, Jan 24, 2021, 23:17 Jiekun ***@***.***> wrote: Yes I considered this could be (but currently not) used in other cases. Thank you very much. clear enough — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#63 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACG3UQYDZ3JX2MGD4Q4MV3S3TPDVANCNFSM4WQOQH3Q> .