Hi Hamlin,
On 19/01/2018 4:28 PM, Hamlin Li wrote:
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.
Okay, then I suggest changing:
// so need to set a longer timeout for restart.
to
// so need to set a longer timeout than STARTTIME_MS for restart.
Thanks,
David
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());
