RFR - JDK-8209517: com/sun/jdi/BreakpointWithFullGC.java fails with timeout

[Daniel D. Daugherty]

On 8/15/18 8:46 PM, Alex Menkov wrote:
> Hi Dan,
>
> Thank you for the review.
> See replies inline.
>
> On 08/15/2018 17:25, Daniel D. Daugherty wrote:
>> On 8/15/18 6:42 PM, Alex Menkov wrote:
>>> Hi all,
>>>
>>> please review a fix for
>>> https://bugs.openjdk.java.net/browse/JDK-8209517
>>> webrev:
>>> http://cr.openjdk.java.net/~amenkov/sh2java/step1_regression/webrev/
>>
>> test/jdk/com/sun/jdi/BreakpointWithFullGC.java
>>      No comments.
>>
>> test/jdk/com/sun/jdi/lib/jdb/Jdb.java
>>      L101:                     30, TimeUnit.SECONDS);
>>          I'm assuming this is a 30 second timeout for getting the
>>          initial "Listening for transport..." message. Keep in mind
>>          that this may need to scale with timeoutFactor. I have seen
>>          runs with -Xcomp take several minutes here...
>>
>>      L168:                 waitFor(10, TimeUnit.SECONDS);
>>      L178:                 debuggee.waitFor(10, TimeUnit.SECONDS);
>>          10 seconds may not be enough on a loaded machine with
>>          lots of parallel processes. Keep in mind that this
>>          may need to scale with timeoutFactor.
>
> Good point.
> I'll think how to handle this properly and will update timeouts in one 
> of the subsequent fixes.

Agreed. I should have been more clear. We'll keep an eye open
timeouts with this test and if they happen, we should tweak
the timeout value algorithm.

> I'd prefer to push this fix as soon as I get 2nd approval (as it 
> produces noise in CI).

Very much agreed. These failures are happening in every JDK12
tier3 CI job set (for some reason).


>
>>
>> test/jdk/com/sun/jdi/lib/jdb/JdbCommand.java
>>      No comments.
>>
>> test/jdk/com/sun/jdi/lib/jdb/JdbTest.java
>>      No comments.
>>
>> Looks good to me. Is BreakpointWithFullGC.java the only test
>> that uses the com/sun/jdi/lib/jdb/*.java library code? That
>> would surprise me, but I haven't been in this test code for
>> a very long time.
>
> com/sun/jdi/lib/jdb classes were introduced to be used by tests 
> converted from com/sun/jdi/*.sh 
> (https://bugs.openjdk.java.net/browse/JDK-8201652)
> For now 2 tests are converted:
> test/jdk/com/sun/jdi/BreakpointWithFullGC.java
> test/jdk/com/sun/jdi/ArrayLengthDumpTest.java
>
> ArrayLengthDumpTest.java does not require to be updated (works fine 
> with updated lib/jdb)

Thanks for closing the loop on this.

Dan


>
> --alex
>
>>
>> Thumbs up!
>>
>> Dan
>>
>>
>>>
>>> Cause of the BreakpointWithFullGC failures is a mess of jdb and 
>>> debuggee outputs (the test runs debuggee by using CommandLineLaunch 
>>> connector, so jdb redirects debuggee stdout to its own stdout).
>>> To solve it test framework was updated to launch debuggee first 
>>> (redirecting its output) and then connecting jdb to existing process.
>>> The approach allow to drop "simple prompt" logic (jdb mode when 
>>> debugee is not yet running).
>>>
>>> Mach5 passed 400 runs ("4 std platforms" x "--test-repeat 100") 
>>> without any issues.
>>>
>>> --alex
>>