RFR: 8226575: OperatingSystemMXBean should be made container aware

[Mandy Chung]

On 12/5/19 12:50 PM, Bob Vandette wrote:
>
>>>> It may worth considering adding Metrics::getSwapLimit and
>>>> Metrics::getSwapUsage and move the computation to the implementation of
>>>> Metrics.  Bob may have an opinion.
>> There was no any new input regarding this so I decided to leave it unchanged.
> Sorry, I didn’t respond to this.  Since the calculation required for getFreeSwapSpaceSize requires retries
> due to the access of multiple changing values, I think it’s best to leave things as they are so the caller of
> these methods understands the limitations of the API.

OK with me.
> Also, the fact that swap size metrics include memory sizes is fully documented in both the cgroup and docker
> online documentation so it’s probably best to be consistent.
>
>>>> 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?
>> Per Docker docs the minimum allowed value for  memory limit (--memory option) is 4 megabytes.
>> And if memory limit is unset the return value is -1.  Thus, in my understanding the value 0 is only possible
>> if something went wrong while retrieving this metric.
> That is true but shouldn’t you return -1 in that case?
>
> I originally thought it was ok to fall back to the host data for 0 values but I think its better to return unavailable (-1)
> I think you might want to change all >= 0 to > 0 and return -1 if any of the values are 0.  This would be more consistent.

+1

The javadoc should be changed and returns -1 when it's unavailable and 
the CSR should also be updated to reflect this.    I'm sure Joe can 
re-approve the CSR quickly when the fix is reviewed and approved.

> You should only fall back to the original logic (host values) if container values are set to unlimited.
>
+1

Mandy