Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initialize swappiness in libcontainer cgroups template #14030

Merged
merged 1 commit into from
Jun 25, 2015

Conversation

estesp
Copy link
Contributor

@estesp estesp commented Jun 18, 2015

By default, the cgroup setting in libcontainer's configs.Cgroup for
memory swappiness will default to 0, which is a valid choice for memory
swappiness, but that means by default every container's memory
swappiness will be set to zero instead of the default 60, which is
probably not what users are expecting.

When the swappiness UI PR comes into Docker, there will be docker run
controls to set this per container, but for now we want to make sure
not to change the default, as well as work around an older kernel
issue that refuses to allow it to be set when cgroup hiearchies are in
use.

See docker/libcontainer#639 for more information. Also see closed PR #13882 which can probably be reconsidered now that the libcontainer update PR has been merged.

Docker-DCO-1.1-Signed-off-by: Phil Estes estesp@linux.vnet.ibm.com (github: estesp)

By default, the cgroup setting in libcontainer's configs.Cgroup for
memory swappiness will default to 0, which is a valid choice for memory
swappiness, but that means by default every container's memory
swappiness will be set to zero instead of the default 60, which is
probably not what users are expecting.

When the swappiness UI PR comes into Docker, there will be docker run
controls to set this per container, but for now we want to make sure
*not* to change the default, as well as work around an older kernel
issue that refuses to allow it to be set when cgroup hiearchies are in
use.

Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (github: estesp)
@LK4D4
Copy link
Contributor

LK4D4 commented Jun 18, 2015

Makes sense. It's always hard to do something good with default values :/

estesp added a commit to estesp/libcontainer that referenced this pull request Jun 19, 2015
As related to #639, this at least makes the "nsinit" consumer of
libcontainer initialize the value to "-1" and also allows the user to
specify a setting for the container.

This is an analog to Docker PR moby/moby#14030.

Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (github: estesp)
@cpuguy83
Copy link
Member

PR for swapiness UI is already up: #14004

@estesp
Copy link
Contributor Author

estesp commented Jun 19, 2015

@cpuguy83 ah, cool, didn't realize another one was opened now. This change is still required with our without a UI for setting it from the user.

estesp added a commit to estesp/libcontainer that referenced this pull request Jun 19, 2015
As related to #639, this at least makes the "nsinit" consumer of
libcontainer initialize the value to "-1" and also allows the user to
specify a setting for the container.

This is an analog to Docker PR moby/moby#14030.

Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (github: estesp)
@cpuguy83
Copy link
Member

janky is being janky.

LGTM

@LK4D4
Copy link
Contributor

LK4D4 commented Jun 25, 2015

LGTM

LK4D4 added a commit that referenced this pull request Jun 25, 2015
Initialize swappiness in libcontainer cgroups template
@LK4D4 LK4D4 merged commit c281549 into moby:master Jun 25, 2015
@estesp estesp deleted the init-memory-swappiness branch June 25, 2015 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants