kernel memcg notification enabled via experimental flag by derekwaynecarr · Pull Request #38258 · kubernetes/kubernetes

derekwaynecarr · 2016-12-07T03:32:28Z

Kubelet integrates with kernel memcg notification API if and only if enabled via experimental flag.

k8s-reviewable · 2016-12-07T03:32:35Z

This change is

derekwaynecarr · 2016-12-07T03:33:30Z

/cc @saad-ali @thockin @vishh @dchen1107 @mtaufen @eparis -- this is WIP while i generate a bunch of files, but this would be the PR that would gate integration with this kernel feature. hope to finish this up this evening.

k8s-ci-robot · 2016-12-07T03:41:39Z

Jenkins GCI GKE smoke e2e failed for commit 451fe67. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-12-07T03:41:39Z

Jenkins GKE smoke e2e failed for commit 451fe67. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-12-07T03:41:40Z

Jenkins Kubemark GCE e2e failed for commit 451fe67. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-12-07T03:41:40Z

Jenkins GCI GCE e2e failed for commit 451fe67. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-12-07T03:42:09Z

Jenkins GCE e2e failed for commit 451fe67. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-12-07T03:42:10Z

Jenkins GCE etcd3 e2e failed for commit 451fe67. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-12-07T03:42:10Z

Jenkins unit/integration failed for commit 451fe67. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-12-07T03:42:39Z

Jenkins GCE Node e2e failed for commit 451fe67. Full PR test history.

The magic incantation to run this job again is @k8s-bot node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-12-07T03:43:10Z

Jenkins CRI GCE Node e2e failed for commit 451fe67. Full PR test history.

The magic incantation to run this job again is @k8s-bot cri node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

vishh · 2016-12-07T03:51:18Z

lgtm

mtaufen · 2016-12-07T04:19:36Z

 	if obj.EvictionPressureTransitionPeriod == zeroDuration {
 		obj.EvictionPressureTransitionPeriod = metav1.Duration{Duration: 5 * time.Minute}
 	}
+	if obj.ExperimentalKernelMemcgNotification == nil {


ExperimentalKernelMemcgNotification is a bool, not a *bool.

yeah, i just fixed a push with that. i was still messing w/ generators and just threw this up to make people aware i was working on it.

mtaufen · 2016-12-07T04:22:34Z

 	fs.DurationVar(&s.EvictionPressureTransitionPeriod.Duration, "eviction-pressure-transition-period", s.EvictionPressureTransitionPeriod.Duration, "Duration for which the kubelet has to wait before transitioning out of an eviction pressure condition.")
 	fs.Int32Var(&s.EvictionMaxPodGracePeriod, "eviction-max-pod-grace-period", s.EvictionMaxPodGracePeriod, "Maximum allowed grace period (in seconds) to use when terminating pods in response to a soft eviction threshold being met.  If negative, defer to pod specified value.")
 	fs.StringVar(&s.EvictionMinimumReclaim, "eviction-minimum-reclaim", s.EvictionMinimumReclaim, "A set of minimum reclaims (e.g. imagefs.available=2Gi) that describes the minimum amount of resource the kubelet will reclaim when performing a pod eviction if that resource is under pressure.")
+	fs.BoolVar(&s.ExperimentalKernelMemcgNotification, "experimental-kernel-memcg-notification", s.ExperimentalKernelMemcgNotification, "If enabled, the kubelet will integrate with the kernel memcg notification to determine if memory eviction thresholds are crossed rather than polling.")


I wonder if this would be better guarded by a feature gate than a flag; with a feature gate we don't have a new flag that needs to be removed later. Just a thought.

i think its fair to assume that not all kernels are equal and there could be some distros that require it to be off so a flag not tied to feature gate seems right in the long run?

Yeah true. This is the kind of switch that might need to be in for the long haul, due to the kernel issue. A flag is probably the appropriate solution.

Should we put a note in this flag that it may cause soft lockups due to an unidentified kernel issue? I've heard some worries that we don't want people trying to "make Kubernetes faster" by turning this on without realizing it could cause other problems.

we haven't proven that it causes soft lockups have we?

We did, in some sense. Soft lockups are kernel problem, so proper fix is beyond our reach, but we did prove that this triggers soft lockup on debian in CVM. @dchen1107

derekwaynecarr · 2016-12-07T04:27:15Z

i am having some trouble updating generated-protobuf and will finish this up in the morning.

k8s-ci-robot · 2016-12-07T04:29:49Z

Jenkins verification failed for commit cd099531c7dd8ca0d3ca6da1747b2ad161f2385b. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

dims · 2016-12-07T14:34:36Z

@derekwaynecarr looks like running hack/update-openapi-spec.sh updates a few more files.

derekwaynecarr · 2016-12-07T14:36:58Z

@dims -- yeah, i had docker issues last night that prevented me from running the full hack/update suite. i just fixed that up locally, and the generation stuff is all running now, should have a second commit for this pr soon with generated stuff.

derekwaynecarr · 2016-12-07T16:02:30Z

All things green.

mtaufen · 2016-12-07T17:20:42Z

There is still some question as to whether this is in fact the root cause. See #37853 for more details; @thockin was able to reproduce the issue by thrashing a service and pods even after reverting 1ec69f6 and 2583116.

mtaufen · 2016-12-07T17:21:19Z

We don't want to start merging things until we are more confident in the root cause.

derekwaynecarr · 2016-12-07T17:50:46Z

@mtaufen - i am still biased to take the gate since we know it caused problems with older ubuntu trusty images.

dchen1107 · 2016-12-07T22:08:36Z

LGTM

cc/ @fabioy we should make sure --experimental-kernel-memcg-notification is false in GKE production.

k8s-github-robot · 2016-12-07T22:22:31Z

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

k8s-github-robot · 2016-12-07T23:00:10Z

Automatic merge from submit-queue (batch tested with PRs 38318, 38258)

saad-ali · 2016-12-07T23:38:41Z

@derekwaynecarr This merged, can you please prepare a cherry pick to the 1.5 branch.

derekwaynecarr · 2016-12-07T23:56:04Z

@saad-ali see #38296

saad-ali · 2016-12-08T00:02:32Z

Ah perfect, will get that merged.

k8s-cherrypick-bot · 2016-12-08T01:03:46Z

Commit found in the "release-1.5" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 7, 2016

derekwaynecarr mentioned this pull request Dec 7, 2016

kubelet: eviction: add memcg threshold notifier to improve eviction responsiveness #32577

Merged

derekwaynecarr force-pushed the kernel-memcg-flag branch 2 times, most recently from efea36c to 451fe67 Compare December 7, 2016 03:37

derekwaynecarr added cherrypick-candidate release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Dec 7, 2016

derekwaynecarr added this to the v1.5 milestone Dec 7, 2016

k8s-github-robot assigned dchen1107 Dec 7, 2016

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 7, 2016

vishh self-assigned this Dec 7, 2016

mtaufen reviewed Dec 7, 2016

View reviewed changes

derekwaynecarr force-pushed the kernel-memcg-flag branch from 451fe67 to cd09953 Compare December 7, 2016 04:24

This was referenced Dec 7, 2016

Fix Node soft-lockup #38287

Closed

Fix Node soft-lockup #38285

Closed

derekwaynecarr added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Dec 7, 2016

derekwaynecarr added 2 commits December 7, 2016 10:09

Enable kernel memcg notification via additional flag

5b2d1c2

Add generated artifacts for new flag

bcca039

derekwaynecarr force-pushed the kernel-memcg-flag branch from cd09953 to bcca039 Compare December 7, 2016 15:10

k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 7, 2016

derekwaynecarr changed the title ~~WIP: kernel memcg notification enabled via experimental flag~~ kernel memcg notification enabled via experimental flag Dec 7, 2016

derekwaynecarr mentioned this pull request Dec 7, 2016

kernel memcg notification enabled via experimental flag #38296

Merged

eparis added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 7, 2016

mtaufen added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Dec 7, 2016

gmarek mentioned this pull request Dec 7, 2016

root cause kernel soft lockups #37853

Closed

dchen1107 removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Dec 7, 2016

k8s-github-robot merged commit ca04936 into kubernetes:master Dec 7, 2016

saad-ali added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Dec 8, 2016

k8s-cherrypick-bot removed the cherrypick-candidate label Dec 8, 2016

Conversation

derekwaynecarr commented Dec 7, 2016

Uh oh!

k8s-reviewable commented Dec 7, 2016

Uh oh!

derekwaynecarr commented Dec 7, 2016

Uh oh!

k8s-ci-robot commented Dec 7, 2016

Uh oh!

k8s-ci-robot commented Dec 7, 2016

Uh oh!

k8s-ci-robot commented Dec 7, 2016

Uh oh!

k8s-ci-robot commented Dec 7, 2016

Uh oh!

k8s-ci-robot commented Dec 7, 2016

Uh oh!

k8s-ci-robot commented Dec 7, 2016

Uh oh!

k8s-ci-robot commented Dec 7, 2016

Uh oh!

k8s-ci-robot commented Dec 7, 2016

Uh oh!

k8s-ci-robot commented Dec 7, 2016

Uh oh!

vishh commented Dec 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

derekwaynecarr commented Dec 7, 2016

Uh oh!

k8s-ci-robot commented Dec 7, 2016

Uh oh!

dims commented Dec 7, 2016

Uh oh!

derekwaynecarr commented Dec 7, 2016

Uh oh!

derekwaynecarr commented Dec 7, 2016

Uh oh!

mtaufen commented Dec 7, 2016

Uh oh!

mtaufen commented Dec 7, 2016

Uh oh!

derekwaynecarr commented Dec 7, 2016

Uh oh!

dchen1107 commented Dec 7, 2016

Uh oh!

k8s-github-robot commented Dec 7, 2016

Uh oh!

k8s-github-robot commented Dec 7, 2016

Uh oh!

saad-ali commented Dec 7, 2016

Uh oh!

derekwaynecarr commented Dec 7, 2016

Uh oh!

saad-ali commented Dec 8, 2016

Uh oh!

k8s-cherrypick-bot commented Dec 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!