Re: RFR: 81820709 - Container Awareness JEP

[mandy chung]

On 5/24/18 12:31 PM, Bob Vandette wrote:

Yes, I saw that but wasn’t sure how new text that’s added to the
launcher.properties file would get localized.  Is there a process for
getting this done?
No action should be needed.  Changes to .properties files are tracked 
and localized version will be updated by the globalization team.


Mandy

Re: RFR: 81820709 - Container Awareness JEP

[Bob Vandette]

> On May 24, 2018, at 2:42 PM, mandy chung  wrote:
> 
> 
> 
> On 5/23/18 7:39 AM, Bob Vandette wrote:
>>> 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
> 
> 137 if (subsystem == null) return 0L;
> 
> should this throw NPE?  same applies to all getXXXValue methods.
> 
> I think instance methods are appropriate since they obtain
> the stat for a given subsystem unless null subsystem can
> be passed as argument?

There will be situations where some platforms will not have all subsystems 
available.
I’ve documented the APIs to state that if a specific Metric is not available, 
the return
will be 0 (null).  This is implemented by passing the unavailable subsystem 
(NULL)
to the query methods. 

> 
>  73 public String Path() {
>  74 return path;
>  75 }
> 
> Just notice the method name "Path()" - should be lowercase "path()”?

Ok.

> 
>> Not sure what this is in reference to, please advise?
> 
>  51 private static final Metrics instance = initContainerSubSystems();
>  53 private static final String providerName = "cgroupv1";
> 
> INSTANCE and PROVIDER_NAME

Ok, thanks.  I’ll take care of that.

> 
>>> 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?
> 
> The existing launcher.properties lists platform-specific options
> in text form:
> 
> The following options are Mac OS X specific:\n\
>-XstartOnFirstThread
>:
> 
> That's one possibility.

Yes, I saw that but wasn’t sure how new text that’s added to the 
launcher.properties file would get
localized.  Is there a process for getting this done?

Bob.

> 
>> Here’s the new output:
>> ./java -XshowSettings:system
> 
> Thanks for trimming the output.
> 
>> 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.
> 
> Sounds good.
> 
> Thanks
> Mandy

Re: RFR: 81820709 - Container Awareness JEP

[mandy chung]

On 5/23/18 7:39 AM, Bob Vandette wrote:

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


 137 if (subsystem == null) return 0L;

should this throw NPE?  same applies to all getXXXValue methods.

I think instance methods are appropriate since they obtain
the stat for a given subsystem unless null subsystem can
be passed as argument?

  73 public String Path() {
  74 return path;
  75 }

Just notice the method name "Path()" - should be lowercase "path()"?


Not sure what this is in reference to, please advise?


  51 private static final Metrics instance = initContainerSubSystems();
  53 private static final String providerName = "cgroupv1";

INSTANCE and PROVIDER_NAME


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?


The existing launcher.properties lists platform-specific options
in text form:

The following options are Mac OS X specific:\n\
-XstartOnFirstThread
:

That's one possibility.


Here’s the new output:

./java -XshowSettings:system


Thanks for trimming the output.


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.


Sounds good.

Thanks
Mandy

Re: 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 
)

See comments below ...

> On Apr 17, 2018, at 10:25 PM, mandy chung  wrote:
> 
> 
> 
> 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  * 1. All processes, including the current process within a 
> container.
> 
>includes the numbering. You can drop "1." and other numbers.
>  
>   42  * 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/ 
>> 
>> 
>> 
>> 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: 10
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
> 

Re: 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  * 1. All processes, including the current process within a container.

   includes the numbering. You can drop "1." and other numbers.
 
  42  * 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

RFR: 81820709 - Container Awareness JEP

[Bob Vandette]

Here is a first pass at an implementation of the Container Awareness JEP.
This JEP adds an implementation of an internal API for the extraction of system 
metrics
for processes running in Isolation Groups (Containers).  The plan is to get the 
internal 
API integrated in JDK 11 with support for Linux x64 and then follow this work 
up with support
for alternate platforms, the addition of a JMX MBean and Java Flight Recorder.


JEP:

https://bugs.openjdk.java.net/browse/JDK-8182070


JAVADOC:

http://cr.openjdk.java.net/~bobv/8182070/v01/javadoc/jdk/internal/platform/Metrics.html


WEBREV:

http://cr.openjdk.java.net/~bobv/8182070/v01/webrev


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.

% java -XshowSettings:system

Operating System Metrics:
Provider: cgroupv1
Effective CPU Count: 24
CPUTime per Processor: 
[0]: 52805305 (ns)
[1]: 70799492 (ns)
[2]: 27449618 (ns)
[3]: 12957734 (ns)
[4]: 38382720 (ns)
[5]: 20325731 (ns)
[6]: 36374924 (ns)
[7]: 40279640 (ns)
[8]: 17557347 (ns)
[9]: 19056675 (ns)
[10]: 66185888 (ns)
[11]: 56539480 (ns)
[12]: 10009386 (ns)
[13]: 19139797 (ns)
[14]: 2257349 (ns)
[15]: 8712468 (ns)
[16]: 10306911 (ns)
[17]: 9814800 (ns)
[18]: 3516611 (ns)
[19]: 747174 (ns)
[20]: 4380756 (ns)
[21]: 11803118 (ns)
[22]: 1076297 (ns)
[23]: 8069315 (ns)

CPU Usage is: 550599580 (ns)
CPU User Usage is: 36 (ticks)
CPU System Usage is: 10 (ticks)
CPU Period: 10
CPU Quota: -1
CPU Shares: -1
CPU Number of Periods: 0
CPU Number of Throttled Periods: 0
CPU Throttled Time: 0
CPUSet Exclusive: false
CPUSet Memory Exclusive: false
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
CPUSet Memory Pressure: 0.0
Memory Failed Count: 0
Memory Limit: Unlimited
Memory Used: 43.31M
Max Memory Used: 48.82M
Memory Soft Limit: Unlimited
Memory & Swap Failed Count: 0.00K
Memory & Swap Limit: Unlimited
Memory & Swap Used: 43.93M
Max Memory & Swap Used: 48.82M
Kernel Memory Failed Count: 0.00K
Kernel Memory Limit: Unlimited
Kernel Memory Used: 0.00K
Kernel Max Memory Used: 0.00K
TCP Memory Failed Count: 0.00K
TCP Memory Limit: Unlimited
TCP Memory Used: 0.00K
TCP Max Memory Used: 0.00K
Out Of Memory Killer Enabled: true
BLKIO: Number of I/O Operations Completed: 42
BLKIO: Bytes Transferred from disk: 4923392

Bob Vandette