On Wed, 12 Aug 2026 14:18:23 GMT, Ivan Bereziuk <[email protected]> wrote:

>> Please review this PR that makes "failcount" case of 
>> `TestDockerMemoryMetrics.java` more robust.
>> The "failcount" case is failing in some environments (OL9+ in my case).
>> 
>> With this change I am trying to address several issues:
>> * the test was not setting swap-space, and assumed OS will map mirror 
>> `--memory` in size. 128MiB for the test was not always enough for a 
>> successful run.
>>    * updated the test with additional `--memory-swap` flat to control the 
>> swap headroom in container so that failcount gets more chances to be 
>> incremented while main memory is full. It may also be that some OSes are 
>> more eager the others to update counters in cgroup-fs. This tweak helps.
>> *  the `new byte[1 MiB]` allocations alone reserves heap space for the 
>> application space but may leave the memory uncommitted. It can be that 512 
>> MiB Java heap fills up and throws OutOfMemoryError before RSS crosses the 
>> 128MiB limit.
>>   * fix by dirtying the chunks after we allocate them.
>> * removed `catch`'ing `java.lang.OutOfMemoryError` errors to avoid secondary 
>> `OutOfMemoryError`.
>> * The test inherits JVM flags set externally and so `-Xmx` is often passed 
>> to the JVM running in a container.
>>   *  Make `-Xmx` flag set from within the test to *win* over the one set 
>> externally (e.g. by our make scripts for task definition).
>> 
>> * the additional logs help seeing the dynamics of the test, Example OL9:
>> 
>> Initial memory fail count: 0
>> Allocated:   0M, Memory usage:  42M, Memory and swap:  42M
>> .......Allocated:   8M, Memory usage:  72M, Memory and swap:  72M
>> .......Allocated:  16M, Memory usage:  88M, Memory and swap:  88M
>> .......Allocated:  24M, Memory usage: 106M, Memory and swap: 106M
>> .......Allocated:  32M, Memory usage: 123M, Memory and swap: 123M
>> .......Allocated:  40M, Memory usage: 127M, Memory and swap: 142M
>> .......Allocated:  48M, Memory usage: 127M, Memory and swap: 163M
>> .......Allocated:  56M, Memory usage:  97M, Memory and swap: 185M
>> .......Allocated:  64M, Memory usage: 113M, Memory and swap: 200M
>> .......Allocated:  72M, Memory usage: 127M, Memory and swap: 215M
>> .......Allocated:  80M, Memory usage: 127M, Memory and swap: 251M
>> .......Allocated:  88M, Memory usage: 127M, Memory and swap: 249M
>> .......Allocated:  96M, Memory usage: 127M, Memory and swap: 266M
>> .......Allocated: 104M, Memory usage: 125M, Memory and swap: 287M
>> .
>> DEBUG: Bytes allocation length 1: 1048576
>> Final memory fail count: 1
>> TEST PASSED!!!
>> ``` 
>> 
>> Tested `TestDocker...
>
> Ivan Bereziuk has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   space alias fix. comment tweak

Looks good! Just one small comment nit.

test/jdk/jdk/internal/platform/docker/MetricsMemoryTester.java line 75:

> 73: 
> 74:         // We need swap to execute this test. Otherwise OOM killer acts 
> with
> 75:         // SIGSEGV before we read the fail counter.

The OOM killer sends out a `SIGKILL`, not a `SIGSEGV`

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

Marked as reviewed by cnorrbin (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/32300#pullrequestreview-4960827668
PR Review Comment: https://git.openjdk.org/jdk/pull/32300#discussion_r3803897662

Reply via email to