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

[Severin Gehwolf]

Hi Andrey,

On Mon, 2020-07-20 at 20:48 +0300, Andrey Petushkov wrote:
> 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

OK. So this might be another case of where the container auto-detection 
gets it wrong. The JDK thinks it's containerized, while it actually
isn't. For real cases where this might happen the suggested work-around 
is to use -XX:-UseContainerSupport. The patch to disable Java metrics
using this switch is beeing proposed here:
http://mail.openjdk.java.net/pipermail/jdk8u-dev/2020-August/012334.html

The intention is to integrate it together with the
OperatingSystemMXBean container awareness patch for 8u.

> > > 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

Any news on the jdk/jdk front? I'm guessing the container test don't
run there as they have '@requires docker-support'?

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
> 
>