RFR: 8231489: GC watermark_0_1 failed due to "metaspace.gc.Fault: GC has happened too rare"

[Per Liden]

Thanks Stefan!

/Per

On 10/8/19 11:28 AM, Stefan Johansson wrote:
> 
> 
> On 2019-10-08 10:39, Thomas Schatzl wrote:
>> Hi,
>>
>> On 03.10.19 10:47, Per Liden wrote:
>>> vmTestbase/metaspace/gc/HighWaterMarkTest relies on timing and fails 
>>> when "Metaspace GC Threshold" isn't handled in a STW pause.
>>>
>>> The problem can be reproduced on both G1 and ZGC, but it's hard, as 
>>> the window is small. However, it reproduces every time when injecting 
>>> a 100ms delay to prolong the GC cycle a bit. This test used to be 
>>> disabled for G1 with ClassUnloadingWithConcurrentMark, but 
>>> JDK-8204163 enabled it about a year ago.
>>>
>>> Fixing the test properly is tricky. As far as I can see, we can either:
>>> 1) Disable this test for G1+ClassUnloadingWithConcurrentMark and ZGC, or
>>> 2) Add a sleep in the test loop, to make the race less likely to 
>>> happen, or
>>> 3) Remove the test completely, with the rational that it's a buggy 
>>> low value test.
>>>
>>> I've gone with 1) here. The test is already disabled for CMS today, 
>>> with code in the test itself (i.e. not using @requires), so I did two 
>>> alternative patches:
>>>
>>> A) Follows the existing style to disable the other GCs:
>>> http://cr.openjdk.java.net/~pliden/8231489/webrev.0-alt1
>>>
>>> B) Adds @requires to the tests using the HighWaterMarkTest class, and 
>>> removes the old check to disable CMS:
>>> http://cr.openjdk.java.net/~pliden/8231489/webrev.0-alt2
>>>
>>> I prefer B, but I don't have a strong opinion on which way to go.
>>>
>>
>> B is fine with me.
> Same here, I think it is good to use @requires even if they are a bit 
> complicated in this case.
> 
> Looks good,
> Stefan
> 
>>
>> Looks good.
>>
>> Thomas
>>