Merge pull request #2084 from ClickHouse/fix-default-timeouts

[client-v2] Removed old defaults and replaced new ones

Author: Paultagoras

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

@@ -255,47 +255,6 @@ public static class Builder {
         public Builder() {
             this.endpoints = new HashSet<>();
             this.configuration = new HashMap<String, String>();
-            // TODO: set defaults configuration values
-            this.setConnectTimeout(30, SECONDS)
-                    .setSocketTimeout(2, SECONDS)
-                    .setSocketRcvbuf(804800)
-                    .setSocketSndbuf(804800)
-                    .compressServerResponse(true)
-                    .compressClientRequest(false);
-        }
-
-        /**
-         * Builds a client object with the provided configuration through URL parameters.
-         *  (e.g. http://localhost:8123/someDatabase?user=default)
-         * @param urlString - URL formatted string with protocol, host, port, and client configuration settings.
-         * @return Client - a client object
-         */
-        public Builder fromUrl(String urlString) {
-            try {
-                URL url = new URL(urlString);
-                Map<String, String> urlProperties = HttpAPIClientHelper.parseUrlParameters(url);
-
-                // Add the endpoint
-                boolean secure = url.getProtocol().equalsIgnoreCase("https");
-                int port = url.getPort();
-                if (port == -1) {
-                    port = secure ? ClickHouseHttpProto.DEFAULT_HTTPS_PORT : ClickHouseHttpProto.DEFAULT_HTTP_PORT;
-                }
-                this.addEndpoint(Protocol.HTTP, url.getHost(), port, secure);
-
-                for (ClientConfigProperties properties: ClientConfigProperties.values()) {
-                    String value = urlProperties.get(properties.getKey());
-                    if (value != null) {
-                        this.configuration.put(properties.getKey(), value);
-                    }
-                }
-
-                // Set the database
-            } catch (java.net.MalformedURLException e) {
-                throw new IllegalArgumentException("Configuration via URL should be done with a valid URL string", e);
-            }
-
-            return this;
         }
 
         /**
@@ -520,6 +479,7 @@ public Builder setSocketTimeout(long timeout) {
 
         /**
          * Default socket timeout in milliseconds. Timeout is applied to read and write operations.
+         * Default is 0.
          *
          * @param timeout - socket timeout value
          * @param unit - time unit
@@ -1076,6 +1036,7 @@ public Client build() {
          * Default size for a buffers used in networking.
          */
         public static final int DEFAULT_BUFFER_SIZE = 8192;
+        public static final int DEFAULT_SOCKET_BUFFER_SIZE = 804800;
 
         private void setDefaults() {
 
@@ -1169,6 +1130,18 @@ private void setDefaults() {
             if (!configuration.containsKey(ClientConfigProperties.APP_COMPRESSED_DATA.getKey())) {
                 appCompressedData(false);
             }
+
+            if (!configuration.containsKey(ClientConfigProperties.SOCKET_OPERATION_TIMEOUT.getKey())) {
+                setSocketTimeout(0, SECONDS);
+            }
+
+            if (!configuration.containsKey(ClientConfigProperties.SOCKET_RCVBUF_OPT.getKey())) {
+                setSocketRcvbuf(DEFAULT_SOCKET_BUFFER_SIZE);
+            }
+
+            if (!configuration.containsKey(ClientConfigProperties.SOCKET_SNDBUF_OPT.getKey())) {
+                setSocketSndbuf(DEFAULT_SOCKET_BUFFER_SIZE);
+            }
         }
     }
 

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

@@ -39,6 +39,7 @@
 
 import java.io.ByteArrayInputStream;
 import java.net.Socket;
+import java.net.SocketTimeoutException;
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.time.temporal.ChronoUnit;
@@ -52,6 +53,7 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.regex.Pattern;
 import java.util.function.Supplier;
@@ -1025,6 +1027,38 @@ public void testJWTWithCloud() throws Exception {
         }
     }
 
+    @Test(groups = { "integration" })
+    public void testWithDefaultTimeouts() {
+        if (isCloud()) {
+            return; // mocked server
+        }
+
+        int proxyPort = new Random().nextInt(1000) + 10000;
+        WireMockServer proxy = new WireMockServer(WireMockConfiguration
+                .options().port(proxyPort)
+                .notifier(new Slf4jNotifier(true)));
+        proxy.start();
+        proxy.addStubMapping(WireMock.post(WireMock.anyUrl())
+                .willReturn(WireMock.aResponse().withFixedDelay(5000)
+                        .withStatus(HttpStatus.SC_OK)
+                        .withHeader("X-ClickHouse-Summary", "{ \"read_bytes\": \"10\", \"read_rows\": \"1\"}")).build());
+
+        try (Client client = new Client.Builder().addEndpoint(Protocol.HTTP, "localhost", proxyPort, false)
+                .setUsername("default")
+                .setPassword("")
+                .useNewImplementation(true)
+                .build()) {
+            int startTime = (int) System.currentTimeMillis();
+            try {
+                client.query("SELECT 1").get();
+            } catch (Exception e) {
+                Assert.fail("Elapsed Time: " + (System.currentTimeMillis() - startTime), e);
+            }
+        } finally {
+            proxy.stop();
+        }
+    }
+
     protected Client.Builder newClient() {
         ClickHouseNode node = getServer(ClickHouseProtocol.HTTP);
         boolean isSecure = isCloud();