[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    RE: RFR : JDK-8175542 - JMX: Not enough JDP packets received
From:       Amit Sapre <amit.sapre () oracle ! com>
Date:       2018-01-15 5:50:47
Message-ID: bde33b97-1fa5-411b-b4c5-303c074ef812 () default
[Download RAW message or body]

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@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@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@openjdk.java.net
> > <mailto:serviceability-dev@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
> > 


[prev in list] [next in list] [prev in thread] [next in thread]