You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "timoninmaxim (via GitHub)" <gi...@apache.org> on 2026/02/09 08:58:50 UTC
[PR] IGNITE-20973 Add connection timeout configuration for java thin client [ignite]
timoninmaxim opened a new pull request, #12710:
URL: https://github.com/apache/ignite/pull/12710
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
### The Contribution Checklist
- [ ] There is a single JIRA ticket related to the pull request.
- [ ] The web-link to the pull request is attached to the JIRA ticket.
- [ ] The JIRA ticket has the _Patch Available_ state.
- [ ] The pull request body describes changes that have been made.
The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
- [ ] The pull request title is treated as the final commit message.
The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
- [ ] A reviewer has been mentioned through the JIRA comments
(see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers))
- [ ] The pull request has been checked by the Teamcity Bot and
the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
### Notes
- [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
- [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
- [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
- [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20973 Add handshake timeout configuration for java thin client [ignite]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12710:
URL: https://github.com/apache/ignite/pull/12710#discussion_r2787314417
##########
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java:
##########
@@ -57,8 +58,11 @@ public final class ClientConfiguration implements Serializable {
/** @serial Tcp no delay. */
private boolean tcpNoDelay = true;
- /** @serial Timeout. 0 means infinite. */
- private int timeout;
+ /** @serial Handshake timeout in milliseconds. 0 means infinite. */
+ private int handshakeTimeout;
+
+ /** @serial Request timeout in milliseconds. 0 means infinite. */
+ private int reqTimeout;
Review Comment:
ClientConfiguration is Serializable with a fixed serialVersionUID (0L), but the legacy `timeout` field has been removed and replaced with `handshakeTimeout`/`reqTimeout`. Deserializing previously-serialized ClientConfiguration instances will now silently lose the timeout value (new fields default to 0). Consider preserving serialization compatibility by keeping a deprecated `timeout` field and/or implementing custom deserialization (e.g., mapping legacy `timeout` to both new fields when present).
##########
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java:
##########
@@ -227,18 +231,65 @@ public ClientConfiguration setTcpNoDelay(boolean tcpNoDelay) {
}
/**
- * @return Send/receive timeout in milliseconds.
+ * @deprecated Use {@link #getHandshakeTimeout()} and {@link #getRequestTimeout()} instead.
+ * @return Request timeout in milliseconds.
*/
+ @Deprecated
public int getTimeout() {
- return timeout;
+ if (reqTimeout != handshakeTimeout) {
+ U.warn(logger, String.format(
+ "Deprecated getTimeout() API is used while request timeout (%d) differs from handshake timeout (%d). " +
+ "Returning request timeout. Please use getRequestTimeout() and getHandshakeTimeout() instead.",
+ reqTimeout, handshakeTimeout
+ ));
+ }
Review Comment:
`getTimeout()` now logs a warning whenever request/handshake timeouts differ. Since this is a getter, callers may invoke it frequently (including via legacy code paths), which can spam logs. Consider logging this warning only once (e.g., via a static/instance guard) or moving the warning to the setters when a divergence is introduced.
##########
modules/core/src/test/java/org/apache/ignite/internal/client/thin/TimeoutTest.java:
##########
@@ -217,4 +218,103 @@ public void testClientTimeoutOnOperation() throws Exception {
}
}
}
+
+ /**
+ * Test that connection timeout is independent of request timeout during connection establishment.
+ */
+ @Test
+ @SuppressWarnings("ThrowableNotThrown")
+ public void testConnectionTimeoutIndependentOfRequest() throws Exception {
+ ServerSocket sock = new ServerSocket();
+ sock.bind(new InetSocketAddress("127.0.0.1", DFLT_PORT));
+
+ IgniteInternalFuture<?> fut = GridTestUtils.runAsync(() -> {
+ Socket accepted = null;
+
+ try {
+ accepted = sock.accept();
+
+ while (!Thread.currentThread().isInterrupted())
+ Thread.sleep(10);
+ }
+ finally {
+ U.closeQuiet(accepted);
+ }
+ });
+
+ try {
+ ClientConfiguration cfg = new ClientConfiguration()
+ .setAddresses("127.0.0.1:" + DFLT_PORT)
+ .setHandshakeTimeout(500)
+ .setRequestTimeout(Integer.MAX_VALUE);
+
+ GridTestUtils.assertThrowsWithCause(
+ () -> Ignition.startClient(cfg),
+ IgniteFutureTimeoutCheckedException.class
+ );
+
+ fut.cancel();
+ }
+ finally {
Review Comment:
In `testConnectionTimeoutIndependentOfRequest`, the accept thread loops until interrupted, but `fut.cancel()` is only called after the assertion succeeds. If `assertThrowsWithCause` fails (or throws unexpectedly), the test can leave a non-terminating background thread running. Move `fut.cancel()` (and ideally `fut.get()`/await) into the `finally` block to guarantee cleanup even on assertion failures.
```suggestion
}
finally {
fut.cancel();
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20973 Add handshake timeout configuration for java thin client [ignite]
Posted by "timoninmaxim (via GitHub)" <gi...@apache.org>.
timoninmaxim commented on code in PR #12710:
URL: https://github.com/apache/ignite/pull/12710#discussion_r2783586277
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpClientChannel.java:
##########
@@ -736,7 +740,7 @@ private void handshake(ProtocolVersion ver, String user, String pwd, Map<String,
handshakeReq(ver, user, pwd, userAttrs);
try {
- ByteBuffer buf = timeout > 0 ? fut.get(timeout) : fut.get();
+ ByteBuffer buf = connTimeout > 0 ? fut.get(connTimeout) : fut.get();
Review Comment:
Actually, there is a misleading behavior. We call `eventListener.onHandshakeStart(ver)` before the loop, but may retry handshake with different `ver`. At the same time we log "Handshake failed" for each attempt.
I suppose that we should move `eventListener.onHandshakeStart` within the loop, to fix the `ver` it notifies. In this context `handshakeTimeout` is OK.
WDYT about this? Should we investigate moving of this call within this ticket?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20973 Add handshake timeout configuration for java thin client [ignite]
Posted by "timoninmaxim (via GitHub)" <gi...@apache.org>.
timoninmaxim merged PR #12710:
URL: https://github.com/apache/ignite/pull/12710
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20973 Add handshake timeout configuration for java thin client [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12710:
URL: https://github.com/apache/ignite/pull/12710#issuecomment-3870401014
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12710) **Quality Gate failed**
Failed conditions
 [8 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12710) (required ≤ 1)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12710)
##
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20973 Add handshake timeout configuration for java thin client [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12710:
URL: https://github.com/apache/ignite/pull/12710#issuecomment-3878315445
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12710) **Quality Gate failed**
Failed conditions
 [5 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12710) (required ≤ 1)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12710)
##
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20973 Add handshake timeout configuration for java thin client [ignite]
Posted by "alex-plekhanov (via GitHub)" <gi...@apache.org>.
alex-plekhanov commented on code in PR #12710:
URL: https://github.com/apache/ignite/pull/12710#discussion_r2786791544
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpClientChannel.java:
##########
@@ -736,7 +740,7 @@ private void handshake(ProtocolVersion ver, String user, String pwd, Map<String,
handshakeReq(ver, user, pwd, userAttrs);
try {
- ByteBuffer buf = timeout > 0 ? fut.get(timeout) : fut.get();
+ ByteBuffer buf = connTimeout > 0 ? fut.get(connTimeout) : fut.get();
Review Comment:
I think it's ok to move this call within this ticket.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20973 Add handshake timeout configuration for java thin client [ignite]
Posted by "alex-plekhanov (via GitHub)" <gi...@apache.org>.
alex-plekhanov commented on code in PR #12710:
URL: https://github.com/apache/ignite/pull/12710#discussion_r2783239752
##########
modules/core/src/test/java/org/apache/ignite/internal/client/thin/TimeoutTest.java:
##########
@@ -217,4 +218,104 @@ public void testClientTimeoutOnOperation() throws Exception {
}
}
}
+
+ /**
+ * Test that connection timeout is independent of request timeout during connection establishment.
+ */
+ @Test
+ @SuppressWarnings("ThrowableNotThrown")
+ public void testConnectionTimeoutIndependentOfRequest() throws Exception {
+ ServerSocket sock = new ServerSocket();
+ sock.bind(new InetSocketAddress("127.0.0.1", DFLT_PORT));
+
+ IgniteInternalFuture<?> fut = GridTestUtils.runAsync(() -> {
+ Socket accepted = null;
+
+ try {
+ accepted = sock.accept();
+
+ while (!Thread.currentThread().isInterrupted())
+ Thread.sleep(10);
+ }
+ finally {
+ if (accepted == null)
Review Comment:
`accepted != null`?
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpClientChannel.java:
##########
@@ -736,7 +740,7 @@ private void handshake(ProtocolVersion ver, String user, String pwd, Map<String,
handshakeReq(ver, user, pwd, userAttrs);
try {
- ByteBuffer buf = timeout > 0 ? fut.get(timeout) : fut.get();
+ ByteBuffer buf = connTimeout > 0 ? fut.get(connTimeout) : fut.get();
Review Comment:
We can wait here more time than handshake timeout (wait for connection established for handshakeTimeout time, wait for each handshake request for hanshakeTimeout time, there can be more than one hanshake request if client and server versions are not equal). Is it expected?
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ClientChannelConfiguration.java:
##########
@@ -172,10 +176,26 @@ public boolean isTcpNoDelay() {
}
/**
- * @return Timeout.
+ * @deprecated Use {@link #getHandshakeTimeout()} and {@link #getRequestTimeout()} instead.
+ * @return Request timeout.
*/
+ @Deprecated
Review Comment:
Why do we still need this method? It's in internal API and can be deleted IMO.
##########
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java:
##########
@@ -227,19 +231,63 @@ public ClientConfiguration setTcpNoDelay(boolean tcpNoDelay) {
}
/**
- * @return Send/receive timeout in milliseconds.
+ * @deprecated Use {@link #getHandshakeTimeout()} and {@link #getRequestTimeout()} instead.
+ * @return Request timeout in milliseconds.
*/
+ @Deprecated
public int getTimeout() {
- return timeout;
+ if (reqTimeout != handshakeTimeout) {
+ U.warn(logger, String.format(
+ "Deprecated getTimeout() API is used while request timeout (%d) differs from handshake timeout (%d). " +
+ "Returning request timeout. Please use getRequestTimeout() and getHandshakeTimeout() instead.",
+ reqTimeout, handshakeTimeout
+ ));
+ }
+
+ return reqTimeout;
}
/**
+ * @deprecated Use {@link #setHandshakeTimeout(int)} and {@link #setRequestTimeout(int)} instead.
* @param timeout Send/receive timeout in milliseconds.
* @return {@code this} for chaining.
*/
+ @Deprecated
public ClientConfiguration setTimeout(int timeout) {
- this.timeout = timeout;
+ this.handshakeTimeout = timeout;
+ this.reqTimeout = timeout;
Review Comment:
Maybe NL after this line? (for other setters too)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20973 Add handshake timeout configuration for java thin client [ignite]
Posted by "timoninmaxim (via GitHub)" <gi...@apache.org>.
timoninmaxim commented on code in PR #12710:
URL: https://github.com/apache/ignite/pull/12710#discussion_r2787494530
##########
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java:
##########
@@ -227,18 +231,65 @@ public ClientConfiguration setTcpNoDelay(boolean tcpNoDelay) {
}
/**
- * @return Send/receive timeout in milliseconds.
+ * @deprecated Use {@link #getHandshakeTimeout()} and {@link #getRequestTimeout()} instead.
+ * @return Request timeout in milliseconds.
*/
+ @Deprecated
public int getTimeout() {
- return timeout;
+ if (reqTimeout != handshakeTimeout) {
+ U.warn(logger, String.format(
+ "Deprecated getTimeout() API is used while request timeout (%d) differs from handshake timeout (%d). " +
+ "Returning request timeout. Please use getRequestTimeout() and getHandshakeTimeout() instead.",
+ reqTimeout, handshakeTimeout
+ ));
+ }
Review Comment:
@ptupitsyn yes, I'm fixing it. Thanks for calling the agent :)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20973 Add connection timeout configuration for java thin client [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12710:
URL: https://github.com/apache/ignite/pull/12710#issuecomment-3870325328
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12710) **Quality Gate failed**
Failed conditions
 [8 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12710) (required ≤ 1)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12710)
##
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20973 Add handshake timeout configuration for java thin client [ignite]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #12710:
URL: https://github.com/apache/ignite/pull/12710#discussion_r2787470828
##########
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java:
##########
@@ -227,18 +231,65 @@ public ClientConfiguration setTcpNoDelay(boolean tcpNoDelay) {
}
/**
- * @return Send/receive timeout in milliseconds.
+ * @deprecated Use {@link #getHandshakeTimeout()} and {@link #getRequestTimeout()} instead.
+ * @return Request timeout in milliseconds.
*/
+ @Deprecated
public int getTimeout() {
- return timeout;
+ if (reqTimeout != handshakeTimeout) {
+ U.warn(logger, String.format(
+ "Deprecated getTimeout() API is used while request timeout (%d) differs from handshake timeout (%d). " +
+ "Returning request timeout. Please use getRequestTimeout() and getHandshakeTimeout() instead.",
+ reqTimeout, handshakeTimeout
+ ));
+ }
Review Comment:
@timoninmaxim looks like a valid concern.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-20973 Add handshake timeout configuration for java thin client [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12710:
URL: https://github.com/apache/ignite/pull/12710#issuecomment-3877203058
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12710) **Quality Gate failed**
Failed conditions
 [5 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12710) (required ≤ 1)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12710)
##
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org