Resolved changes between req cancellation and retries

Author: chernser

client-v2/src/main/java/com/clickhouse/client/api/Client.java

@@ -1688,11 +1688,15 @@ public CompletableFuture<InsertResponse> insert(String tableName,
                     } catch (Exception e) {
                         String msg = requestExMsg("Insert", (i + 1), durationSince(startTime).toMillis(), requestSettings.getQueryId());
                         lastException = httpClientHelper.wrapException(msg, e, requestSettings.getQueryId());
-                        if (i < maxAttempts) {
-                            LOG.warn("Retrying.", e);
-                            selectedEndpoint = nodeSelector.getNextAliveNode(selectedEndpoint);
+                        if (httpClientHelper.shouldRetry(e, requestSettings.getAllSettings())) {
+                            if (i < maxAttempts) {
+                                LOG.warn("Retrying.", e);
+                                selectedEndpoint = nodeSelector.getNextAliveNode(selectedEndpoint);
+                            } else {
+                                nodeSelector.getNextAliveNode(selectedEndpoint);
+                            }
                         } else {
-                            nodeSelector.getNextAliveNode(selectedEndpoint);
+                            throw lastException;
                         }
                     } finally {
                         // Insert completes once the request returns; the response exposes no stream to read afterwards,

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

@@ -716,9 +716,6 @@ public TransportResponse executeRequest(TransportRequest transportRequest) throw
                     throw new ClientMisconfigurationException("Proxy authentication required. Please check your proxy settings.");
                 case HttpStatus.SC_BAD_GATEWAY:
                     throw new ConnectException("Server returned '502 Bad gateway'. Check network and proxy settings.");
-            } else if (httpResponse.getCode() == HttpStatus.SC_SERVICE_UNAVAILABLE) {
-                httpResponse.close();
-                throw new ConnectException("Server returned '503 Service unavailable'.");
                 case HttpStatus.SC_SERVICE_UNAVAILABLE:
                     throw new ConnectException("Server returned '503 Service Unavailable'. Check network settings.");
                 case HttpStatus.SC_BAD_REQUEST:

client-v2/src/test/java/com/clickhouse/client/InsertExceptionClassificationTest.java

@@ -6,9 +6,10 @@
 import com.clickhouse.client.api.metadata.TableSchema;
 import com.clickhouse.client.api.serde.DataSerializationException;
 import com.clickhouse.client.api.transport.Endpoint;
+import com.clickhouse.client.api.transport.internal.TransportRequest;
+import com.clickhouse.client.api.transport.internal.TransportResponse;
 import com.clickhouse.data.ClickHouseColumn;
 import net.jpountz.lz4.LZ4Factory;
-import org.apache.hc.core5.http.ClassicHttpResponse;
 import org.apache.hc.core5.io.IOCallback;
 import org.mockito.Mockito;
 import org.testng.Assert;
@@ -20,6 +21,7 @@
 import java.lang.reflect.Field;
 import java.net.SocketException;
 import java.util.Collections;
+import java.util.Map;
 
 public class InsertExceptionClassificationTest {
 
@@ -96,17 +98,27 @@ private static String repeat(char ch, int count) {
 
     private static final class CallbackHttpClientHelper extends HttpAPIClientHelper {
         private final OutputStream outputStream;
-
+        private IOCallback<OutputStream> writeCallback;
         private CallbackHttpClientHelper(OutputStream outputStream) {
             super(Collections.emptyMap(), null, false, LZ4Factory.fastestJavaInstance());
             this.outputStream = outputStream;
         }
 
         @Override
-        public ClassicHttpResponse executeRequest(Endpoint server, java.util.Map<String, Object> requestConfig,
-                                                  IOCallback<OutputStream> writeCallback) throws Exception {
+        public TransportRequest createRequest(Endpoint server, Map<String, Object> requestConfig, String body) {
+            return null;
+        }
+
+        @Override
+        public TransportRequest createRequest(Endpoint server, Map<String, Object> requestConfig, IOCallback<OutputStream> writeCallback) {
+            this.writeCallback = writeCallback;
+            return null;
+        }
+
+        @Override
+        public TransportResponse executeRequest(TransportRequest request) throws Exception {
             writeCallback.execute(outputStream);
-            return Mockito.mock(ClassicHttpResponse.class);
+            return Mockito.mock(TransportResponse.class);
         }
 
         @Override

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

@@ -5,6 +5,7 @@
 import com.clickhouse.client.api.internal.HttpAPIClientHelper.CustomSSLConnectionFactory;
 import com.clickhouse.client.api.transport.Endpoint;
 import com.clickhouse.client.api.transport.HttpEndpoint;
+import com.clickhouse.client.api.transport.internal.TransportRequest;
 import net.jpountz.lz4.LZ4Factory;
 import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
 import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
@@ -247,7 +248,8 @@ public void testExecuteRequestThrowsConnectExceptionOn502() throws Exception {
 
         Endpoint endpoint = new HttpEndpoint("localhost", 8123, false, "/");
         try {
-            helper.executeRequest(endpoint, new HashMap<>(), "SELECT 1");
+            TransportRequest req = helper.createRequest(endpoint, new HashMap<>(), "SELECT 1");
+            helper.executeRequest(req).close();
             Assert.fail("Expected ConnectException to be thrown");
         } catch (ConnectException e) {
             // expected
@@ -275,7 +277,8 @@ public void testExecuteRequestThrowsConnectExceptionOn503() throws Exception {
 
         Endpoint endpoint = new HttpEndpoint("localhost", 8123, false, "/");
         try {
-            helper.executeRequest(endpoint, new HashMap<>(), "SELECT 1");
+            TransportRequest req = helper.createRequest(endpoint, new HashMap<>(), "SELECT 1");
+            helper.executeRequest(req).close();
             Assert.fail("Expected ConnectException to be thrown");
         } catch (ConnectException e) {
             // expected