Throttle #97
Throttle #97
Conversation
# Conflicts: # go.sum
wire limiter with limiterUserHandler and limiterSystemHandler
| } | ||
| } | ||
|
|
||
| if httpError := tollbooth.LimitByKeys(lmt, keys); httpError != nil { |
There was a problem hiding this comment.
There appears to be a bug in tollbooth, where they stop iterating over the keys as soon as they detect that the error is returned.
However, we actually need to iterate over all keys all the time, otherwise, we're leaking limiter permits (or, more precisely under-counting the permits for the keys that follow the first key that exceeded the limit).
Potentially worth copying the method from tollbooth and re-implementing it here.
| lmt := tollbooth.NewLimiter(float64(reqSec), nil) | ||
| fn := func(w http.ResponseWriter, r *http.Request) { | ||
| if httpError := tollbooth.LimitByKeys(lmt, []string{"system"}); httpError != nil { | ||
| http.Error(w, http.StatusText(http.StatusTooManyRequests), http.StatusTooManyRequests) |
There was a problem hiding this comment.
Well... Technically it's too early to return an error here. We also need to go and check routes before returning. Essentially we need to check all the keys even though we already know that the request should not be admitted into the system (see also the comment below for the explanation).
There was a problem hiding this comment.
I see what you mean in both comments, good points. But I think we may ignore both issues for now and undercount hits in those cases. Practically it will limit excessive activity good enough, not as precise as it can but still ok.
implements simple throttling #91