You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu> on 2026/08/13 18:12:26 UTC
[S] Change in asterixdb[trinity]: [NO ISSUE][HYR][NET] Bound the wait for an IPC handle to connect
From Michael Blow <mb...@apache.org>:
Michael Blow has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21547?usp=email )
Change subject: [NO ISSUE][HYR][NET] Bound the wait for an IPC handle to connect
......................................................................
[NO ISSUE][HYR][NET] Bound the wait for an IPC handle to connect
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- IPCHandle.waitTillConnected() waited on the monitor with no deadline,
as its own TODO noted. A handle which never reached CONNECTED or
CLOSED therefore parked its caller forever, and the maxRetries loop in
IPCConnectionManager.getIPCHandle() - which exists precisely to
recover from a failed connect - was unreachable
- take a timeout, and on expiry mark the handle CLOSED and return false,
which is what getIPCHandle() already interprets as a failed connection
- default 120s, overridable with hyracks.ipc.connect.timeout.sec. It
must stay above the SSL handshake deadline (60s by default) so that a
handshake still legitimately in progress is not abandoned here
- teardown of the abandoned channel still relies on the network thread's
existing close paths; marking the handle CLOSED at least lets
unregisterHandle() reap it rather than leaving it in the handle map
- this is a backstop. With the SSL handshake now bounded, a stalled
handshake fails and closes its channel on its own; this covers the
non-SSL stalls and anything else that leaves a handle untransitioned
Ext-ref: MB-73310
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
Change-Id: I1199e4e8b08eb6abf1649142c8efa7dd00600e27
---
M hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/impl/IPCConnectionManager.java
M hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/impl/IPCHandle.java
2 files changed, 28 insertions(+), 4 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/47/21547/1
diff --git a/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/impl/IPCConnectionManager.java b/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/impl/IPCConnectionManager.java
index c25cf02..743a9cc 100644
--- a/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/impl/IPCConnectionManager.java
+++ b/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/impl/IPCConnectionManager.java
@@ -56,6 +56,9 @@
private static final int INITIAL_RETRY_DELAY_MILLIS = 100;
private static final int MAX_RETRY_DELAY_MILLIS = 15000;
private static final int MAX_STOP_JOIN_WAIT_MILLIS = 30000;
+ // must exceed the SSL handshake deadline, so that a handshake still in progress is not abandoned here
+ private static final int CONNECT_TIMEOUT_MILLIS =
+ (int) TimeUnit.SECONDS.toMillis(Integer.getInteger("hyracks.ipc.connect.timeout.sec", 120));
private final IPCSystem system;
@@ -133,7 +136,7 @@
networkThread.selector.wakeup();
}
}
- if (handle.waitTillConnected()) {
+ if (handle.waitTillConnected(CONNECT_TIMEOUT_MILLIS)) {
return handle;
}
if (maxRetries < 0 || retries++ < maxRetries) {
diff --git a/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/impl/IPCHandle.java b/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/impl/IPCHandle.java
index cc0a852..54862d6 100644
--- a/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/impl/IPCHandle.java
+++ b/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/impl/IPCHandle.java
@@ -21,12 +21,17 @@
import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
import java.nio.channels.SelectionKey;
+import java.util.concurrent.TimeUnit;
import org.apache.hyracks.api.network.ISocketChannel;
import org.apache.hyracks.ipc.api.IIPCHandle;
import org.apache.hyracks.ipc.exceptions.IPCException;
+import org.apache.hyracks.util.annotations.AiProvenance;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
final class IPCHandle implements IIPCHandle {
+ private static final Logger LOGGER = LogManager.getLogger();
private static final int DEFAULT_BUFFER_SIZE = 1024 * 1024;
private final IPCSystem system;
@@ -131,14 +136,30 @@
notifyAll();
}
- synchronized boolean waitTillConnected() throws InterruptedException {
+ /**
+ * Waits for this handle to reach a terminal state, for at most {@code timeoutMillis}. On expiry the
+ * handle is marked {@link HandleState#CLOSED} so that it is treated as a failed connection and reaped
+ * by {@link IPCConnectionManager#unregisterHandle}; teardown of its channel remains with the network
+ * thread's existing close paths.
+ *
+ * @return true if the handle became connected, false if it was closed or the wait timed out
+ */
+ @AiProvenance(agent = AiProvenance.Agent.CLAUDE_OPUS_5, tool = AiProvenance.Tool.CLAUDE_CODE_UI, contributionKind = AiProvenance.ContributionKind.REFACTORED, notes = "bound the connect wait")
+ synchronized boolean waitTillConnected(long timeoutMillis) throws InterruptedException {
+ final long deadlineNanos = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(timeoutMillis);
while (true) {
switch (state) {
case INITIAL:
case CONNECT_SENT:
case CONNECT_RECEIVED:
- // TODO: need a reasonable timeout here
- wait();
+ final long remainingNanos = deadlineNanos - System.nanoTime();
+ if (remainingNanos <= 0) {
+ LOGGER.warn("timed out after {}ms waiting to connect to {} (state {})", timeoutMillis,
+ remoteAddress, state);
+ setState(HandleState.CLOSED);
+ return false;
+ }
+ TimeUnit.NANOSECONDS.timedWait(this, remainingNanos);
break;
case CONNECTED:
case CLOSED:
--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21547?usp=email
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: asterixdb
Gerrit-Branch: trinity
Gerrit-Change-Id: I1199e4e8b08eb6abf1649142c8efa7dd00600e27
Gerrit-Change-Number: 21547
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Blow <mb...@apache.org>