RFR: 8321931: memory_swap_current_in_bytes reports 0 as "unlimited" [v7]

[Severin Gehwolf]

On Thu, 25 Jan 2024 16:42:50 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> We make a distinction between 0 for a limit vs non-limit cgroup values.
>> 
>> For all values (limit or non-limit), 0 simply means 0.
>> 
>> Output before from `jcmd PID VM.info`:
>> 
>> 
>> container (cgroup) information:
>> container_type: cgroupv2
>> cpu_cpuset_cpus: not supported
>> cpu_memory_nodes: not supported
>> active_processor_count: 8
>> cpu_quota: not supported
>> cpu_period: not supported
>> cpu_shares: not supported
>> memory_limit_in_bytes: unlimited
>> memory_and_swap_limit_in_bytes: unlimited
>> memory_soft_limit_in_bytes: unlimited
>> memory_usage_in_bytes: 11129120 k
>> memory_max_usage_in_bytes: not supported
>> memory_swap_current_in_bytes: unlimited
>> memory_swap_max_limit_in_bytes: unlimited
>> maximum number of tasks: 18963
>> current number of tasks: 33
>> 
>> 
>> Output now from `jcmd PID VM.info`:
>> 
>> 
>> container (cgroup) information:
>> container_type: cgroupv2
>> cpu_cpuset_cpus: not supported
>> cpu_memory_nodes: not supported
>> active_processor_count: 8
>> cpu_quota: not supported
>> cpu_period: not supported
>> cpu_shares: not supported
>> memory_limit_in_bytes: unlimited
>> memory_and_swap_limit_in_bytes: unlimited
>> memory_soft_limit_in_bytes: unlimited
>> memory_usage_in_bytes: 4962584 k
>> memory_max_usage_in_bytes: not supported
>> memory_swap_current_in_bytes: 0
>> memory_swap_max_limit_in_bytes: unlimited
>> maximum number of tasks: 18963
>> current number of tasks: 33
>> 
>> In this example `memory_swap_current_in_bytes` should be printed as equal to 0, not "unlimited".
>
> Gerard Ziemski has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - whitespace
>  - fix how we check

The test would be a bit easier to read if we extracted the loose matching to a function. Suggestion:


[...]
               out.shouldContain("memory_swap_max_limit_in_bytes");
               checkLineForValue(str, "memory_swap_max_limit_in_bytes", "0");
[...]

private static void checkLineForValue(String output, String match, String value) {
[...]
}

test/hotspot/jtreg/containers/docker/TestContainerInfo.java line 83:

> 81:                 if (s.contains("memory_swap_max_limit_in_bytes")) {
> 82:                     if (!s.contains("0")) {
> 83:                         throw new RuntimeException("memory_swap_max_limit_in_bytes NOT 0!");

Suggestion: `throw new RuntimeException("memory_swap_max_limit_in_bytes NOT 0! Line was: " + s);`

test/hotspot/jtreg/containers/docker/TestContainerInfo.java line 93:

> 91:                         throw new RuntimeException("memory_swap_current_in_bytes NOT 0!");
> 92:                     }
> 93:                     if (s.contains("Unlimited")) {

It's actually lower case `unlimited`.

-------------

PR Review: https://git.openjdk.org/jdk/pull/17314#pullrequestreview-1844228470
PR Review Comment: https://git.openjdk.org/jdk/pull/17314#discussion_r1466673181
PR Review Comment: https://git.openjdk.org/jdk/pull/17314#discussion_r1466674187