RFR(XS): 8227317: [TESTBUG] jdk docker/TestDockerMemoryMetrics.java fails on systems w/o kernel memory accounting

[mikhailo.seledtsov at oracle.com]

Severin, Leonid,

   Thank you for review.

Misha

On 10/23/19 5:25 PM, mikhailo.seledtsov at oracle.com wrote:
> Here is the updated webrev: 
> http://cr.openjdk.java.net/~mseledtsov/8227317.02/
>
>
> Thanks,
>
> Misha
>
> On 10/23/19 3:26 PM, mikhailo.seledtsov at oracle.com wrote:
>> Hi Severin,
>>
>> On 10/23/19 2:08 AM, Severin Gehwolf wrote:
>>> Hi Misha,
>>>
>>> On Tue, 2019-10-22 at 11:50 -0700, mikhailo.seledtsov at oracle.com wrote:
>>>> Please review this change that adds extra check for test case
>>>> testKernelMemoryLimit(). If system does not support kernel memory
>>>> accounting then verification for this test case is skipped.
>>>>
>>>>       JBS: https://bugs.openjdk.java.net/browse/JDK-8227317
>>>>       Webrev: http://cr.openjdk.java.net/~mseledtsov/8227317.01/
>>>   123         OutputAnalyzer oa = DockerTestUtils.dockerRunJava(opts);
>>>   124         if (oa.getStderr().contains("kernel memory accounting 
>>> disabled")) {
>>>   125             System.out.println("Kernel memory accounting 
>>> disabled on this system, " +
>>>   126                                        "skipping the test case");
>>>   127             return;
>>>   128         }
>>>   129         oa.shouldHaveExitValue(0).shouldContain("TEST 
>>> PASSED!!!");
>>>   130     }
>>>
>>>  From what I understand this message comes from some container runtimes
>>> (runc in this case), right? It seems to be a case where the system
>>> potentially has kernel memory accounting, but the container runtime was
>>> compiled without support for it. Have you checked whether systems where
>>> the test fails also have 'cgroup.memory=nokmem' as kernel
>>> commandline[1]?
>>
>> I am not sure, could be either case. In any case, the tests should be 
>> able to handle this type of error.
>>
>> Perhaps, I should change the message to simply echo the original 
>> message from the docker command: "Kernel memory accounting disabled, 
>> skipping test case",
>>
>> that is w/o mentioning "on this system".
>>
>>
>>> In either case it seems diligent to add a comment prior the if.
>>>
>>> Suggestion:
>>>
>>>         OutputAnalyzer oa = DockerTestUtils.dockerRunJava(opts);
>>>         // Some container runtimes (e.g. runc, docker 18.09)
>>>         // have been built without kernel memory accounting. In
>>>         // that case, the runtime issues a message on stderr saying
>>>         // so. Skip the test in that case.
>>>         if (oa.getStderr().contains("kernel memory accounting 
>>> disabled")) {
>>
>> Thanks. I will add the comment that you recommend.
>>
>> Thank you,
>>
>> Misha
>>
>>>
>>> Thanks,
>>> Severin
>>>
>>> [1] cat /proc/cmdline | grep cgroup.memory=nokmem
>>>
>>>>       Testing:
>>>>          1. Ran the test on systems with disabled kernel memory
>>>> accounting before and after the fix - fix works
>>>>          2. Ran the test case 60 times on multiple Linux-x64 
>>>> systems - no
>>>> failures
>>>>
>>>>
>>>> Thank you,
>>>> Misha
>>>>