Hi Bob, Note: I've changed the bug title to be version-less. This is really an issue in the detection logic irrespective of cgroup version in use.
On Fri, 2020-02-21 at 10:12 -0500, Bob Vandette wrote: > > On Feb 21, 2020, at 9:30 AM, Severin Gehwolf <sgehw...@redhat.com> wrote: > > > > Hi Bob, > > > > On Fri, 2020-02-21 at 09:11 -0500, Bob Vandette wrote: > > > Severin, > > > > > > Don’t we need the contents of /proc/self/mountinfo in order to construct > > > the path to the cgroup controllers? > > > > There is only one for unified (cgroups v2), but yes it's beeing used. > > See CgroupV2Subsystem.initSubsystem() and > > CgroupV1Subsystem.initSubsystem(). For affected systems, no controllers > > are mounted, so the effect will be null Metrics, as before JDK-8231111. > > Maybe I didn't understand the question, sorry. > > If you don’t have access to the information required to get metrics, I just > assumed that > you would return NULL in CgroupSubsystemFactory.create() rather than making > the > assumption that it works only to fail later. You are right. It makes little sense to continue in that case. Updated webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8239559/02/webrev/ > Alternatively, we could consider assuming the mount point is /sys/fs/cgroup > for cgroupv1 in > the case you are trying to support. This would involve using > /proc/self/cgroup to get the list > of controllers and then use that list to call createSubSystemController and > setSubSystemControllerPath with the default path. > > I think we need to understand the extent of the problem on these older > systems before > deciding a course of action. Do we see the same empty mountinfo file in a > docker container > running on these older systems or is this just a host issue? If docker > containers work fine, then > I wouldn’t bother trying to make this work. That's the thing. I don't think any of those older systems support docker in the first place. Thanks, Severin > Bob. > > > > On Thu, 2020-02-20 at 14:50 +0000, Baesken, Matthias wrote: > > > > Hi Severin, > > > > > > > > grep cgroup /proc/self/mountinfo > > > > > > > > returns nothing. > > > > > > > > Best Regards, Matthias > > > > > > > > > > Assuming your fix is correct, don’t we also need to apply the same change > > > to the hotspot source cgroupSubsystem_linux.cpp? > > > > Yes, tracked with JDK-8239785. > > > > Thanks, > > Severin > > > > > Bob; > > > > > > > > > > On Feb 21, 2020, at 8:32 AM, Severin Gehwolf <sgehw...@redhat.com> > > > > wrote: > > > > > > > > Hi, > > > > > > > > Could I please get a review of this fix to the detection heuristic of > > > > cgroup v1 vs cgroup v2? Matthias (in CC) discovered that on some old > > > > systems the JDK Metrics code throws InternalError caused by wrong > > > > detection logic when Metrics are being created on Linux. > > > > > > > > The reason for this is that hierarchy IDs of 0 in /proc/cgroups is > > > > being used as a heuristic to detect cgroups v2 systems. Apparently some > > > > old systems like RHEL 6 and SLES 11 have no cgroups controllers > > > > mounted, thus, triggering a false positive. > > > > > > > > The fix is to also look at /proc/self/mountinfo and correct logic in > > > > this case. > > > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8239559 > > > > webrev: > > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8239559/01/webrev/ > > > > > > > > Testing: docker/cgroups tests on hybrid (cgroups v1) and unified > > > > hierarchy (cgroups v2). New regression test. Looks good here. > > > > > > > > Unfortunately, I wasn't able to reproduce this on an actual affected > > > > system. I somewhat reproduced via the derived regression test based on > > > > data from reporters. I'd appreciate any testing on systems where this > > > > reproduces. > > > > > > > > Thanks, > > > > Severin > > > >
