Batch updates of multiple Pods. #4531
Conversation
| case <-time.After(kl.resyncInterval): | ||
| glog.V(4).Infof("Periodic sync") | ||
| } | ||
| // If we already caught some update, try to wait for some short time | ||
| // to possibly batch it with other incoming updates. | ||
| for ; unsyncedPod; { |
There was a problem hiding this comment.
nit:
for unsyncedPod {
...
}
|
Overall this looks good (although I'm guessing it's still a WIP). The whole waiting seems a bit bad. I don't have a better solution though :) Do we know what kind of win we get from doing this versus making SyncPod() faster? From what we mentioned in #4119. |
|
@dchen1107 taking this over from you, let me know if you would still like to review :) |
|
Why do you think that this waiting is bad - I used a very similar pattern in my previous project and it worked pretty well. The missing this we may want to do here is to not try to accumulate a possibly infinite number of notifications, but instead set a limit for time (or number of those notifications that can be batched). Regarding making SyncPods() faster - we definitely want to do it to. I will start working on it today. If you think that we should wait with this PR for that one, I'm fine with that. |
|
I don't have a more technical reason than it being racy (although does improve performance in practice). I'm fine with the PR as is :) LGTM and will merge, but if you have time tomorrow can you run your tests again and see the gain this gets us? I'm curious to track how we improve there, |
Batch updates of multiple Pods.
Try to batch updates of multiple Pods in Kubelet, to avoid multiple (expensive) calls to Docker.
This is slightly (by ~10-20%) reducing the latency of starting the last Pod when we're starting 50 equal pods on the same node.