Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

cgroups: add support for oom control - #417

Merged
vmarmol merged 2 commits into
docker-archive:masterfrom
HuKeping:master
Mar 6, 2015
Merged

cgroups: add support for oom control#417
vmarmol merged 2 commits into
docker-archive:masterfrom
HuKeping:master

Conversation

@HuKeping

Copy link
Copy Markdown
Contributor

Redo #412
This patch add support for diable OOM Killer.

Signed-off-by: Hu Keping hukeping@huawei.com

@LK4D4

Copy link
Copy Markdown
Contributor

@HuKeping I'm not sure but should be pretty easy to write test for this. At least in docker we have test for oom :)

@HuKeping

Copy link
Copy Markdown
ContributorAuthor

ok, test will come soon

@HuKeping

Copy link
Copy Markdown
ContributorAuthor

hi @LK4D4 I am not quite sure what you would like this test to do. I think the oom control test would be better in docker/docker?

@HuKeping

Copy link
Copy Markdown
ContributorAuthor

do we have plan how often will docker/docker merge libcontainer? it seems a lot of docker's PR depend on libcontainer

@LK4D4

Copy link
Copy Markdown
Contributor

@HuKeping I'm working on move docker to new libcontainer API. There are some problems :) After that we can update libcontainer as often as we need it in docker.

@HuKeping

Copy link
Copy Markdown
ContributorAuthor

@LK4D4 OK and by the way what do you want for this PR's test? I didn't see much to do.

@LK4D4

Copy link
Copy Markdown
Contributor

@HuKeping Actually I prefer to have good coverage for libcontainer too. There was pretty bad problems because of tests lack. But if you don't want write a test, then we can leave it for docker birthday party :)

@HuKeping

Copy link
Copy Markdown
ContributorAuthor

@LK4D4 yes, of cause I'd like to post a test for this , just don't know what should it be like. Is it to check the writeable of file memory.oom_control?

@HuKeping
HuKepingforce-pushed the master branch 2 times, most recently from 2e8cab9 to 4332ffcCompareMarch 6, 2015 10:49
@HuKeping

Copy link
Copy Markdown
ContributorAuthor

test added, ping @LK4D4

@LK4D4

LK4D4 commented Mar 6, 2015

Copy link
Copy Markdown
Contributor

LGTM

1 similar comment
@vmarmol

Copy link
Copy Markdown
Contributor

LGTM

vmarmol added a commit that referenced this pull request Mar 6, 2015
cgroups: add support for oom control
@vmarmol
vmarmol merged commit dd3cb88 into docker-archive:masterMar 6, 2015
This patch add support for diable OOM Killer.
Signed-off-by: Hu Keping <hukeping@huawei.com>
Signed-off-by: Hu Keping <hukeping@huawei.com>
@HuKeping

Copy link
Copy Markdown
ContributorAuthor

hi @LK4D4 , would you please help me to review the related PR on docker/docker
moby/moby#11034

wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jul 12, 2017
It's backed by memory.oom_control, so this commit moves it in with
the rest of the memory-controller config.
Looking at the history, the initial request landing a setting for this
in the Docker/OCI ecosystem seems to be [1], which added
Cgroup.OomKillDisable. That commit was carried from libcontainer into
runC [2] where it is now Resources.OomKillDisable [3]. From runC it
was carried into this repo (with some renaming) in [4]. Subsequent
early doc updates landed in [5,6]. In none of those can I find
discussion about why the setting is not already under memory. I
expect the reason is that the runC structures are flat, so "under
memory" is not a thing there. But in this spec, resources has
per-controller sub-properties. The fact that disableOOMKiller
belonged to the memory controller may have been overlooked in [4] and
never revisited until now.
[1]: docker-archive/libcontainer#417
Subject: cgroups: add support for oom control
[2]: opencontainers/runc@295c708
Subject: cgroups: add support for oom control
[3]: https://github.com/opencontainers/runc/blob/v1.0.0-rc3/libcontainer/configs/cgroup_unix.go#L113-L114
[4]: opencontainers#51
Subject: Add Go types for specification
[5]: opencontainers#137
Subject: Adding cgroups path to the Spec.
[6]: opencontainers#199
Subject: runtime: config: linux: add cgroups informations
Signed-off-by: W. Trevor King <wking@tremily.us>
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@HuKeping@LK4D4@vmarmol