[PING] RFR [XS]: 8244500: jtreg test error in test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java

[Severin Gehwolf]

On Wed, 2020-06-17 at 09:09 -0400, Bob Vandette wrote:
> Should we just accept the kernel error message rather than add all the additional value checking?
> 
>      "out.shouldContain("Your kernel does not support swap limit capabilities”);

I'd prefer to keep the other checks as well. The bug explains why that
is and if we see a config which fails this as well, something is wrong.
Hopefully that makes some sense.

> Why isn’t the try block around the check for .shouldMatch("OperatingSystemMXBean\\.getFreeSwapSpaceSize: [1-9][0-9]+”);

Why should it? For most set ups this catch block is never reached. The
only setup we are aware of which reaches this is the system Matthias
runs tests on. I've never heard of a system where this fails as well.

> Why doesn’t this call fail as well??

Because the system values are being used for
OperatingSystemMXBean.getTotalSwapSpaceSize() when:
Metrics.getMemoryLimit() > 0 && Metrics.getMemoryAndSwapLimit() == -1

> Have you tried this test on a system with cgrouopv1 and v2 that doesn’t have swap enabled?

I don't think so. I don't have access to such a system.

How do you mean, though? A system which has swap disabled on the system
level or a container without swap? I believe the former is exactly the
case of Matthias' system. But I don't know. The latter should also be
covered via a test Jie added a while back.

> I’m concerned that we’ll be back to fixing this when some other configuration combination fails.

I understand but find it unlikely. Those are systems which are going to
be hard to detect correctly and do-the-right-thing. As mentioned in the
bug there is no way to distinguish between unlimited swap + a memory
limit from kernel-doesn't-support-swap-limits + a memory limit.

Note that https://bugs.openjdk.java.net/browse/JDK-8236617 "fixed" this
for the old implementation. Unfortunately, it doesn't seem like it was
investigated why a 0 limit was being returned. That was when the old
implementation was still in place (prior cgroups v2 support). In the
old impl, when files were missing 0 was returned for some cases. In
fact, the fix of JDK-8236617 with the old impl would have wrongly
returned 0 for getTotalSwapSpaceSize() for a system which got
configured with unlimited swap size (--memory-swap=-1). In the new
impl, this returns physical swap size which seems better.

On that note, the code of JDK-8236617 seems unreachable now.

Thanks,
Severin


> Bob.
> 
> 
> > On Jun 17, 2020, at 5:13 AM, Severin Gehwolf <sgehwolf at redhat.com> wrote:
> > 
> > On Fri, 2020-06-05 at 16:07 +0200, Severin Gehwolf wrote:
> > > On Fri, 2020-06-05 at 10:30 +0000, Baesken, Matthias wrote:
> > > > > Here would be my suggestion
> > > > > (hopefully this one isn't too restrictive again):
> > > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8244500/03/webrev/
> > > > 
> > > > Hi Severin, this worked nicely .
> > > > Thanks for providing this .
> > > 
> > > Great. If somebody else is willing to OK this, I'll push it.
> > 
> > Could I have a second review of this, please? I believe it's the best
> > we can do for this test failure Matthias is seeing.
> > 
> > Thanks,
> > Severin
> >