RFR: 81820709 - Container Awareness JEP

[mandy chung]

On 4/3/18 10:09 PM, Bob Vandette wrote:
> WEBREV:
>
> http://cr.openjdk.java.net/~bobv/8182070/v01/webrev

I reviewed the webrev and look okay in general. I will look through the 
javadoc next.

Metrics.java

   37  *<li> 1. All processes, including the current process within a container.

   <ol> includes the numbering. You can drop "1." and other numbers.
  
   42  *<li> or

This adds a bullet.  Maybe dropping this line.

   81      * @return The name of the provider or null if Metrics are
   82      *         not enabled.
   85     public String getProvider();

Should this method always return non-null name?

For optional metric (when it's not available), the method returns 0.  For example:
  533      * @return The number of bytes transferred or 0 if this metric is not available.

How does the client know if the metrics is not available or zero?  Or the client does not care?

jdk/internal/platform/cgroupv1/Metrics.java

  274         return SubSystem.getLongValue(cpuacct, "cpuacct.usage");

Should this be an instance method?  like cpuacct.getLongValue("cpuacct.usage");

final field name can be made all caps.

I know you are going to include regression tests.

>
> WEBREV including a Prototype MBEAN for exposing these Metrics:
>
> This prototype will not be integrated as part of this JEP.  It’s for information only.
>
> http://cr.openjdk.java.net/~bobv/8182070/v01/mbean-proto/
>
>
> This feature adds a new -XshowSetting option “system” which displays the
> available system Metrics.

What does java --help-extra show?  The help message should include 
-XshowSettings:system only on Linux.

>
> % java -XshowSettings:system

I expect this option shows static/configuration information rather than 
timing statistics e.g. CPU time and usage.  It may be a smaller set but 
it may be good information though.

It's more appropriate for monitoring tools to show the timing statistics 
and resource consumption rather than the launcher.

Mandy