RFR: 8284950: CgroupV1 detection code should consider memory.swappiness [v12]
Severin Gehwolf
sgehwolf at openjdk.java.net
Tue May 3 11:57:32 UTC 2022
On Tue, 3 May 2022 09:21:11 GMT, xpbob <duke at openjdk.java.net> wrote:
>> set memory.swappiness to 0,swap space will not be used
>> determine the value of memory.swappiness
>> https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt
>>
>>
>> Memory Limit: 50.00M
>> Memory Soft Limit: Unlimited
>> Memory & Swap Limit: 100.00M
>> Maximum Processes Limit: 4194305
>>
>> =>
>>
>> Memory Limit: 50.00M
>> Memory Soft Limit: Unlimited
>> Memory & Swap Limit: 50.00M
>> Maximum Processes Limit: 4194305
>
> xpbob has updated the pull request incrementally with one additional commit since the last revision:
>
> remove no use copy
To be honest, the original test change was closer to what I was expecting to see. Could you clarify why we'd need to use `cgcreate` et.al. for this test? As far as I'm aware `--memory-swappiness` should work just as well.
src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 150:
> 148: GET_CONTAINER_INFO(julong, _memory->controller(), "/memory.swappiness",
> 149: "Swappiness is: " JULONG_FORMAT, JULONG_FORMAT, swappiness);
> 150: if(swappiness == 0) {
Style: Space before `(`
src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 152:
> 150: if(swappiness == 0) {
> 151: jlong memlimit = read_memory_limit_in_bytes();
> 152: log_trace(os, container)("Memory and Swap Limit has been reset to " JULONG_FORMAT " because of Swappiness is 0", memlimit);
`s/because of Swappiness is 0/because swappiness is 0/g`
test/hotspot/jtreg/containers/docker/TestCgroupV1Memory.java line 58:
> 56: try {
> 57: testMemoryLimit();
> 58: testMemoryLimitWithSwappiness();
Only `testMemoryLimitWithSwappiness()` should be `cgroupv1` conditional.
test/lib/jdk/test/lib/containers/cgroup/CgroupV1TestUtils.java line 34:
> 32:
> 33:
> 34: public class CgroupV1TestUtils {
I'm not sure what the point of this class is. Wouldn't we be able to test the same thing with the existing docker infrastructure using `--memory-swappiness=0` as docker option? As far as I can see this utility adds a dependency in `libcgroup` and related tools, which isn't necessarily installed on test systems. Docker dep, on the other hand, is enforced via `@require` tag.
-------------
Changes requested by sgehwolf (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8285
More information about the core-libs-dev
mailing list