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

[Amit Sapre]

Thanks Harsha & David for the review.

I agree that this fix may not address all the other scenarios which you mentioned. Thanks for bringing it.
This fix is an attempt for making sure that tests reports pass if desired number of packets are received.

David,
I will added all your suggestions to this bug  for further reference, in case we hit another timeout issue.

Thanks,
Amit

> -----Original Message-----
> From: David Holmes
> Sent: Thursday, January 11, 2018 12:39 PM
> To: Amit Sapre; Harsha wardhana B; serviceability-dev at openjdk.java.net
> Subject: Re: RFR : JDK-8175542 - JMX: Not enough JDP packets received
> 
> 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.0
> > 0/>
> >
> >     Thanks,
> >
> >     Amit
> >