RFR of 8194284: CheckRegisterInLog.java fails with java.lang.RuntimeException: CheckRegisterInLog got exception timeout 6480000ms out of range

Hamlin Li huaming.li at oracle.com
Fri Jan 19 06:28:24 UTC 2018


On 19/01/2018 2:11 PM, David Holmes wrote:
> Hi Hamlin,
>
> On 19/01/2018 3:04 PM, Hamlin Li wrote:
>> Hi Roger, David
>>
>> Please check the updated webrev at: 
>> http://cr.openjdk.java.net/~mli/8194284/webrev.00/
>
> RMID.java:
>
> This comment no longer makes sense:
>
>       // when restart rmid, it may take more time than usual because of
>       // "port in use" by a possible interloper (check JDK-8168975),
>       // so need to set a longer timeout for restart.
> !     private static final long RESTART_TIMEOUT = (long)(TIMEOUT_BASE 
> * 0.9);
>
> Actually I'm not sure it originally made sense - longer than what? But 
> as it stands RESTART_TIMEOUT is smaller than TIMEOUT_BASE so the 
> comment really seems odd. Perhaps 8168975 will shed some light on the 
> intent. ??
Hi David,

The comment still makes sense.
Before 8168975, restart() calls start() which is indeed 
start(STARTTIME_MS), where STARTTIME_MS is 15_000L, but we found out in 
some situation restart() needs more time than start();
So in 8168975, restart() calls start(restartTimeout), where 
restartTimeout is RESTART_TIMEOUT in 8194284.
>
> The TestLibrary change looks good.
Do you also think the comment makes sense with my explanation?

Thank you
-Hamlin
>
> Thanks,
> David
>
>> Thank you
>>
>> -Hamlin
>>
>>
>> On 18/01/2018 10:33 PM, Roger Riggs wrote:
>>> Hi Hamlin,
>>>
>>> The base bug is that the timeoutFactor is being applied twice, once 
>>> in the RMID constructor
>>> and again in computeDeadline.  It is better to cleanup the 
>>> implementation of the test library
>>> than to fudge the values.  Either respect the timeouts passed in 
>>> (remove the scaling from computeDeadline)
>>> or consistently leave it to the lower level routines.  Pick 1.
>>>
>>> Thanks, Roger
>>>
>>>
>>> On 1/18/2018 1:59 AM, Hamlin Li wrote:
>>>> Would you please review the following patch?
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8194284
>>>>
>>>> webrev as below.
>>>>
>>>>
>>>> Thank you
>>>>
>>>> -Hamlin
>>>>
>>>> ------------------------------------------------------------------------ 
>>>>
>>>>
>>>> diff -r 0dec8c41170c test/jdk/java/rmi/testlibrary/TestLibrary.java
>>>> --- a/test/jdk/java/rmi/testlibrary/TestLibrary.java    Wed Jan 17 
>>>> 20:07:50 2018 -0800
>>>> +++ b/test/jdk/java/rmi/testlibrary/TestLibrary.java    Thu Jan 18 
>>>> 14:54:50 2018 +0800
>>>> @@ -188,8 +188,12 @@
>>>>      public static long computeDeadline(long timestamp, long 
>>>> timeout) {
>>>>          final long MAX_TIMEOUT_MS = 3_600_000L;
>>>>
>>>> -        if (timeout < 0L || timeout > MAX_TIMEOUT_MS) {
>>>> +        if (timeout < 0L) {
>>>>              throw new IllegalArgumentException("timeout " + 
>>>> timeout + "ms out of range");
>>>> +        } else if (timeout > MAX_TIMEOUT_MS) {
>>>> +            System.out.format("timeout value(%d) exceeds 
>>>> MAX_TIMEOUT_MS(%d), "
>>>> +                    + "use MAX_TIMEOUT_MS instead!%n", timeout, 
>>>> MAX_TIMEOUT_MS);
>>>> +            timeout = MAX_TIMEOUT_MS;
>>>>          }
>>>>
>>>>          return timestamp + (long)(timeout * getTimeoutFactor());
>>>>
>>>
>>



More information about the core-libs-dev mailing list