Re: RFR: Container Fixes (8219652, 8217766, 8212528)

[Bob Vandette]

I will be committing each change individually but thought I’d save some time 
testing/reviewing.

Thanks,
Bob.


> On Mar 18, 2019, at 10:12 AM, Severin Gehwolf  wrote:
> 
> Hi Bob,
> 
> Changes look OK to me.
> 
> I'd prefer if those issues would get addressed separately, though. 3
> webrevs for 3 bugs, over lumping 3 bugs into 1 webrev. This makes it
> A) easier to review B) easier to backport to older releases if/when the
> time comes. Some of those issues are rather unrelated after all.
> 
> Thanks,
> Severin
> 
> On Thu, 2019-03-14 at 10:15 -0400, Bob Vandette wrote:
>> Ping ...
>> 
>> 
>> Please review these three fixes for Linux Docker/cgroup container
>> support.
>> 
>> WEBREV:
>> 
>> http://cr.openjdk.java.net/~bobv/8217766/webrev.0
>> 
>> ISSUE1:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8219562 - Line of code in
>> osContainer_linux.cpp#L102 appears unreachable
>> 
>> This change corrects a rarely used hotspot code path making it
>> compatible with the Java based Metrics.
>> 
>> ISSUE2:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8212528 - Wrong cgroup
>> subsystem being used for some CPU Container Metrics
>> 
>> Most Linux distros provide symbolic links for cpuacct and cpu
>> controller directories.  Docker on the Mac does not.
>> This causes some of the cpu statistics to be unreported since the
>> directory name was incorrect.
>> 
>> ISSUE3:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8217766 - Container Support
>> doesn't work for some Join Controllers combinations
>> 
>> The cgroup identification -implemented by parsing
>> /proc/self/mountinfo
>> and /proc/self/cgroup- assumed each cgroup controller was mounted
>> disjoint from the others (except for "cpu" and "cpuacct"
>> controllers).
>> Which means, we expected one single controller per mountinfo line.
>> 
>> This matches the way most Linux distributions currently configure
>> cgroupsv1 by default. Yet controllers can be grouped arbitrarily.
>> For instance, using the  systemd directive.
>> One use case for that is to let Kubernetes' kubelet discover his own
>> dedicated and reserved cgroup hierarchy. In that situation, the JVM
>> fails to discover the expected cgroup controllers set, and, when
>> running
>> containerized, default to a suboptimal understanding of available
>> resources.
>> 
>> Supporting arbitrarily controllers groups per mountpoint actually
>> allows for simpler and easier to follow code, as we don't need nested
>> if/else for every controller.
>> 
>> This fix also updates the Containers Metrics, to support joint
>> controllers.
>> 
>> 
>> Bob.
> 

Re: RFR: Container Fixes (8219652, 8217766, 8212528)

[Severin Gehwolf]

Hi Bob,

Changes look OK to me.

I'd prefer if those issues would get addressed separately, though. 3
webrevs for 3 bugs, over lumping 3 bugs into 1 webrev. This makes it
A) easier to review B) easier to backport to older releases if/when the
time comes. Some of those issues are rather unrelated after all.

Thanks,
Severin

On Thu, 2019-03-14 at 10:15 -0400, Bob Vandette wrote:
> Ping ...
> 
> 
> Please review these three fixes for Linux Docker/cgroup container
> support.
> 
> WEBREV:
> 
> http://cr.openjdk.java.net/~bobv/8217766/webrev.0
> 
> ISSUE1:
> 
> https://bugs.openjdk.java.net/browse/JDK-8219562 - Line of code in
> osContainer_linux.cpp#L102 appears unreachable
> 
> This change corrects a rarely used hotspot code path making it
> compatible with the Java based Metrics.
> 
> ISSUE2:
> 
> https://bugs.openjdk.java.net/browse/JDK-8212528 - Wrong cgroup
> subsystem being used for some CPU Container Metrics
> 
> Most Linux distros provide symbolic links for cpuacct and cpu
> controller directories.  Docker on the Mac does not.
> This causes some of the cpu statistics to be unreported since the
> directory name was incorrect.
> 
> ISSUE3:
> 
> https://bugs.openjdk.java.net/browse/JDK-8217766 - Container Support
> doesn't work for some Join Controllers combinations
> 
> The cgroup identification -implemented by parsing
> /proc/self/mountinfo
> and /proc/self/cgroup- assumed each cgroup controller was mounted
> disjoint from the others (except for "cpu" and "cpuacct"
> controllers).
> Which means, we expected one single controller per mountinfo line.
> 
> This matches the way most Linux distributions currently configure
> cgroupsv1 by default. Yet controllers can be grouped arbitrarily.
> For instance, using the  systemd directive.
> One use case for that is to let Kubernetes' kubelet discover his own
> dedicated and reserved cgroup hierarchy. In that situation, the JVM
> fails to discover the expected cgroup controllers set, and, when
> running
> containerized, default to a suboptimal understanding of available
> resources.
> 
> Supporting arbitrarily controllers groups per mountpoint actually
> allows for simpler and easier to follow code, as we don't need nested
> if/else for every controller.
> 
> This fix also updates the Containers Metrics, to support joint
> controllers.
> 
> 
> Bob.

Re: RFR: Container Fixes (8219652, 8217766, 8212528)

[David Holmes]

Hi Bob,

Sorry I took a look but can't really review it in detail as I don't know 
any of the cgroup details. Not sure who may ... perhaps Misha (cc'd).


On 15/03/2019 12:15 am, Bob Vandette wrote:

Ping ...


Please review these three fixes for Linux Docker/cgroup container support.

WEBREV:

http://cr.openjdk.java.net/~bobv/8217766/webrev.0


src/java.base/linux/classes/jdk/internal/platform/cgroupv1/Metrics.java

The copyright update should have been from:

* Copyright (c) 2018, Oracle

to

* Copyright (c) 2018, 2019, Oracle

David
-


ISSUE1:

https://bugs.openjdk.java.net/browse/JDK-8219562 - Line of code in 
osContainer_linux.cpp#L102 appears unreachable

This change corrects a rarely used hotspot code path making it compatible with 
the Java based Metrics.

ISSUE2:

https://bugs.openjdk.java.net/browse/JDK-8212528 - Wrong cgroup subsystem being 
used for some CPU Container Metrics

Most Linux distros provide symbolic links for cpuacct and cpu controller 
directories.  Docker on the Mac does not.
This causes some of the cpu statistics to be unreported since the directory 
name was incorrect.

ISSUE3:

https://bugs.openjdk.java.net/browse/JDK-8217766 - Container Support doesn't 
work for some Join Controllers combinations

The cgroup identification -implemented by parsing /proc/self/mountinfo
and /proc/self/cgroup- assumed each cgroup controller was mounted
disjoint from the others (except for "cpu" and "cpuacct" controllers).
Which means, we expected one single controller per mountinfo line.

This matches the way most Linux distributions currently configure
cgroupsv1 by default. Yet controllers can be grouped arbitrarily.
For instance, using the JoinControllers systemd directive.
One use case for that is to let Kubernetes' kubelet discover his own
dedicated and reserved cgroup hierarchy. In that situation, the JVM
fails to discover the expected cgroup controllers set, and, when running
containerized, default to a suboptimal understanding of available resources.

Supporting arbitrarily controllers groups per mountpoint actually
allows for simpler and easier to follow code, as we don't need nested
if/else for every controller.

This fix also updates the Containers Metrics, to support joint controllers.


Bob.

RFR: Container Fixes (8219652, 8217766, 8212528)

[Bob Vandette]

Ping ...


Please review these three fixes for Linux Docker/cgroup container support.

WEBREV:

http://cr.openjdk.java.net/~bobv/8217766/webrev.0

ISSUE1:

https://bugs.openjdk.java.net/browse/JDK-8219562 - Line of code in 
osContainer_linux.cpp#L102 appears unreachable

This change corrects a rarely used hotspot code path making it compatible with 
the Java based Metrics.

ISSUE2:

https://bugs.openjdk.java.net/browse/JDK-8212528 - Wrong cgroup subsystem being 
used for some CPU Container Metrics

Most Linux distros provide symbolic links for cpuacct and cpu controller 
directories.  Docker on the Mac does not.
This causes some of the cpu statistics to be unreported since the directory 
name was incorrect.

ISSUE3:

https://bugs.openjdk.java.net/browse/JDK-8217766 - Container Support doesn't 
work for some Join Controllers combinations

The cgroup identification -implemented by parsing /proc/self/mountinfo
and /proc/self/cgroup- assumed each cgroup controller was mounted
disjoint from the others (except for "cpu" and "cpuacct" controllers).
Which means, we expected one single controller per mountinfo line.

This matches the way most Linux distributions currently configure
cgroupsv1 by default. Yet controllers can be grouped arbitrarily.
For instance, using the JoinControllers systemd directive.
One use case for that is to let Kubernetes' kubelet discover his own
dedicated and reserved cgroup hierarchy. In that situation, the JVM
fails to discover the expected cgroup controllers set, and, when running
containerized, default to a suboptimal understanding of available resources.

Supporting arbitrarily controllers groups per mountpoint actually
allows for simpler and easier to follow code, as we don't need nested
if/else for every controller.

This fix also updates the Containers Metrics, to support joint controllers.


Bob.

Re: RFR: Container Fixes (8219652, 8217766, 8212528)

[Bob Vandette]

Sorry, missed a link:

http://cr.openjdk.java.net/~bobv/8217766/webrev.0/ 


Bob.

> On Mar 11, 2019, at 11:13 AM, Bob Vandette  wrote:
> 
> Please review these three fixes for Linux Docker/cgroup container support.
> 
> https://bugs.openjdk.java.net/browse/JDK-8219562 - Line of code in 
> osContainer_linux.cpp#L102 appears unreachable
> 
> This change corrects a rarely used hotspot code path to be compatible with 
> the Java based Metrics.
> 
> https://bugs.openjdk.java.net/browse/JDK-8212528 - Wrong cgroup subsystem 
> being used for some CPU Container Metrics
> 
> Most Linux distros provide symbolic links for cpuacct and cpu controller 
> directories.  Docker on the Mac does not.
> This causes some of the cpu statistics to be unreported.
> 
> https://bugs.openjdk.java.net/browse/JDK-8217766 - Container Support doesn't 
> work for some Join Controllers combinations
> 
> The cgroup identification -implemented by parsing /proc/self/mountinfo
> and /proc/self/cgroup- assumed each cgroup controller was mounted
> disjoint from the others (except for "cpu" and "cpuacct" controllers).
> Which means, we expected one single controller per mountinfo line.
> 
> This matches the way most Linux distributions currently configure
> cgroupsv1 by default. Yet controllers can be grouped arbitrarily.
> For instance, using the JoinControllers systemd directive.
> One use case for that is to let Kubernetes' kubelet discover his own
> dedicated and reserved cgroup hierarchy. In that situation, the JVM
> fails to discover the expected cgroup controllers set, and, when running
> containerized, default to a suboptimal understanding of available resources.
> 
> Supporting arbitrarily controllers groups per mountpoint actually
> allows for simpler and easier to follow code, as we don't need nested
> if/else for every controller.
> 
> This fix also updates the Containers Metrics, to support joint controllers.
> 
> Bob.
> 
> 
> 

RFR: Container Fixes (8219652, 8217766, 8212528)

[Bob Vandette]

Please review these three fixes for Linux Docker/cgroup container support.

https://bugs.openjdk.java.net/browse/JDK-8219562 - Line of code in 
osContainer_linux.cpp#L102 appears unreachable

This change corrects a rarely used hotspot code path to be compatible with the 
Java based Metrics.

https://bugs.openjdk.java.net/browse/JDK-8212528 - Wrong cgroup subsystem being 
used for some CPU Container Metrics

Most Linux distros provide symbolic links for cpuacct and cpu controller 
directories.  Docker on the Mac does not.
This causes some of the cpu statistics to be unreported.

https://bugs.openjdk.java.net/browse/JDK-8217766 - Container Support doesn't 
work for some Join Controllers combinations

The cgroup identification -implemented by parsing /proc/self/mountinfo
and /proc/self/cgroup- assumed each cgroup controller was mounted
disjoint from the others (except for "cpu" and "cpuacct" controllers).
Which means, we expected one single controller per mountinfo line.

This matches the way most Linux distributions currently configure
cgroupsv1 by default. Yet controllers can be grouped arbitrarily.
For instance, using the JoinControllers systemd directive.
One use case for that is to let Kubernetes' kubelet discover his own
dedicated and reserved cgroup hierarchy. In that situation, the JVM
fails to discover the expected cgroup controllers set, and, when running
containerized, default to a suboptimal understanding of available resources.

Supporting arbitrarily controllers groups per mountpoint actually
allows for simpler and easier to follow code, as we don't need nested
if/else for every controller.

This fix also updates the Containers Metrics, to support joint controllers.

Bob.