Jittering periods of some kubelet's sync loops:

[ingvagabund]

In order to synchronize the current state of Kubernetes's objects (e.g. pods, containers, etc.),
periodic synch loops are run. When there is a lot of objects to synchronize with,
loops increase communication traffic. At some point when all the traffic interfere cpu usage curve
hits the roof causing 100% cpu utilization.

To distribute the traffic in time, some sync loops can jitter their period in each loop
and help to flatten the curve.

kubelet sync loops that are jittered:

• kubelet.syncLoop responsible for housekeeping
• prober manager: checking containers for readiness

[k8s-github-robot]

Labelling this PR as size/XL

[k8s-bot]

GCE e2e build/test failed for commit a52d0a2d3b2c5e31760139673584dc67e8ee4497.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[k8s-bot]

GCE e2e build/test failed for commit a8e40992f0f4decadbaf5af27d5e7b21d68ffef7.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[yujuhong]

IIUC, this PR is based on #19917, and only the last commit (a8e40992f0f4decadbaf5af27d5e7b21d68ffef7) is new.

The jitters introduced into kubelet in that commit don't really matter. What should be modified is the the individual workers' (pod workers, probers) sync/probing period.
E.g.,

kubernetes/pkg/kubelet/pod_workers.go

Line 212 in ff04de4

p.workQueue.Enqueue(uid, p.resyncInterval)

,

kubernetes/pkg/kubelet/pod_workers.go

Line 214 in ff04de4

// Error occurred during the sync; back off and then retry.

kubernetes/pkg/kubelet/prober/worker.go

Line 96 in ff04de4

probeTicker := time.NewTicker(time.Duration(w.spec.PeriodSeconds) * time.Second)

In general, I think introducing jitters is good. I did some tests after #19850 was merged and jittering period did not have significant difference in the cpu resource usage data. Perhaps with more pods (100 per node), this would make a difference. I would like to get some data before the PR is merged though.

[ingvagabund]

That is correct.

Thanks for the tips, I will take a look.

[timothysc]

Could we separate out the PR's? It seems this should be 2 different PRs.

1. Conversion to wait.*
2. Update some kubernetes-defaults

In general, I think introducing jitters is good. I did some tests after #19850 was merged and jittering period did not have significant difference in the cpu resource usage data.

Our profiles simply show a flattening of the spikes, but the ~90th-percentile usually remains steady. You would need something like pbench to see the spikes b/c most profiling would average it away.

[yujuhong]

Could we separate out the PR's? It seems this should be 2 different PRs.

1. Conversion to wait.*
2. Update some kubernetes-defaults

@timothysc, only the last commit is new. The rest is just #19917.

Our profiles simply show a flattening of the spikes, but the ~90th-percentile usually remains steady. You would need something like pbench to see the spikes b/c most profiling would average it away.

Could you share some numbers? That'd be very useful.

[pmorie]

@yujuhong @timothysc this is meant to be a distinct PR which depends on #19917

[k8s-github-robot]

PR needs rebase

[timothysc]

Looks like the other commit landed so we can probably rebase this one.

[k8s-bot]

GCE e2e build/test failed for commit c0cf0d1578408387045fdafa80241c46cf2460ed.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[k8s-github-robot]

Labelling this PR as size/M

[ingvagabund]

probeTicker := time.NewTicker(time.Duration(w.spec.PeriodSeconds) * time.Second)
...
probeLoop:
    for w.doProbe() {
        // Wait for next probe tick.
        select {
        case <-w.stop:
            break probeLoop
        case <-probeTicker.C:
            // continue
        }
    }

got replaced with

wait.JitterUntil(func() { w.doProbe() }, time.Duration(w.spec.PeriodSeconds)*time.Second, workerProbeJitterFactor, w.stop)

Why? reasoning:

The for loop can be replaced equivalently with:

wait.Until(func() { w.doProbe() }, time.Duration(w.spec.PeriodSeconds)*time.Second, w.stop)

Once w.stop is closed, the for loop ends (break probeLoop). The same happens for Until as whenever w.stop is closed, the Until ends. The for loop periodically loops (calls w.doProbe) as probeTicker.C ticks. Until calls w.doProbe periodically with the period that is the same as for the ticker. So at the end both for and Until provides the same functionality.

The second step is replacing Until with JitterUntil and introducing jitter factor.

[ingvagabund]

If the doProbe returns false, the Until will not stop :(. Grrr

[k8s-bot]

GCE e2e test build/test passed for commit 0f98e735fce2d3bc7335ca6ede6fba9d1a499ce6.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[timothysc]

So I think @yujuhong is the expert on which timers to mod here.

[yujuhong]

Can we use a smaller factor such as 0.5? I think that should be enough to distribute the sync times.

[ingvagabund]

Both factors decreased to 0.5

[k8s-github-robot]

The author of this PR is not in the whitelist for merge, can one of the admins add the 'ok-to-merge' label?

[timothysc]

@ingvagabund thanks for your help in seeing this one through.

[ingvagabund]

Np. Thank you all for spending your time on reviewing the PR.

[pmorie]

Tagging for @yujuhong

[k8s-github-robot]

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

[bgrant0607]

We shouldn't LGTM PRs with WIP in the title.

[pmorie]

@bgrant0607 my bad, I had missed that.

[bgrant0607]

Ah, I see @pmorie changed the title. Nevermind. The change hasn't been reflected in the submit queue yet.

[k8s-bot]

GCE e2e build/test failed for commit 392fc66.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[wojtek-t]

cluster didn't started correctly

@k8s-bot e2e test this please github issue: #IGNORE

[k8s-bot]

GCE e2e test build/test passed for commit 392fc66.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[k8s-github-robot]

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

[k8s-bot]

GCE e2e test build/test passed for commit 392fc66.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[k8s-github-robot]

Automatic merge from submit-queue

[ixdy]

@wojtek-t why didn't you file an issue for the cluster failing to start?

[jeremyeder]

@pweil- will our rebase catch this?

[pweil-]

@jeremyeder this should be included

[timothysc]

@pweil - We determined this is a "nice to have" but not a blocker, as you will drag in transitive deps on this one. The larger issue of cadvisor jitter should be in the upcoming rebase with the lasted cadvisor updates.

So you can make the call here.

/cc @ncdc