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. ??
The TestLibrary change looks good.
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());
