Added retry on no reponse exception

Author: chernser

clickhouse-http-client/src/main/java/com/clickhouse/client/http/ApacheHttpConnectionImpl.java

@@ -3,7 +3,6 @@
 import com.clickhouse.client.AbstractSocketClient;
 import com.clickhouse.client.ClickHouseClient;
 import com.clickhouse.client.ClickHouseConfig;
-import com.clickhouse.client.ClickHouseException;
 import com.clickhouse.client.ClickHouseNode;
 import com.clickhouse.client.ClickHouseRequest;
 import com.clickhouse.client.ClickHouseSocketFactory;
@@ -34,6 +33,7 @@
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.HttpRequest;
+import org.apache.hc.core5.http.NoHttpResponseException;
 import org.apache.hc.core5.http.config.Registry;
 import org.apache.hc.core5.http.config.RegistryBuilder;
 import org.apache.hc.core5.http.io.SocketConfig;
@@ -45,7 +45,6 @@
 
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLException;
-
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
@@ -58,7 +57,6 @@
 import java.net.HttpURLConnection;
 import java.net.InetSocketAddress;
 import java.net.Socket;
-import java.net.StandardSocketOptions;
 import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.List;
@@ -251,11 +249,28 @@ protected ClickHouseHttpResponse post(ClickHouseConfig config, String sql, Click
         ClickHouseHttpEntity postBody = new ClickHouseHttpEntity(config, contentType, contentEncoding, boundary,
                 sql, data, tables);
         post.setEntity(postBody);
-        CloseableHttpResponse response;
-        try {
-            response = client.execute(post);
-        } catch (IOException e) {
-            throw new ConnectException(ClickHouseUtils.format("HTTP request failed: %s", e.getMessage()));
+        CloseableHttpResponse response = null;
+
+        for (int attempt = 0; attempt < 2; attempt++) {
+            log.debug("HTTP request attempt {}", attempt);
+            try {
+                response = client.execute(post);
+                break;
+            } catch (NoHttpResponseException e) {
+                log.warn("HTTP request failed: ", e.getMessage());
+                if (attempt > 0) {
+                    throw new ConnectException(e.getMessage());
+                } else {
+                    continue;
+                }
+            } catch (IOException e) {
+                log.error("HTTP request failed", e);
+                throw new ConnectException(e.getMessage());
+            }
+        }
+        if (response == null) {
+            // Should not happen but needed for compiler
+            throw new ConnectException("HTTP request failed");
         }
 
         checkResponse(config, response);
@@ -376,6 +391,8 @@ public HttpConnectionManager(Registry<ConnectionSocketFactory> socketFactory, Cl
 
             ConnectionConfig connConfig = ConnectionConfig.custom()
                     .setConnectTimeout(Timeout.of(config.getConnectionTimeout(), TimeUnit.MILLISECONDS))
+                    .setTimeToLive(20, TimeUnit.SECONDS)
+                    .setValidateAfterInactivity(10, TimeUnit.SECONDS)
                     .build();
             setDefaultConnectionConfig(connConfig);
 

clickhouse-http-client/src/main/java/com/clickhouse/client/http/config/ClickHouseHttpOption.java

@@ -4,6 +4,7 @@
 import com.clickhouse.data.ClickHouseChecker;
 
 import java.io.Serializable;
+import java.net.UnknownHostException;
 
 /**
  * Http client options.
@@ -64,7 +65,28 @@ public enum ClickHouseHttpOption implements ClickHouseOption {
      * Only one role can be set at a time.
      */
     REMEMBER_LAST_SET_ROLES("remember_last_set_roles", false,
-            "Whether to remember last set role and send them in every next requests as query parameters.");
+            "Whether to remember last set role and send them in every next requests as query parameters."),
+
+    /**
+     * Whether to retry on failure with AsyncHttpClient. Failure includes some 'critical' IO exceptions:
+     * <ul>
+     *     <li>{@code java.net.UnknownHostException}</li>
+     *     <li>{@code org.apache.hc.core5.http.ConnectionClosedException}</li>
+     *     <li>{@code org.apache.hc.core5.http.NoHttpResponseException}</li>
+     * </ul>
+     *
+     * And next status codes:
+     * <ul>
+     *     <li>{@code 503 Service Unavailable}</li>
+     * </ul>
+     */
+    AHC_RETRY_ON_FAILURE("ahc_retry_on_failure", true, "Whether to retry on failure with AsyncHttpClient."),
+
+    /**
+     * Retry interval in milliseconds for AsyncHttpClient (if {@link #AHC_RETRY_ON_FAILURE} is enabled).
+     */
+    AHC_RETRY_INTERVAL("ahc_retry_interval", 100, "Retry interval in milliseconds."),
+    ;
 
     private final String key;
     private final Serializable defaultValue;

clickhouse-http-client/src/test/java/com/clickhouse/client/http/ApacheHttpConnectionImplTest.java

@@ -24,8 +24,10 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import com.github.tomakehurst.wiremock.WireMockServer;
+import com.github.tomakehurst.wiremock.admin.model.ScenarioState;
 import com.github.tomakehurst.wiremock.client.WireMock;
 import com.github.tomakehurst.wiremock.http.Fault;
+import com.github.tomakehurst.wiremock.stubbing.Scenario;
 import org.apache.hc.client5.http.socket.PlainConnectionSocketFactory;
 import org.testng.Assert;
 import org.testng.annotations.Test;
@@ -144,4 +146,41 @@ public void testFailureWhileRequest() {
             faultyServer.stop();
         }
     }
+
+    @Test(groups = {"unit"})
+    public void testRetryOnFailure() {
+        faultyServer = new WireMockServer(9090);
+        faultyServer.start();
+        try {
+            faultyServer.addStubMapping(WireMock.post(WireMock.anyUrl())
+                            .inScenario("Retry")
+                            .whenScenarioStateIs(Scenario.STARTED)
+                            .willReturn(WireMock.aResponse().withFault(Fault.EMPTY_RESPONSE))
+                            .willSetStateTo("Failed")
+                    .build());
+            faultyServer.addStubMapping(WireMock.post(WireMock.anyUrl())
+                            .inScenario("Retry")
+                            .whenScenarioStateIs("Failed")
+                            .willReturn(WireMock.aResponse()
+                                    .withHeader("X-ClickHouse-Summary",
+                                            "{ \"read_bytes\": \"10\", \"read_rows\": \"1\"}"))
+                    .build());
+
+            ClickHouseHttpClient httpClient = new ClickHouseHttpClient();
+            ClickHouseConfig config = new ClickHouseConfig();
+            httpClient.init(config);
+            ClickHouseRequest request = httpClient.read("http://localhost:9090/").query("SELECT 1");
+
+            ClickHouseResponse response = null;
+            try {
+                response = httpClient.executeAndWait(request);
+            } catch (ClickHouseException e) {
+                Assert.fail("Should not throw exception", e);
+            }
+            Assert.assertEquals(response.getSummary().getReadBytes(), 10);
+            Assert.assertEquals(response.getSummary().getReadRows(), 1);
+        } finally {
+            faultyServer.stop();
+        }
+    }
 }