Skip to content

Commit 2c8240c

Browse files
blayeclaude
andcommitted
[client-v2, jdbc-v2] Make 159 TIMEOUT_EXCEEDED non-retryable and honour setQueryTimeout
Ports the 0.9.9 patch (#3073) to main. The patch was merged into the v0.9.9 maintenance branch only, so 0.10.0 and main carry the original behaviour. - client-v2: `ServerException` no longer reports code 159 TIMEOUT_EXCEEDED as retryable. The server raises it once the query has already consumed its whole `max_execution_time` budget, so an automatic retry spends that budget again. - jdbc-v2: `Statement#setQueryTimeout` is applied as the `max_execution_time` server setting when asynchronous operations are disabled, because the query then runs in the calling thread and a future timeout cannot interrupt it. A negative value is rejected; zero clears the setting. - jdbc-v2: an execution timeout is reported as `SQLTimeoutException`. Two deliberate differences from the 0.9.9 patch: - `SQLTimeoutException` carries the ClickHouse error code as its vendor code, matching what `ExceptionUtils.toSqlState` already does for `ServerException`. The 0.9.9 patch used the `(String, Throwable)` constructor, which leaves `getErrorCode()` at 0, so callers that classify on the vendor code had to walk the cause chain. - The `ConnectionImpl` hunk and the `connectionLvlExecTimeout` field are left out. They exist to restore a connection-level `max_execution_time` on `setQueryTimeout(0)`; on main the same result comes from resetting the option and falling back to the client configuration. The part of that hunk that makes the documented `default_query_settings` property take effect is a separate bug and belongs in its own change. `HttpAPIClientHelperTest.serverExceptionRetryCases` used 159 as its example of a retryable code. The test covers `shouldRetry` reading the `ServerException` from the cause instead of throwing `ClassCastException`, so it now uses 209 SOCKET_TIMEOUT and its intent is unchanged. The unrelated backports carried by #3073 (ClickHouseSqlUtils keywords, DatabaseMetaDataImpl engine map, test stabilisation) are already on main. Closes #3136 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent ae682fc commit 2c8240c

7 files changed

Lines changed: 167 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,16 @@
171171

172172
### Bug Fixes
173173

174+
- **[client-v2]** `ServerException` with code `159 TIMEOUT_EXCEEDED` is no longer reported as retryable. The server
175+
raises it once the query has already consumed its whole `max_execution_time` budget, so every automatic retry spends
176+
that budget again and multiplies the load that caused the timeout. The fix was released in `0.9.9` but never reached
177+
`main`, so `0.10.0` shipped the original behaviour.
178+
(https://github.com/ClickHouse/clickhouse-java/issues/3136)
179+
- **[jdbc-v2]** Fixed `Statement#setQueryTimeout` being ignored. The client executes a query in the calling thread
180+
unless asynchronous operations are enabled, and the future timeout then has no effect, so the value is applied as the
181+
`max_execution_time` server setting instead. An execution timeout is reported as `SQLTimeoutException` carrying the
182+
server error code as its vendor code. The fix was released in `0.9.9` but never reached `main`, so `0.10.0` shipped
183+
the original behaviour. (https://github.com/ClickHouse/clickhouse-java/issues/3136)
174184
- **[jdbc-v2]** Added the non-reserved keywords `AGGREGATE`, `BOUNDED`, `EXTEND`, `HANDLER`, `IDLE`, `PROTOCOL`,
175185
`RECENT`, `TIMEOUT` and `UNORDERED` (ClickHouse `26.8+`; `IDLE`, `TIMEOUT` and `RECENT` come from the multi-word
176186
keywords `IDLE TIMEOUT` and `RECENT SAMPLES`) to the list of keywords allowed in identifier positions. The server

‎client-v2/src/main/java/com/clickhouse/client/api/ServerException.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ public class ServerException extends ClickHouseException {
88

99
public static final int UNKNOWN_SETTING = 115;
1010

11+
public static final int EXECUTION_TIMEOUT = 159;
12+
1113
private final int code;
1214

1315
private final int transportProtocolCode;
@@ -56,10 +58,9 @@ public String getQueryId() {
5658
private boolean discoverIsRetryable(int code, String message, int transportProtocolCode) {
5759
// Let's check if we have a ServerException to reference the error code
5860
// https://github.com/ClickHouse/ClickHouse/blob/master/src/Common/ErrorCodes.cpp
59-
switch (code) { // UNEXPECTED_END_OF_FILE
61+
switch (code) {
6062
case 3: // UNEXPECTED_END_OF_FILE
6163
case 107: // FILE_DOESNT_EXIST
62-
case 159: // TIMEOUT_EXCEEDED
6364
case 164: // READONLY
6465
case 202: // TOO_MANY_SIMULTANEOUS_QUERIES
6566
case 203: // NO_FREE_CONNECTION
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package com.clickhouse.client.api;
2+
3+
import org.testng.Assert;
4+
import org.testng.annotations.DataProvider;
5+
import org.testng.annotations.Test;
6+
7+
public class ServerExceptionTest {
8+
9+
@DataProvider(name = "retryableCodes")
10+
public static Object[][] retryableCodes() {
11+
return new Object[][] {
12+
// Execution timeout means the server already spent the whole time budget on the query,
13+
// so retrying it can only spend it again.
14+
{159, "TIMEOUT_EXCEEDED", false},
15+
{60, "UNKNOWN_TABLE", false},
16+
{62, "SYNTAX_ERROR", false},
17+
{241, "MEMORY_LIMIT_EXCEEDED", true},
18+
{209, "SOCKET_TIMEOUT", true},
19+
{210, "NETWORK_ERROR", true},
20+
{999, "KEEPER_EXCEPTION", true},
21+
};
22+
}
23+
24+
@Test(groups = {"unit"}, dataProvider = "retryableCodes")
25+
public void testIsRetryable(int code, String codeName, boolean expected) {
26+
ServerException exception = new ServerException(code, "DB::Exception: " + codeName, 500, "query-id");
27+
28+
Assert.assertEquals(exception.isRetryable(), expected,
29+
"Unexpected retryability for code " + code + " (" + codeName + ")");
30+
Assert.assertEquals(exception.getCode(), code);
31+
}
32+
}

‎client-v2/src/test/java/com/clickhouse/client/api/internal/HttpAPIClientHelperTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,8 +341,8 @@ public void testExecuteRequestThrowsConnectExceptionOn503() throws Exception {
341341

342342
@DataProvider(name = "serverExceptionRetryCases")
343343
public static Object[][] serverExceptionRetryCases() {
344-
// Server code 159 (TIMEOUT_EXCEEDED) is retryable; code 60 (TABLE_NOT_FOUND) is not.
345-
ServerException retryable = new ServerException(159, "TIMEOUT_EXCEEDED", 500, "q1");
344+
// Server code 209 (SOCKET_TIMEOUT) is retryable; code 60 (TABLE_NOT_FOUND) is not.
345+
ServerException retryable = new ServerException(209, "SOCKET_TIMEOUT", 500, "q1");
346346
ServerException nonRetryable = new ServerException(60, "TABLE_NOT_FOUND", 404, "q2");
347347
return new Object[][]{
348348
// ServerException thrown directly (behaviour that already worked; pinned as contrast).

‎docs/features.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ Compatibility-sensitive traits:
7979
- Schema and database context: Supports database selection through URL, `setSchema`, `USE`, and statement-level settings.
8080
- Non-transactional operation: Exposes ClickHouse-appropriate transaction behavior with auto-commit semantics and unsupported transactional features.
8181
- Statement execution: Supports `execute`, `executeQuery`, `executeUpdate`, large update counts, and forward-only/read-only statements.
82-
- Query cancellation and timeout: Supports JDBC query timeout handling and query cancellation through server-side `KILL QUERY`, with optional JDBC `cluster_name` property support to add `ON CLUSTER '<name>'` for cluster-wide cancellation.
82+
- Query cancellation and timeout: Supports JDBC query timeout handling and query cancellation through server-side `KILL QUERY`, with optional JDBC `cluster_name` property support to add `ON CLUSTER '<name>'` for cluster-wide cancellation. `Statement#setQueryTimeout` is applied as the `max_execution_time` server setting, because without asynchronous operations the query runs in the calling thread and a client-side future timeout cannot interrupt it. Exceeding the timeout raises `SQLTimeoutException`, whose vendor code is the ClickHouse error code when the server reported the timeout.
8383
- Batch execution: Supports batched statements and prepared-statement batches, including multi-row rewrite for eligible `INSERT ... VALUES` statements.
8484
- Prepared statements: Supports `?` parameters through client-side SQL rendering and validates that all parameters are bound before execution.
8585
- SQL parsing and classification: Classifies SQL to distinguish queries, updates, inserts, `USE`, and role-changing statements, with selectable parser backends.

‎jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.clickhouse.jdbc;
22

33
import com.clickhouse.client.api.ClientConfigProperties;
4+
import com.clickhouse.client.api.ServerException;
45
import com.clickhouse.client.api.data_formats.ClickHouseFormatReader;
56
import com.clickhouse.client.api.data_formats.JSONEachRowFormatReader;
67
import com.clickhouse.client.api.internal.ServerSettings;
@@ -17,6 +18,7 @@
1718
import java.net.SocketTimeoutException;
1819
import java.sql.ResultSet;
1920
import java.sql.SQLException;
21+
import java.sql.SQLTimeoutException;
2022
import java.sql.SQLWarning;
2123
import java.sql.Statement;
2224
import java.util.ArrayDeque;
@@ -26,6 +28,7 @@
2628
import java.util.UUID;
2729
import java.util.concurrent.ConcurrentLinkedQueue;
2830
import java.util.concurrent.TimeUnit;
31+
import java.util.concurrent.TimeoutException;
2932
import java.util.function.Supplier;
3033
import java.util.regex.Matcher;
3134
import java.util.regex.Pattern;
@@ -360,6 +363,7 @@ protected ResultSetImpl executeQueryImpl(String sql, QuerySettings settings) thr
360363
}
361364
handleSocketTimeoutException(e);
362365
onResultSetClosed(null);
366+
throwOnExecutionTimeout(e, mergedSettings.getQueryId());
363367
throw ExceptionUtils.toSqlState(e);
364368
}
365369
}
@@ -370,6 +374,29 @@ protected void handleSocketTimeoutException(Exception e) {
370374
}
371375
}
372376

377+
/**
378+
* Translates an execution timeout into {@link SQLTimeoutException}, which the JDBC spec requires when a
379+
* statement exceeds the limit set by {@link #setQueryTimeout(int)}. A timeout is reported either by the
380+
* client, when the query is awaited with a timeout, or by the server as error code
381+
* {@link ServerException#EXECUTION_TIMEOUT}. The server error code is carried over as the vendor code so
382+
* callers can classify the failure without unwrapping the cause chain.
383+
*
384+
* @param e exception thrown by the query execution
385+
* @param queryId id of the query that failed
386+
*/
387+
protected void throwOnExecutionTimeout(Exception e, String queryId) throws SQLTimeoutException {
388+
ServerException serverException = e instanceof ServerException ? (ServerException) e
389+
: e.getCause() instanceof ServerException ? (ServerException) e.getCause() : null;
390+
boolean isTimeout = e instanceof TimeoutException || e.getCause() instanceof TimeoutException
391+
|| (serverException != null && serverException.getCode() == ServerException.EXECUTION_TIMEOUT);
392+
393+
if (isTimeout) {
394+
throw new SQLTimeoutException("Query execution time exceeded limit (queryId=" + queryId + ")",
395+
ExceptionUtils.SQL_STATE_OPERATION_CANCELLED,
396+
serverException == null ? ServerException.CODE_UNKNOWN : serverException.getCode(), e);
397+
}
398+
}
399+
373400
@Override
374401
public int executeUpdate(String sql) throws SQLException {
375402
ensureOpen();
@@ -396,6 +423,7 @@ protected long executeUpdateImpl(String sql, QuerySettings settings) throws SQLE
396423
lastQueryId = response.getQueryId();
397424
} catch (Exception e) {
398425
handleSocketTimeoutException(e);
426+
throwOnExecutionTimeout(e, mergedSettings.getQueryId());
399427
throw ExceptionUtils.toSqlState(e);
400428
}
401429

@@ -469,9 +497,33 @@ public int getQueryTimeout() throws SQLException {
469497
@Override
470498
public void setQueryTimeout(int seconds) throws SQLException {
471499
ensureOpen();
500+
if (seconds < 0) {
501+
throw new SQLException("Timeout should be >= 0 but " + seconds + " was passed");
502+
}
503+
504+
if (seconds > 0) {
505+
// With asynchronous operations the query is awaited with a timeout, which bounds the call on its own.
506+
// Otherwise it runs in the calling thread and `max_execution_time` is the only way to bound it.
507+
if (!isAsyncOperationsEnabled()) {
508+
getLocalSettings().setMaxExecutionTime(seconds);
509+
}
510+
} else {
511+
getLocalSettings().resetOption(ClientConfigProperties.serverSetting(ServerSettings.MAX_EXECUTION_TIME));
512+
}
472513
queryTimeout = seconds;
473514
}
474515

516+
private boolean isAsyncOperationsEnabled() {
517+
try {
518+
return Boolean.parseBoolean(getConnection().getClient().getConfiguration()
519+
.getOrDefault(ClientConfigProperties.ASYNC_OPERATIONS.getKey(),
520+
ClientConfigProperties.ASYNC_OPERATIONS.getDefaultValue()));
521+
} catch (Exception e) {
522+
LOG.error("Failed to read client configuration " + ClientConfigProperties.ASYNC_OPERATIONS.getKey(), e);
523+
return false;
524+
}
525+
}
526+
475527
@Override
476528
public void cancel() throws SQLException {
477529
if (closed) {

‎jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.clickhouse.jdbc;
22

33
import com.clickhouse.client.api.ClientConfigProperties;
4+
import com.clickhouse.client.api.ServerException;
45
import com.clickhouse.client.api.Session;
56
import com.clickhouse.client.api.data_formats.GsonJsonParserFactory;
67
import com.clickhouse.client.api.data_formats.JacksonJsonParserFactory;
@@ -24,6 +25,7 @@
2425
import java.sql.ResultSet;
2526
import java.sql.ResultSetMetaData;
2627
import java.sql.SQLException;
28+
import java.sql.SQLTimeoutException;
2729
import java.sql.Statement;
2830
import java.time.LocalDate;
2931
import java.util.Arrays;
@@ -44,13 +46,18 @@
4446
import static org.testng.Assert.assertSame;
4547
import static org.testng.Assert.assertThrows;
4648
import static org.testng.Assert.assertTrue;
49+
import static org.testng.Assert.expectThrows;
4750
import static org.testng.Assert.fail;
4851

4952

5053
@Test(groups = {"integration"})
5154
public class StatementTest extends JdbcIntegrationTest {
5255
private static final Logger log = LoggerFactory.getLogger(StatementTest.class);
5356

57+
/** Runs long enough on a single thread for a low `max_execution_time` to interrupt it. */
58+
private static final String SLOW_QUERY =
59+
"SELECT count(), sum(sipHash64(number)) FROM numbers(1000000000) SETTINGS max_threads = 1";
60+
5461
@Test(groups = {"integration"})
5562
public void testExecuteQuerySimpleNumbers() throws Exception {
5663
try (Connection conn = getJdbcConnection()) {
@@ -1763,6 +1770,66 @@ public void testEscapedSQLToNative(String sql, String expected) {
17631770
assertEquals(StatementImpl.escapedSQLToNative(sql), expected);
17641771
}
17651772

1773+
@Test(groups = {"integration"})
1774+
public void testSetQueryTimeoutRejectsNegativeValue() throws Exception {
1775+
try (Connection conn = getJdbcConnection();
1776+
Statement stmt = conn.createStatement()) {
1777+
assertThrows(SQLException.class, () -> stmt.setQueryTimeout(-1));
1778+
assertEquals(stmt.getQueryTimeout(), 0);
1779+
}
1780+
}
1781+
1782+
@Test(groups = {"integration"})
1783+
public void testSetQueryTimeoutSetsAndResetsMaxExecutionTime() throws Exception {
1784+
try (Connection conn = getJdbcConnection();
1785+
StatementImpl stmt = (StatementImpl) conn.createStatement()) {
1786+
assertNull(stmt.getLocalSettings().getMaxExecutionTime());
1787+
1788+
stmt.setQueryTimeout(5);
1789+
assertEquals(stmt.getQueryTimeout(), 5);
1790+
assertEquals(stmt.getLocalSettings().getMaxExecutionTime(), Integer.valueOf(5));
1791+
1792+
stmt.setQueryTimeout(0);
1793+
assertEquals(stmt.getQueryTimeout(), 0);
1794+
assertNull(stmt.getLocalSettings().getMaxExecutionTime());
1795+
}
1796+
}
1797+
1798+
@Test(groups = {"integration"})
1799+
public void testSetQueryTimeoutLeavesMaxExecutionTimeUnsetForAsyncOperations() throws Exception {
1800+
Properties config = new Properties();
1801+
config.setProperty(ClientConfigProperties.ASYNC_OPERATIONS.getKey(), "true");
1802+
try (Connection conn = getJdbcConnection(config);
1803+
StatementImpl stmt = (StatementImpl) conn.createStatement()) {
1804+
stmt.setQueryTimeout(5);
1805+
1806+
assertEquals(stmt.getQueryTimeout(), 5);
1807+
assertNull(stmt.getLocalSettings().getMaxExecutionTime());
1808+
}
1809+
}
1810+
1811+
@Test(groups = {"integration"})
1812+
public void testServerExecutionTimeoutIsReportedAsSqlTimeoutException() throws Exception {
1813+
try (Connection conn = getJdbcConnection();
1814+
Statement stmt = conn.createStatement()) {
1815+
stmt.setQueryTimeout(1);
1816+
1817+
SQLTimeoutException e = expectThrows(SQLTimeoutException.class, () -> stmt.executeQuery(SLOW_QUERY));
1818+
assertEquals(e.getErrorCode(), ServerException.EXECUTION_TIMEOUT);
1819+
}
1820+
}
1821+
1822+
@Test(groups = {"integration"})
1823+
public void testServerExecutionTimeoutIsReportedAsSqlTimeoutExceptionOnUpdate() throws Exception {
1824+
try (Connection conn = getJdbcConnection();
1825+
Statement stmt = conn.createStatement()) {
1826+
stmt.setQueryTimeout(1);
1827+
1828+
SQLTimeoutException e = expectThrows(SQLTimeoutException.class, () -> stmt.executeUpdate(SLOW_QUERY));
1829+
assertEquals(e.getErrorCode(), ServerException.EXECUTION_TIMEOUT);
1830+
}
1831+
}
1832+
17661833
private static String getDBName(Statement stmt) throws SQLException {
17671834
try (ResultSet rs = stmt.executeQuery("SELECT database()")) {
17681835
rs.next();

0 commit comments

Comments
 (0)