Skip to content

ratelimitprocessor: ThrottleBehaviorDelay misinterprets reset time #619

@axw

Description

@axw

The rate limit processor has blocking (delay) and non-blocking (error) modes. By default it is non-blocking.

The blocking mode is handled in this code:

case ThrottleBehaviorDelay:
delay := time.Duration(resp.GetResetTime()-createdAt) * time.Millisecond
timer := time.NewTimer(delay)
defer timer.Stop()
select {
case <-ctx.Done():
return ctx.Err()
case <-timer.C:
}

This code is essentially sleeping until the reset time, and then continuing. The reset time is when the rate limit resets - see https://github.com/gubernator-io/gubernator/blob/427194a6c593e6d9da7283fbaea49939052c8676/gubernator.proto#L204-L205. Imagine the rate limit is exhausted and >1 clients hit this code path: they will both be allowed through at the reset time, regardless of how many items they request.

I think there should be a loop here: sleep until the reset, then request again with the remaining items, and repeat as necessary.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions