On 12/3/19 9:40 PM, Daniil Titov wrote:
Under what circumstance that limit or memLimit is < 0?
The memory limit metrics is not available if JVM runs on Linux host ( not in a
docker container) or if a docker container was started without
specifying a memory limit ( without '--memory=' Docker option) . In latter
there is no limit on how much memory the container can use and
it can use as much memory as the host's OS allows.
OK. Please add a comment to the code.
It may worth considering adding Metrics::getSwapLimit and
Metrics::getSwapUsage and move the computation to the implementation of
Metrics. Bob may have an opinion.
Also it seems correct for the memory related methods to check if
(containerMetrics != null && containerMetrics.getMemoryLimit() >= 0).
BTW what does it mean if limit == 0?
Is it worth specifying this case?
I believe yes, since it covers the cases when JVM runs on a Linux host or a
docker container was started without memory limitation.
I was wondering if the javadoc should specify that.
It fallbacks to return the system's total swap space size - this is not really
what it should report.
For the case when JVM runs on a Linux host it is exactly what we want. The only
problematic case is if JVM runs inside a docker container without a memory
limit set.
However, I am not sure how we could differentiate these 2 cases.
As this is the case when the limit is not set in the container, it
returns the system metrics which sounds appropriate.
Similarly, getFreeMemorySize and getTotalMemorySize and getCpuLoad.
For getTotalMemorySize I think we are good here. If limit is not set then all
memory the host's OS have is available.
For getFreeMemorySize the problematic case is if is the memory limit is set but
the memory usage for some reason is not available
(containerMetrics.getMemoryUsage() returns 0).
Will zero memory usage happen?
Probably in this case we should just return -1 as currently
getFreePhysicalMemorySize0() does if it cannot retrieve a valid result.
For getCpuLoad() the problematic case if CPU quotas are active but CpuPeriod,
CpuNumPeriods , or getCpuUsage are unavailable or if a valid CPU load for some
CPU was
not retrieved, or if all retrieved CPU load values happen to be zeros. Probably
we should just return -1 in these cases rather then falling back to
getSystemCpuLoad0()
returning -1 sounds right.
src/jdk.management/windows/classes/com/sun/management/internal/OperatingSystemImpl.java
There is no strong need to make the deprecated methods as default methods.
If they were default methods, they only need to be implemented once as opposed
to in all OS-specific implementations.
I could make these methods defaults if you feel it is a better approach here.
It's not strictly needed but I can go either way.
CheckOperatingSystemMXBean.java
System.out.println(String.format(...)) can simply be replaced with
System.out.format.
I will include this change in the next webrev, thank you!
thanks
Mandy
