RFR [XS]: 8244500: jtreg test error in test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java

[David Holmes]

Hi Matthias,

On 16/05/2020 12:59 am, Baesken, Matthias wrote:
> Hello David ,  I get  the warning  always on this linux machine (I think it depends on machine setup and/or kernel   version).
> 
>> Surely it must be specified to return 0  or some such sentinel in such a case?
> 
> The doc just says this :
> 
> 62    /**
> 63     * Returns the total amount of swap space in bytes.
> 64     *
> 65     * @return the total amount of swap space in bytes.
> 66     */
> 67    public long getTotalSwapSpaceSize();
> 
> ( however  it returned previously 0 before the latest changes in the container support code came in ).

That sounds like a bug was introduced to me then.

> Regarding  the  getFreeSwapSpaceSize  issue  ,   Jie   provided a change to  src/jdk.management/unix/classes/com/sun/management/internal/OperatingSystemImpl.java
>   Because of this   I could remove  the try-construct .
> 
> New  webrev :
> http://cr.openjdk.java.net/~mbaesken/webrevs/8244500.3/

I'm quite confused by your changes. Your first version simply adjusted 
the test to check for the warning. But now you are also changing the 
main logic. I can't assess the validity of what you are doing sorry.

I've cc'd Severin and Bob as I think they need to evaluate what the real 
problem is here and when it was introduced.

David
----

> Best regards, Matthias
> 
> 
> 
> -----Original Message-----
> From: David Holmes <david.holmes at oracle.com>
> Sent: Donnerstag, 14. Mai 2020 07:02
> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
> Cc: jiefu(傅杰) <jiefu at tencent.com>
> Subject: Re: RFR [XS]: 8244500: jtreg test error in test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java
> 
> On 14/05/2020 12:28 am, Baesken, Matthias wrote:
>> Hello, here is a new webrev removing the comment :
>>
>> http://cr.openjdk.java.net/~mbaesken/webrevs/8244500.1/
>>
>> It removes the comment , and it also  handles  0  for getFreeSwapSpaceSize
>> (  this can occur  after   JDK-8242480 : Negative value may be returned by getFreeSwapSpaceSize() in the docker   was pushed  recently ).
> 
> Sorry Matthias but I still see this partly as a bug in the underlying
> container support code. If you get the warning then what does
> getTotalSwapSpaceSize() return? Surely it must be specified to return 0
> or some such sentinel in such a case?
> 
> Other comments:
> 
> !         try {
> !
> out.shouldMatch("OperatingSystemMXBean\\.getFreeSwapSpaceSize:
> [1-9][0-9]+");
> !         } catch(RuntimeException ex) { // allow 0, see 8242480
> !
> out.shouldContain("OperatingSystemMXBean.getFreeSwapSpaceSize: 0");
> !         }
> 
> can't you just change the original regex to match >= 0?
> 
> Style nit: avoid historical comments like "see 8242480" and "since
> 8231111 ...".
> 
> Thanks,
> David
> 
>> Best regards, Matthias
>>
>> -----Original Message-----
>> From: Baesken, Matthias
>> Sent: Mittwoch, 13. Mai 2020 10:11
>> To: 'David Holmes' <david.holmes at oracle.com>; 'hotspot-dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
>> Subject: RE: RFR [XS]: 8244500: jtreg test error in test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java
>>
>>
>>> But according to the comment immediately before your changes:
>>>
>>> // in case of warnings like : "Your kernel does not support swap limit
>>> capabilities or the cgroup is not mounted. Memory limited without swap."
>>> // the getTotalSwapSpaceSize does not return the expected result, but 0
>>>
>>> we should be returning zero. So this seems to be a bug in the
>>> implementation not in the test.
>>
>>    
>> Hi David , I think  in case of  "Your kernel does not support swap limit  capabilities   ... "    we just do not get the values
>>    that are normally expected.
>> Previously we indeed got "0" .
>> Probably the comment should be deleted .
>>
>> Best regards, Matthias
>>