Skip to content

Commit 16520a7

Browse files
chernserchernser
committed
complete Connection#setNetworkTimeout() tests
1 parent 4a0540d commit 16520a7

File tree

4 files changed

+47
-28
lines changed

4 files changed

+47
-28
lines changed

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,6 @@ public class HttpAPIClientHelper {
110110

111111
private final CloseableHttpClient httpClient;
112112

113-
private final RequestConfig baseRequestConfig;
114-
115113
private String proxyAuthHeaderValue;
116114

117115
private final Set<ClientFaultCause> defaultRetryCauses;
@@ -125,11 +123,6 @@ public HttpAPIClientHelper(Map<String, Object> configuration, Object metricsRegi
125123
this.metricsRegistry = metricsRegistry;
126124
this.httpClient = createHttpClient(initSslContext, configuration);
127125

128-
RequestConfig.Builder reqConfBuilder = RequestConfig.custom();
129-
reqConfBuilder.setConnectionRequestTimeout(ClientConfigProperties.CONNECTION_REQUEST_TIMEOUT.getOrDefault(configuration), TimeUnit.MILLISECONDS);
130-
131-
this.baseRequestConfig = reqConfBuilder.build();
132-
133126
boolean usingClientCompression = ClientConfigProperties.COMPRESS_CLIENT_REQUEST.getOrDefault(configuration);
134127
boolean usingServerCompression = ClientConfigProperties.COMPRESS_SERVER_RESPONSE.getOrDefault(configuration);
135128
boolean useHttpCompression = ClientConfigProperties.USE_HTTP_COMPRESSION.getOrDefault(configuration);
@@ -438,12 +431,19 @@ public ClassicHttpResponse executeRequest(Endpoint server, Map<String, Object> r
438431
boolean useHttpCompression = ClientConfigProperties.USE_HTTP_COMPRESSION.getOrDefault(requestConfig);
439432
boolean appCompressedData = ClientConfigProperties.APP_COMPRESSED_DATA.getOrDefault(requestConfig);
440433

441-
req.setConfig(baseRequestConfig);
434+
442435
// setting entity. wrapping if compression is enabled
443436
req.setEntity(wrapRequestEntity(new EntityTemplate(-1, CONTENT_TYPE, null, writeCallback),
444437
clientCompression, useHttpCompression, appCompressedData, lz4Factory, requestConfig));
445438

446439
HttpClientContext context = HttpClientContext.create();
440+
Number responseTimeout = ClientConfigProperties.SOCKET_OPERATION_TIMEOUT.getOrDefault(requestConfig);
441+
Number connectionReqTimeout = ClientConfigProperties.CONNECTION_REQUEST_TIMEOUT.getOrDefault(requestConfig);
442+
RequestConfig reqHttpConf = RequestConfig.custom()
443+
.setResponseTimeout(responseTimeout.longValue(), TimeUnit.MILLISECONDS)
444+
.setConnectionRequestTimeout(connectionReqTimeout.longValue(), TimeUnit.MILLISECONDS)
445+
.build();
446+
context.setRequestConfig(reqHttpConf);
447447

448448
ClassicHttpResponse httpResponse = null;
449449
try {

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,12 +175,19 @@ protected ResultSetImpl executeQueryImpl(String sql, QuerySettings settings) thr
175175
LOG.warn("Failed to close response after exception", e);
176176
}
177177
}
178+
handleSocketTimeoutException(e);
178179
onResultSetClosed(null);
180+
throw ExceptionUtils.toSqlState(e);
181+
}
182+
}
179183

180-
if (e instanceof SocketTimeoutException) {
184+
protected void handleSocketTimeoutException(Exception e) {
185+
if (e.getCause() instanceof SocketTimeoutException || e instanceof SocketTimeoutException) {
186+
try {
181187
this.connection.onNetworkTimeout();
188+
} catch (SQLException e1) {
189+
LOG.warn("Failed to handle network timeout exception", e1);
182190
}
183-
throw ExceptionUtils.toSqlState(e);
184191
}
185192
}
186193

@@ -215,9 +222,7 @@ protected long executeUpdateImpl(String sql, QuerySettings settings) throws SQLE
215222
updateCount = Math.max(0, (int) response.getWrittenRows()); // when statement alters schema no result rows returned.
216223
lastQueryId = response.getQueryId();
217224
} catch (Exception e) {
218-
if (e instanceof SocketTimeoutException) {
219-
this.connection.onNetworkTimeout();
220-
}
225+
handleSocketTimeoutException(e);
221226
throw ExceptionUtils.toSqlState(e);
222227
}
223228

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

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import java.io.InputStream;
1717
import java.io.Reader;
1818
import java.math.BigDecimal;
19-
import java.net.SocketTimeoutException;
2019
import java.net.URL;
2120
import java.sql.Array;
2221
import java.sql.Blob;
@@ -110,9 +109,7 @@ public long executeLargeUpdate() throws SQLException {
110109
try {
111110
writer.commitRow();
112111
} catch (Exception e) {
113-
if (e instanceof SocketTimeoutException) {
114-
this.connection.onNetworkTimeout();
115-
}
112+
handleSocketTimeoutException(e);
116113
throw new SQLException(e);
117114
}
118115

@@ -125,9 +122,7 @@ public long executeLargeUpdate() throws SQLException {
125122
updateCount = Math.max(0, (int) response.getWrittenRows()); // when statement alters schema no result rows returned.
126123
lastQueryId = response.getQueryId();
127124
} catch (Exception e) {
128-
if (e instanceof SocketTimeoutException) {
129-
this.connection.onNetworkTimeout();
130-
}
125+
handleSocketTimeoutException(e);
131126
throw ExceptionUtils.toSqlState(e);
132127
} finally {
133128
try {
@@ -305,9 +300,7 @@ public void addBatch() throws SQLException {
305300
try {
306301
writer.commitRow();
307302
} catch (Exception e) {
308-
if (e instanceof SocketTimeoutException) {
309-
this.connection.onNetworkTimeout();
310-
}
303+
handleSocketTimeoutException(e);
311304
throw new SQLException(e);
312305
}
313306
}

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

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.Base64;
2929
import java.util.Properties;
3030
import java.util.UUID;
31+
import java.util.concurrent.ExecutorService;
3132
import java.util.concurrent.Executors;
3233
import java.util.concurrent.TimeUnit;
3334

@@ -396,13 +397,33 @@ public void abortTest() throws SQLException {
396397
}
397398
}
398399

399-
@Test(groups = { "integration" })
400+
@Test(groups = {"integration"})
400401
public void testNetworkTimeout() throws SQLException {
402+
try (Connection conn = this.getJdbcConnection()) {
403+
Assert.assertThrows(SQLException.class, () -> conn.setNetworkTimeout(null, 1000));
404+
Assert.assertThrows(SQLException.class, () -> conn.setNetworkTimeout(Executors.newSingleThreadExecutor(), -1));
405+
406+
int timeout = 10;
407+
ExecutorService executorService = Executors.newSingleThreadExecutor();
408+
conn.setNetworkTimeout(executorService, timeout);
409+
Assert.assertEquals(conn.getNetworkTimeout(), timeout);
410+
Statement stmt = conn.createStatement();
411+
try {
412+
ResultSet rs = stmt.executeQuery("SELECT sleepEachRow(1) FROM system.numbers LIMIT 2");
413+
fail("Exception expected");
414+
} catch (Exception e) {
415+
Assert.assertTrue(conn.isClosed());
416+
Assert.assertFalse(conn.isValid(1000));
417+
conn.close();
418+
}
419+
420+
try {
421+
stmt.executeQuery("SELECT 1");
422+
} catch (SQLException e) {
423+
Assert.assertTrue(e.getMessage().contains("closed"));
424+
}
425+
}
401426
try {
402-
Connection conn = this.getJdbcConnection();
403-
int t1 = (int) TimeUnit.SECONDS.toMillis(20);
404-
conn.setNetworkTimeout(Executors.newSingleThreadExecutor(), t1);
405-
Assert.assertEquals(t1, conn.getNetworkTimeout());
406427

407428
} catch (Exception e) {
408429

0 commit comments

Comments
 (0)