Add throttler rate-limiting support and return 429 when rate-limited#2
Conversation
| if rateLimitingFactor == 0.0 { // no rate limiting | ||
| maxAllowedConcurrency = float64(maxQueueDepth) | ||
| } | ||
| return int64(math.Max(float64(minQueueDepth), math.Min(math.Floor(maxAllowedConcurrency), float64(maxQueueDepth)))) |
There was a problem hiding this comment.
Go has min and max built-in. Not sure if you can use them instead.
There was a problem hiding this comment.
Since we actually don't care that much about exact float values here and the fact that all parameters are non-negative I cast to int and simplified all the min/max
| @@ -161,6 +180,9 @@ func (b *Breaker) InFlight() int { | |||
|
|
|||
| // UpdateConcurrency updates the maximum number of in-flight requests. | |||
| func (b *Breaker) UpdateConcurrency(size int) { | |||
There was a problem hiding this comment.
I know is not your code, but size is not a very insight-giving variable name here. Size of what ?
There was a problem hiding this comment.
it's the size of the semaphore that guards inflight requests to the ksvc. Basically the concurrency an activator is allowed to sent. Do you want that altered?
There was a problem hiding this comment.
As you prefer for our patch, but if you PR this to upstream, I recommend to take the chance to address this.
There was a problem hiding this comment.
I'll leave it as-is. Upstream I would likely address configuration differently and change more things
norman465
force-pushed
the
rate_limit_1_21_2
branch
from
e045384 to
3e10f8a
Compare
norman465
changed the title
norman465
force-pushed
the
rate_limit_1_21_2
branch
from
3e10f8a to
ced1066
Compare
2 participants
revisionConcurrency/num_activatorsdecides how many requests each activator routes and now rate limits