RFR : JDK-8175542 - JMX: Not enough JDP packets received

[David Holmes]

Hi Amit, Harsha,

On 11/01/2018 4:24 PM, Amit Sapre wrote:
> Hello,
> 
> Ping. Can somebody review this code changes ?

I took a look. If I understand correctly the way the test works is that 
it sets up a socket timeout of "timeout" - to be applied to the 
receive() call - and then allows 20% on top of that value for the 
overall execution time (between the recording of the start time and the 
check of hasTestLivedLongEnough()). The problem we see is that if the 
receive() and related processing takes us past the end deadline then we 
report a timeout, even though we actually managed to complete the test 
logic successfully. The fix simply checks for success before checking 
for the overall timeout - that seems reasonable.

The only concern I'd have - and there's no way to completely mitigate 
this - is that if the logging+connection+creation+receive combined take 
a ridiculously long time (but within the overall jtreg timeout) then we 
won't notice as long as the receive+packet-processing completes okay.

Two things I'd do in the future (if this starts timing out again) before 
changing the overall timeout-factor would be to try:

a) recording the start time just before the loop, instead of before the 
logging and the socket connection and Datagram creation**; and/or

b) changing the timeout overhead allowance from 20% to, say, 30%.

** this has the same problem of hiding unexpected delays in the 
logging+connection etc.

But the proposed fix seems adequate for now.

Thanks,
David

> Thanks,
> 
> Amit
> 
> *From:*Amit Sapre
> *Sent:* Wednesday, January 03, 2018 5:54 PM
> *To:* Harsha wardhana B; serviceability-dev at openjdk.java.net
> *Subject:* RE: RFR : JDK-8175542 - JMX: Not enough JDP packets received
> 
> Thanks Harsha for the inputs.
> 
> I made changes in this webrev : 
> http://cr.openjdk.java.net/~asapre/webrev/2018/JDK-8175542/webrev.01/
> 
> Thanks,
> 
> Amit
> 
> *From:*Harsha Wardhana B
> *Sent:* Wednesday, December 13, 2017 5:32 PM
> *To:* serviceability-dev at openjdk.java.net 
> <mailto:serviceability-dev at openjdk.java.net>
> *Subject:* Re: RFR : JDK-8175542 - JMX: Not enough JDP packets received
> 
> Hi Amit,
> 
> Increasing the timeout 'TIME_OUT_FACTOR' will increase both socket and 
> test-case timeout. The test passed as can be seen from the log, but 
> because of the race-condition: hasTestLivedLongEnough() checked before 
> shouldContinue(), the test was declared failed because of time-out.
> 
> One possible fix would be to change line 80 to,
> 
>                  if (shouldContinue() && hasTestLivedLongEnough())
> 
> Thanks
> 
> Harsha
> 
> On Wednesday 13 December 2017 11:03 AM, Amit Sapre wrote:
> 
>     Hello,
> 
>     Please review the timeout fix for this bug.
> 
>     Bug ID : https://bugs.openjdk.java.net/browse/JDK-8175542
> 
>     Webrev :
>     http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-8175542/webrev.00/ <http://cr.openjdk.java.net/%7Easapre/webrev/2017/JDK-8175542/webrev.00/>
> 
>     Thanks,
> 
>     Amit
>