[8u] RFR: 8226575: OperatingSystemMXBean should be made container aware

Andrey Petushkov andrey at azul.com
Mon Jul 20 17:48:57 UTC 2020


Hi Severin,


On 20.07.2020 14:47, Severin Gehwolf wrote:
> Hi Andrey,
>
> On Fri, 2020-07-17 at 21:44 +0300, Andrey Petushkov wrote:
>> Hi Severin,
>>
>> The code being backported seem to have a bug and will result in a
>> regression if
>> being integrated.
> OK. Do you have any experience on a ballpark number as to how many
> systems this would affect? Is this a common config?
We have seen relevant tests fail only on one type of system. So it might be
rare enough. Let me check whether other systems have all or nothing
cgroups fs
or there are more like that just happened not to have respective test to
fail
>
>> The problem is that, to my understanding it's legal that only some of known
>> controllers are mounted (at least man says they can be mounted
>> individually).
>> This creates a situation that Metrics gets "active" and populated with
>> some of
>> SubSystems, but not all. As a result OperatingSystemImpl considers
>> containerMetrics
>> available and uses it as a source. The SubSystem code is written the way
>> that it
>> returns 0 for any value if a respective subsystem is missing. At the
>> same time
>> OperatingSystemImpl typically uses >=0 as a sanity check of a value
>> returned from Metrics.
>> Effectively preventing from falling back to native implementation for
>> the actual value
>> and returning 0 instead of actual value
>>
>> The problem actually exhibited by GetTotalPhysicalMemorySize test ran on
>> Raspbian Stretch
>> which happen to have cgroup fs but not /sys/fs/cgroup/memory
> Thanks for the heads-up. A couple of questions:
>
>  * Note that this has changed in JDK 15 where unavailable files should
>    return -1 (over 0) in case of missing crgoup files. Have you tested
>    JDK 15 by any chance?
No, not yet. Let me try
>  * Do you see this behaviour in a container or also outside?
The behavior was observed _only_ outside of container. However I believe
we don't run these tests inside a container on RPi. That would be
interesting
to try, let me see
>
>> That's not the problem of a backport and of course should be discussed
>> with upstream
>> developers. It just happened that we've found it with a backport so
>> letting you know
> Yes, agreed. It seems an issue with the cgroups Metrics infra in older
> JDKs. Do you mind creating a bug for this issue?
Sure. Let me see the state of JDK15 wrt this issue before filing a bug

Thanks,
Andrey
>
> Thanks,
> Severin
>
>> Regards,
>> Andrey
>>
>> On 17.07.2020 17:07, Severin Gehwolf wrote:
>>> Hi,
>>>
>>> Please review this OpenJDK 8u backport for OperatingSystemMXBean's
>>> container awareness which has been backported to Oracle JDK (parity
>>> bug). This backport depends on JDK-8203357[1] for Metrics.java which is
>>> being used in this patch.
>>>
>>> Changes as compared to the JDK 11 patch were:
>>>
>>>  * SubSystem.java: 8217338: [Containers] Improve systemd slice memory
>>>    limit support not being there => getLongValueMatchingLine() missing
>>>    => dropped
>>>  * OperatingSystemImpl.java: Introduced Java methods which internally
>>>    call the native methods. Renamed native methods to <oldmethod-name>0
>>>  * Tests were actually done to the hotspot code in later JDKs, thus,
>>>    for this backport the tests are in the hotspot portion of the webrev
>>>    (separate repo).
>>>  * Other than that, it's just the native bits which are in different
>>>    files in JDK 8.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8226575
>>> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8226575/jdk8/01/
>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8248804
>>>
>>> Testing: Included docker tests on Linux x86_64. jdk_management tests and
>>> tier 1 tests. No regressions noted.
>>>
>>> If somebody could test this on Windows/Mac, I'd appreciate it.
>>>
>>> Thanks,
>>> Severin
>>>
>>> [1] http://mail.openjdk.java.net/pipermail/jdk8u-dev/2020-July/012164.html
>>




More information about the jdk8u-dev mailing list