gaturchenko commented on code in PR #2587:
URL: https://github.com/apache/systemds/pull/2587#discussion_r3829093439
##########
src/test/java/org/apache/sysds/test/FederatedWorkerUtils.java:
##########
@@ -25,20 +25,24 @@
import java.util.function.BooleanSupplier;
/**
- * Test helpers that block until a federated worker is accepting TCP
connections on its port.
- *
- * <p>The federated worker opens its TCP port after Netty's {@code
bind().sync()} returns; a successful
- * TCP connect to that port therefore indicates that the worker is ready to
accept requests. The methods
- * here poll for that signal and throw {@link RuntimeException} on timeout or
if the underlying
- * {@code Process}/{@code Thread} exits before the port becomes ready.
+ * Test helpers that block until a federated worker is accepting TCP
connections on its port. The federated worker opens
+ * its TCP port after Netty's {@code bind().sync()} returns; a successful TCP
connect to that port therefore indicates
Review Comment:
Done, please see
[a4b2adc](https://github.com/apache/systemds/pull/2587/commits/a4b2adc2c09f20bd732cec975c9846707cff5324)
##########
src/test/java/org/apache/sysds/test/FederatedWorkerUtils.java:
##########
@@ -174,16 +180,33 @@ public static void waitForWorkers(int[] ports, int
timeoutMs, java.util.function
}
}
- private static boolean tryConnect(int port) {
+ private static boolean tryConnect(int port, long deadline) {
+ final int timeout = attemptTimeout(deadline -
System.currentTimeMillis());
+ if(timeout == 0) // out of time, do not start another attempt
+ return false;
try(Socket s = new Socket()) {
- s.connect(new InetSocketAddress("localhost", port),
CONNECT_TIMEOUT_MS);
+ s.connect(new InetSocketAddress("localhost", port),
timeout);
return true;
}
- catch(IOException e) {
+ catch(IOException e) { // closed port, or a handshake that
outlasted the budget
return false;
}
}
+ /**
+ * Budget for a single connect attempt, capped by the time left until
the overall deadline so that one slow attempt
+ * cannot substantially exceed the limit.
+ *
+ * @param remainingMs time left until the deadline, in ms
+ * @return the timeout to pass to {@link Socket#connect}, or 0 if no
attempt should be made. Never returns 0 while
+ * time is left, because {@code connect} reads a timeout of 0
as 'infinite'.
+ */
+ public static int attemptTimeout(long remainingMs) {
Review Comment:
Done, please see
[a4b2adc](https://github.com/apache/systemds/pull/2587/commits/a4b2adc2c09f20bd732cec975c9846707cff5324)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
