Rate limit replica creation #7869
Conversation
| @@ -64,6 +64,10 @@ const ( | |||
| // 3000 pods. Just creation is limited to 30qps, and watching happens with | |||
| // ~10-30s latency/pod at scale. | |||
| ExpectationsTimeout = 6 * time.Minute | |||
There was a problem hiding this comment.
Can we reduce this timeout now?
There was a problem hiding this comment.
Yeah, was going to send a follow up pr, and observe latencies in production for a bit. That timeout isn't that critical unless we drop events, would you prefer I did so in this one?
There was a problem hiding this comment.
Yeah, why not fix it now, since the TODO will be truthful-but-misleading after this merges.
|
I took a quick look and it generally looks good to me. |
|
Wanted to run density, then ran into a situtation I've been trying to repro for a while and decided to dig into that before destroying the cluster. Density is now done though, PTAL. |
bprashanth
force-pushed
the
rc_rl
branch
2 times, most recently
from
bf4942d
to
f4b9f31
Compare
|
Removed Min function |
| controllerSpec.Spec.Replicas = numReplicas | ||
| manager.controllerStore.Store.Add(controllerSpec) | ||
|
|
||
| for i := 0.0; i < scaleFactor; i++ { |
There was a problem hiding this comment.
suggest don't use floating point; i := 0; i < numReplicas; i += burstReplicas?
There was a problem hiding this comment.
Done, I guess it's clearer this way. Curious if you have another reason for this suggestion?
There was a problem hiding this comment.
Allergic to using comparison operations on floating point numbers :)
(also found the "scale factor" variable name confusing)
|
LGTM |
lavalamp
added
the
lgtm
|
Travis failed with the following error: @bprashanth : Can you take a look? |
|
@nikhiljindal I think that's #6878, I kicked travis, we'll see |
|
Its green now. Merging. |
Rate limit replica creation
Suspend rcs at BurstReplicas until they observe all watch events, so they don't stress out the apiserver. @lavalamp @wojtek-t