Skip to content

Commit 41f3e34

Browse files
zhicwuzhicwu
authored
Merge pull request #540 from zhicwu/failed-to-respond
Retry for NoHttpResponseException
2 parents f6a0d54 + f90acb0 commit 41f3e34

File tree

4 files changed

+103
-29
lines changed

4 files changed

+103
-29
lines changed

‎src/main/java/ru/yandex/clickhouse/settings/ClickHouseConnectionSettings.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ public enum ClickHouseConnectionSettings implements DriverPropertyCreator {
2727
+ " ClickHouse rejects request execution if its time exceeds max_execution_time"),
2828

2929

30+
@Deprecated
3031
KEEP_ALIVE_TIMEOUT("keepAliveTimeout", 30 * 1000, ""),
3132

3233
/**
@@ -35,6 +36,7 @@ public enum ClickHouseConnectionSettings implements DriverPropertyCreator {
3536
TIME_TO_LIVE_MILLIS("timeToLiveMillis", 60 * 1000, ""),
3637
DEFAULT_MAX_PER_ROUTE("defaultMaxPerRoute", 500, ""),
3738
MAX_TOTAL("maxTotal", 10000, ""),
39+
MAX_RETRIES("maxRetries", 3, "Maximum retries(default to 3) for idempotent operation. Set 0 to disable retry."),
3840

3941
/**
4042
* additional

‎src/main/java/ru/yandex/clickhouse/settings/ClickHouseProperties.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public class ClickHouseProperties {
2222
private int timeToLiveMillis;
2323
private int defaultMaxPerRoute;
2424
private int maxTotal;
25+
private int maxRetries;
2526
private String host;
2627
private int port;
2728
private boolean usePathAsDb;
@@ -113,6 +114,7 @@ public ClickHouseProperties(Properties info) {
113114
this.timeToLiveMillis = (Integer)getSetting(info, ClickHouseConnectionSettings.TIME_TO_LIVE_MILLIS);
114115
this.defaultMaxPerRoute = (Integer)getSetting(info, ClickHouseConnectionSettings.DEFAULT_MAX_PER_ROUTE);
115116
this.maxTotal = (Integer)getSetting(info, ClickHouseConnectionSettings.MAX_TOTAL);
117+
this.maxRetries = (Integer)getSetting(info, ClickHouseConnectionSettings.MAX_RETRIES);
116118
this.maxCompressBufferSize = (Integer) getSetting(info, ClickHouseConnectionSettings.MAX_COMPRESS_BUFFER_SIZE);
117119
this.ssl = (Boolean) getSetting(info, ClickHouseConnectionSettings.SSL);
118120
this.sslRootCertificate = (String) getSetting(info, ClickHouseConnectionSettings.SSL_ROOT_CERTIFICATE);
@@ -179,6 +181,7 @@ public Properties asProperties() {
179181
ret.put(ClickHouseConnectionSettings.TIME_TO_LIVE_MILLIS.getKey(), String.valueOf(timeToLiveMillis));
180182
ret.put(ClickHouseConnectionSettings.DEFAULT_MAX_PER_ROUTE.getKey(), String.valueOf(defaultMaxPerRoute));
181183
ret.put(ClickHouseConnectionSettings.MAX_TOTAL.getKey(), String.valueOf(maxTotal));
184+
ret.put(ClickHouseConnectionSettings.MAX_RETRIES.getKey(), String.valueOf(maxRetries));
182185
ret.put(ClickHouseConnectionSettings.MAX_COMPRESS_BUFFER_SIZE.getKey(), String.valueOf(maxCompressBufferSize));
183186
ret.put(ClickHouseConnectionSettings.SSL.getKey(), String.valueOf(ssl));
184187
ret.put(ClickHouseConnectionSettings.SSL_ROOT_CERTIFICATE.getKey(), String.valueOf(sslRootCertificate));
@@ -248,6 +251,7 @@ public ClickHouseProperties(ClickHouseProperties properties) {
248251
setTimeToLiveMillis(properties.timeToLiveMillis);
249252
setDefaultMaxPerRoute(properties.defaultMaxPerRoute);
250253
setMaxTotal(properties.maxTotal);
254+
setMaxRetries(properties.maxRetries);
251255
setMaxCompressBufferSize(properties.maxCompressBufferSize);
252256
setSsl(properties.ssl);
253257
setSslRootCertificate(properties.sslRootCertificate);
@@ -594,6 +598,14 @@ public void setMaxTotal(int maxTotal) {
594598
this.maxTotal = maxTotal;
595599
}
596600

601+
public int getMaxRetries() {
602+
return maxRetries;
603+
}
604+
605+
public void setMaxRetries(int maxRetries) {
606+
this.maxRetries = maxRetries;
607+
}
608+
597609
public int getMaxCompressBufferSize() {
598610
return maxCompressBufferSize;
599611
}

‎src/main/java/ru/yandex/clickhouse/util/ClickHouseHttpClientBuilder.java‎

Lines changed: 20 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,19 @@
2727

2828
import org.apache.http.ConnectionReuseStrategy;
2929
import org.apache.http.Header;
30-
import org.apache.http.HeaderElement;
31-
import org.apache.http.HeaderElementIterator;
3230
import org.apache.http.HttpHeaders;
3331
import org.apache.http.HttpHost;
3432
import org.apache.http.HttpResponse;
33+
import org.apache.http.NoHttpResponseException;
3534
import org.apache.http.auth.AuthScope;
3635
import org.apache.http.auth.UsernamePasswordCredentials;
3736
import org.apache.http.client.AuthCache;
3837
import org.apache.http.client.CredentialsProvider;
38+
import org.apache.http.client.HttpRequestRetryHandler;
3939
import org.apache.http.client.config.RequestConfig;
4040
import org.apache.http.client.protocol.HttpClientContext;
4141
import org.apache.http.config.ConnectionConfig;
4242
import org.apache.http.config.RegistryBuilder;
43-
import org.apache.http.conn.ConnectionKeepAliveStrategy;
4443
import org.apache.http.conn.socket.ConnectionSocketFactory;
4544
import org.apache.http.conn.socket.PlainConnectionSocketFactory;
4645
import org.apache.http.conn.ssl.NoopHostnameVerifier;
@@ -50,11 +49,10 @@
5049
import org.apache.http.impl.client.BasicAuthCache;
5150
import org.apache.http.impl.client.BasicCredentialsProvider;
5251
import org.apache.http.impl.client.CloseableHttpClient;
52+
import org.apache.http.impl.client.DefaultHttpRequestRetryHandler;
5353
import org.apache.http.impl.client.HttpClientBuilder;
5454
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
5555
import org.apache.http.message.BasicHeader;
56-
import org.apache.http.message.BasicHeaderElementIterator;
57-
import org.apache.http.protocol.HTTP;
5856
import org.apache.http.protocol.HttpContext;
5957

6058
import ru.yandex.clickhouse.settings.ClickHouseProperties;
@@ -71,16 +69,32 @@ public ClickHouseHttpClientBuilder(ClickHouseProperties properties) {
7169
public CloseableHttpClient buildClient() throws Exception {
7270
return HttpClientBuilder.create()
7371
.setConnectionManager(getConnectionManager())
72+
.setRetryHandler(getRequestRetryHandler())
7473
.setConnectionReuseStrategy(getConnectionReuseStrategy())
7574
.setDefaultConnectionConfig(getConnectionConfig())
7675
.setDefaultRequestConfig(getRequestConfig())
7776
.setDefaultHeaders(getDefaultHeaders())
7877
.setDefaultCredentialsProvider(getDefaultCredentialsProvider())
79-
.disableContentCompression() // gzip здесь ни к чему. Используется lz4 при compress=1
78+
.disableContentCompression() // gzip is not needed. Use lz4 when compress=1
8079
.disableRedirectHandling()
8180
.build();
8281
}
8382

83+
private HttpRequestRetryHandler getRequestRetryHandler() {
84+
final int maxRetries = properties.getMaxRetries();
85+
return new DefaultHttpRequestRetryHandler(maxRetries, false) {
86+
@Override
87+
public boolean retryRequest(IOException exception, int executionCount, HttpContext context) {
88+
if (executionCount > maxRetries || context == null
89+
|| !Boolean.TRUE.equals(context.getAttribute("is_idempotent"))) {
90+
return false;
91+
}
92+
93+
return (exception instanceof NoHttpResponseException) || super.retryRequest(exception, executionCount, context);
94+
}
95+
};
96+
}
97+
8498
public static HttpClientContext createClientContext(ClickHouseProperties props) {
8599
if (props == null
86100
|| !isConfigurationValidForAuth(props))
@@ -155,29 +169,6 @@ private Collection<Header> getDefaultHeaders() {
155169
return headers;
156170
}
157171

158-
private ConnectionKeepAliveStrategy createKeepAliveStrategy() {
159-
return new ConnectionKeepAliveStrategy() {
160-
@Override
161-
public long getKeepAliveDuration(HttpResponse httpResponse, HttpContext httpContext) {
162-
// in case of errors keep-alive not always works. close connection just in case
163-
if (httpResponse.getStatusLine().getStatusCode() != HttpURLConnection.HTTP_OK) {
164-
return -1;
165-
}
166-
HeaderElementIterator it = new BasicHeaderElementIterator(
167-
httpResponse.headerIterator(HTTP.CONN_DIRECTIVE));
168-
while (it.hasNext()) {
169-
HeaderElement he = it.nextElement();
170-
String param = he.getName();
171-
//String value = he.getValue();
172-
if (param != null && param.equalsIgnoreCase(HTTP.CONN_KEEP_ALIVE)) {
173-
return properties.getKeepAliveTimeout();
174-
}
175-
}
176-
return -1;
177-
}
178-
};
179-
}
180-
181172
private SSLContext getSSLContext()
182173
throws CertificateException, NoSuchAlgorithmException, KeyStoreException, IOException, KeyManagementException {
183174
SSLContext ctx = SSLContext.getInstance("TLS");

‎src/test/java/ru/yandex/clickhouse/util/ClickHouseHttpClientBuilderTest.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package ru.yandex.clickhouse.util;
22

33
import org.apache.http.HttpHost;
4+
import org.apache.http.NoHttpResponseException;
45
import org.apache.http.client.methods.HttpPost;
6+
import org.apache.http.conn.HttpHostConnectException;
57
import org.apache.http.impl.client.CloseableHttpClient;
8+
import org.apache.http.protocol.BasicHttpContext;
9+
import org.apache.http.protocol.HttpContext;
610
import org.testng.annotations.AfterClass;
711
import org.testng.annotations.AfterMethod;
812
import org.testng.annotations.BeforeClass;
@@ -141,7 +145,72 @@ private static Object[][] provideAuthUserPasswordTestData() {
141145
null, null, "baz", "Basic ZGVmYXVsdDpiYXo=" // default:baz
142146
},
143147
};
148+
}
149+
150+
private static WireMockServer newServer() {
151+
WireMockServer server = new WireMockServer(
152+
WireMockConfiguration.wireMockConfig().dynamicPort());
153+
server.start();
154+
server.stubFor(WireMock.post(WireMock.urlPathMatching("/*"))
155+
.willReturn(WireMock.aResponse().withStatus(200).withHeader("Connection", "Keep-Alive")
156+
.withHeader("Content-Type", "text/plain; charset=UTF-8")
157+
.withHeader("Transfer-Encoding", "chunked").withHeader("Keep-Alive", "timeout=3")
158+
.withBody("OK.........................").withFixedDelay(2)));
159+
return server;
160+
}
144161

162+
private static void shutDownServerWithDelay(final WireMockServer server, final long delayMs) {
163+
new Thread() {
164+
public void run() {
165+
try {
166+
Thread.sleep(delayMs);
167+
} catch (InterruptedException e) {
168+
e.printStackTrace();
169+
}
170+
171+
server.shutdownServer();
172+
server.stop();
173+
}
174+
}.start();
145175
}
146176

177+
// @Test(dependsOnMethods = { "testWithRetry" }, expectedExceptions = { NoHttpResponseException.class })
178+
public void testWithoutRetry() throws Exception {
179+
final WireMockServer server = newServer();
180+
181+
ClickHouseProperties props = new ClickHouseProperties();
182+
props.setMaxRetries(0);
183+
ClickHouseHttpClientBuilder builder = new ClickHouseHttpClientBuilder(props);
184+
CloseableHttpClient client = builder.buildClient();
185+
HttpPost post = new HttpPost("http://localhost:" + server.port() + "/?db=system&query=select%201");
186+
187+
shutDownServerWithDelay(server, 100);
188+
189+
try {
190+
client.execute(post);
191+
} finally {
192+
client.close();
193+
}
194+
}
195+
196+
// @Test(expectedExceptions = { HttpHostConnectException.class })
197+
public void testWithRetry() throws Exception {
198+
final WireMockServer server = newServer();
199+
200+
ClickHouseProperties props = new ClickHouseProperties();
201+
// props.setMaxRetries(3);
202+
ClickHouseHttpClientBuilder builder = new ClickHouseHttpClientBuilder(props);
203+
CloseableHttpClient client = builder.buildClient();
204+
HttpContext context = new BasicHttpContext();
205+
context.setAttribute("is_idempotent", Boolean.TRUE);
206+
HttpPost post = new HttpPost("http://localhost:" + server.port() + "/?db=system&query=select%202");
207+
208+
shutDownServerWithDelay(server, 100);
209+
210+
try {
211+
client.execute(post, context);
212+
} finally {
213+
client.close();
214+
}
215+
}
147216
}

0 commit comments

Comments
 (0)