RFR: 81820709 - Container Awareness JEP

[Bob Vandette]

Hi Mandy,

I’m finally getting back to your review of this change now that we’ve made some progress on creating tests.

BTW: This Jira issue is now an RFE rather than a JEP (https://bugs.openjdk.java.net/browse/JDK-8203357 <https://bugs.openjdk.java.net/browse/JDK-8203357>)

See comments below ...

> On Apr 17, 2018, at 10:25 PM, mandy chung <mandy.chung at oracle.com> wrote:
> 
> 
> 
> On 4/3/18 10:09 PM, Bob Vandette wrote:
>> WEBREV:
>> 
>> http://cr.openjdk.java.net/~bobv/8182070/v01/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.
Done
> 
>   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?
Done
> 
> 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?
Unfortunately this is the way that cgroups works.  If the kernel has not been configured to provide the
metrics, the values are all 0’s.  I’m not sure what I can do about this behavior except document it as I have
done in the JavaDocs.
> 
> jdk/internal/platform/cgroupv1/Metrics.java
> 
>  274         return SubSystem.getLongValue(cpuacct, "cpuacct.usage");
> 
> Should this be an instance method?  like cpuacct.getLongValue("cpuacct.usage”);
I did it this way in order to provide a centralized place to check for missing subsystems.  The getLongValue
method does the checking for all subsystems.

> 
> final field name can be made all caps.

Not sure what this is in reference to, please advise?
> 
> 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/ <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.  

The message looks like it comes out of a resource file will need to be localized.
How do we make the message conditional on operating system in that case?  Can I just put
(Linux Only) in the english version and then get it localized?

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

I’ve removed any reporting of resource consumption APIs.  

Here’s the new output:

./java -XshowSettings:system
Operating System Metrics:
    Provider: cgroupv1
    Effective CPU Count: 24
    CPU Period: 100000
    CPU Quota: -1
    CPU Shares: -1
    List of Processors, 24 total: 
    0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 
    List of Effective Processors, 24 total: 
    0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 
    List of Memory Nodes, 2 total: 
    0 1 
    List of Available Memory Nodes, 2 total: 
    0 1 
    CPUSet Memory Pressure Enabled: false
    Memory Limit: Unlimited
    Memory Soft Limit: Unlimited
    Memory & Swap Limit: Unlimited
    Kernel Memory Limit: Unlimited
    TCP Memory Limit: 2048.00T
    Out Of Memory Killer Enabled: true

I’ll be sending out a webrev that includes the tests next week once I’ve integrated them with my change and
perform some testing on different Linux systems and docker containers.

Thanks,
Bob.

> 
> Mandy
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180523/f82fd06e/attachment.html>