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

Treat -1 as default value for memory swappiness #85

Closed
ktraghavendra opened this issue Jul 3, 2015 · 15 comments
Closed

Treat -1 as default value for memory swappiness #85

ktraghavendra opened this issue Jul 3, 2015 · 15 comments

Comments

@ktraghavendra
Copy link
Contributor

In some older kernels setting memory swappiness fails. This happens even when
nobody tries to configure swappiness from docker UI because we would still get
some default value from host config.
With this we treat -1 value as default value and skip the enforcement
of swappiness.

@ktraghavendra
Copy link
Contributor Author

More discussion was here: docker-archive/libcontainer#654

@rajasec
Copy link
Contributor

rajasec commented Jul 6, 2015

@ktraghavendra
With 14.04 LTS ( Kernel 3.13), I'm able to spawn containers properly using latest runc
but if I run the unit tests using make localtest, , tests are failing in integration directory
System Error: write /sys/fs/cgroup/memory/integration/test/memory.swappiness: invalid argument

@ktraghavendra
Copy link
Contributor Author

@rajasec is it because of use_hierarchy is set to 1 ? .. or is it a different problem.?

@rajasec
Copy link
Contributor

rajasec commented Jul 6, 2015

Memory heirarchy is set to 1 in my system
./memory/integration/test/memory.use_hierarchy 1
This is the exact problem which I've seen in make localtest
time="2015-07-07T00:34:05+05:30" level=warning msg="signal: killed"
--- FAIL: TestCheckpoint (0.05s)
checkpoint_test.go:61: [8] System error: write /sys/fs/cgroup/memory/integration/test/memory.swappiness: invalid argument
=== RUN TestExecPS
time="2015-07-07T00:34:05+05:30" level=warning msg="signal: killed"
--- FAIL: TestExecPS (0.12s)
exec_test.go:45: |: [8] System error: write /sys/fs/cgroup/memory/integration/test/memory.swappiness: invalid argument
=== RUN TestUsernsExecPS
time="2015-07-07T00:34:05+05:30" level=warning msg="signal: killed"
--- FAIL: TestUsernsExecPS (0.10s)
exec_test.go:45: |: [8] System error: write /sys/fs/cgroup/memory/integration/test/memory.swappiness: invalid argument
=== RUN TestIPCPrivate

@ktraghavendra
Copy link
Contributor Author

@rajasec This is expected because we are trying to set memory swappiness in the test but we are not able to. I am not sure whether it is correct to PASS the test giving wrong impression that system is able to set swappiness value.
any ideas welcome..

@ktraghavendra
Copy link
Contributor Author

@rajasec one idea for example is, in the test when we get get error, we would still return success if use_hierarchy=1. (but if the test genuinely fails in a newer kernel where writing to memory swappiness is
allowed, it would be wrong)

@rajasec
Copy link
Contributor

rajasec commented Jul 6, 2015

@ktraghavendra
In the following file
libcontainer/integration/template_test.go
I made MemorySwappiness to -1 as per spec_linux.go. Post that all tests passed..

groups: &configs.Cgroup{
Name: "test",
Parent: "integration",
MemorySwappiness: -1,
AllowAllDevices: false,
AllowedDevices: configs.DefaultAllowedDevices,
},
Let me know can I place pull request for this template change ? ( if it is necessary)

@ktraghavendra
Copy link
Contributor Author

@rajasec , Thanks for finding that fix. I feel if test case is genuinely failing on the system (because we try to set it to some value but it does not succeed, probably it is better to know that it is failing and fix it by upgrading the kernel to newer version for e.g., here 3.16+).

But having said that I do not have a strong preference over whether it should be included. May be you could ask for the pull request, and let maintainers throw light upon what works best. (because unnecessary unit test failures are irritating some times).

@ktraghavendra
Copy link
Contributor Author

@rajasec if that fixes the testcase on the working system, please ask for pull request.

@rajasec
Copy link
Contributor

rajasec commented Jul 7, 2015

@ktraghavendra
I'll test with few latest kernels ( 3.18 or 4.x) before asking for the pull request.
@ktraghavendra
One more thing which I observed today with the latest runc which I've pulled hour ago..
Few days back, I've seen Memory.Swappiness was set to -1 in cgroups configs, but looks like the change is been reverted or removed
Due to which I could not launch my container.

c := &configs.Cgroup{
Name: getDefaultID(),
Parent: myCgroupPath,
AllowedDevices: append(devices, allowedDevices...),
}

@rajasec
Copy link
Contributor

rajasec commented Jul 7, 2015

@ktraghavendra
I've compiled & booted the latest kernel 3.18.7 in my Ubuntu system ( 14.04 which had 3.13 kernel)
After the update of the kernel, memory.swappiness is allowed for the cgroups and all the tests are passing. I'm able to modify the swappiness in /sys/fs/cgroup/..../runc/memory.swappiness

I'm also able to start my container properly which was failing in 3.13 with the latest runc

@ktraghavendra
Copy link
Contributor Author

@rajasec Thank you for confirming.

@rajasec
Copy link
Contributor

rajasec commented Jul 7, 2015

For old kernels, we still need memory swappiness to -1.

@rajasec
Copy link
Contributor

rajasec commented Jul 7, 2015

I have generated an pull request

@mrunalp
Copy link
Contributor

mrunalp commented Aug 18, 2015

This is resolved now.

@mrunalp mrunalp closed this as completed Aug 18, 2015
stefanberger pushed a commit to stefanberger/runc that referenced this issue Sep 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants