added retry on 503

ClickHouse · Jul 6, 2024 · 41b3e29 · 41b3e29
1 parent 29cf395
commit 41b3e29
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 12 deletions.
diff --git a/...khouse-http-client/src/main/java/com/clickhouse/client/http/ApacheHttpConnectionImpl.java b/...khouse-http-client/src/main/java/com/clickhouse/client/http/ApacheHttpConnectionImpl.java
@@ -30,6 +30,7 @@
 import org.apache.hc.client5.http.socket.PlainConnectionSocketFactory;
 import org.apache.hc.client5.http.ssl.DefaultHostnameVerifier;
 import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
+import org.apache.hc.core5.http.ConnectionClosedException;
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.HttpRequest;
@@ -253,12 +254,19 @@ protected ClickHouseHttpResponse post(ClickHouseConfig config, String sql, Click
 
         int retryAttempts = config.getBoolOption(ClickHouseHttpOption.AHC_RETRY_ON_FAILURE) ? 2 : 1;
         for (int attempt = 0; attempt < retryAttempts; attempt++) {
+            boolean isLastAttempt = attempt == retryAttempts - 1;
             log.debug("HTTP request attempt " + attempt);
             try {
                 response = client.execute(post);
+
+                if (!isLastAttempt && (response.getCode() == HttpURLConnection.HTTP_UNAVAILABLE)) {
+                    log.debug("HTTP request failed with status code 503, retrying...");
+                    continue;
+                }
+
                 break;
-            } catch (NoHttpResponseException e) {
-                if ((retryAttempts - attempt - 1) == 0) {
+            } catch (NoHttpResponseException | ConnectionClosedException e) {
+                if (isLastAttempt) {
                     throw new ConnectException(e.getMessage());
                 } else {
                     continue;

diff --git a/...use-http-client/src/main/java/com/clickhouse/client/http/config/ClickHouseHttpOption.java b/...use-http-client/src/main/java/com/clickhouse/client/http/config/ClickHouseHttpOption.java
@@ -70,7 +70,6 @@ public enum ClickHouseHttpOption implements ClickHouseOption {
     /**
      * 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>

diff --git a/...se-http-client/src/test/java/com/clickhouse/client/http/ApacheHttpConnectionImplTest.java b/...se-http-client/src/test/java/com/clickhouse/client/http/ApacheHttpConnectionImplTest.java
@@ -28,8 +28,11 @@
 import com.github.tomakehurst.wiremock.client.WireMock;
 import com.github.tomakehurst.wiremock.http.Fault;
 import com.github.tomakehurst.wiremock.stubbing.Scenario;
+import com.github.tomakehurst.wiremock.stubbing.StubMapping;
 import org.apache.hc.client5.http.socket.PlainConnectionSocketFactory;
+import org.apache.hc.core5.http.HttpStatus;
 import org.testng.Assert;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
 public class ApacheHttpConnectionImplTest extends ClickHouseHttpClientTest {
@@ -150,18 +153,12 @@ public void testFailureWhileRequest() {
         }
     }
 
-    @Test(groups = {"unit"})
-    public void testRetryOnFailure() {
+    @Test(groups = {"unit"}, dataProvider = "retryOnFailureProvider")
+    public void testRetryOnFailure(StubMapping failureStub) {
         faultyServer = new WireMockServer(9090);
         faultyServer.start();
         try {
-            faultyServer.addStubMapping(WireMock.post(WireMock.anyUrl())
-                            .withRequestBody(WireMock.equalTo("SELECT 1"))
-                            .inScenario("Retry")
-                            .whenScenarioStateIs(Scenario.STARTED)
-                            .willReturn(WireMock.aResponse().withFault(Fault.EMPTY_RESPONSE))
-                            .willSetStateTo("Failed")
-                    .build());
+            faultyServer.addStubMapping(failureStub);
             faultyServer.addStubMapping(WireMock.post(WireMock.anyUrl())
                             .withRequestBody(WireMock.equalTo("SELECT 1"))
                             .inScenario("Retry")
@@ -190,4 +187,24 @@ public void testRetryOnFailure() {
             faultyServer.stop();
         }
     }
+
+    @DataProvider(name = "retryOnFailureProvider")
+    private static StubMapping[] retryOnFailureProvider() {
+        return new StubMapping[] {
+                WireMock.post(WireMock.anyUrl())
+                        .withRequestBody(WireMock.equalTo("SELECT 1"))
+                        .inScenario("Retry")
+                        .whenScenarioStateIs(Scenario.STARTED)
+                        .willReturn(WireMock.aResponse().withFault(Fault.EMPTY_RESPONSE))
+                        .willSetStateTo("Failed")
+                        .build()
+                ,WireMock.post(WireMock.anyUrl())
+                        .withRequestBody(WireMock.equalTo("SELECT 1"))
+                        .inScenario("Retry")
+                        .whenScenarioStateIs(Scenario.STARTED)
+                        .willReturn(WireMock.aResponse().withStatus(HttpStatus.SC_SERVICE_UNAVAILABLE))
+                        .willSetStateTo("Failed")
+                        .build()
+        };
+    }
 }