fix(client-v2): record connection times only when metrics are enabled

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

@@ -297,6 +297,7 @@ private HttpClientConnectionManager poolConnectionManager(LayeredConnectionSocke
         PoolingHttpClientConnectionManager phccm = connMgrBuilder.build();
         poolControl = phccm;
         if (metricsRegistry != null) {
+            connectionFactory.enableMetrics();
             try {
                 String mGroupName = ClientConfigProperties.METRICS_GROUP_NAME.getOrDefault(configuration);
                 Class<?> micrometerLoader = getClass().getClassLoader().loadClass("com.clickhouse.client.api.metrics.MicrometerLoader");
@@ -1380,9 +1381,19 @@ public MeteredManagedHttpClientConnectionFactory(Http1Config http1Config, CharCo
 
         ConcurrentLinkedQueue<Long> times = new ConcurrentLinkedQueue<>();
 
+        // Connection times are drained only by the metrics gauge (see getTime()), so they are recorded
+        // only when a metrics registry is attached. Otherwise the queue would grow with every new connection.
+        private volatile boolean metricsEnabled = false;
+
+        public void enableMetrics() {
+            this.metricsEnabled = true;
+        }
 
         @Override
         public ManagedHttpClientConnection createConnection(Socket socket) throws IOException {
+            if (!metricsEnabled) {
+                return super.createConnection(socket);
+            }
             long startT = System.currentTimeMillis();
             try {
                 return super.createConnection(socket);

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

@@ -0,0 +1,38 @@
+package com.clickhouse.client.api.internal;
+
+import org.apache.hc.core5.http.config.CharCodingConfig;
+import org.apache.hc.core5.http.config.Http1Config;
+import org.apache.hc.core5.http.impl.io.DefaultHttpResponseParserFactory;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import java.net.Socket;
+
+public class MeteredConnectionFactoryTest {
+
+    private static HttpAPIClientHelper.MeteredManagedHttpClientConnectionFactory newFactory() {
+        return new HttpAPIClientHelper.MeteredManagedHttpClientConnectionFactory(
+                Http1Config.DEFAULT, CharCodingConfig.DEFAULT, DefaultHttpResponseParserFactory.INSTANCE);
+    }
+
+    @Test(groups = {"unit"})
+    public void testConnectionTimesNotRecordedWithoutMetrics() throws Exception {
+        HttpAPIClientHelper.MeteredManagedHttpClientConnectionFactory factory = newFactory();
+        for (int i = 0; i < 10; i++) {
+            factory.createConnection(new Socket());
+        }
+        Assert.assertEquals(factory.times.size(), 0, "Connection times must not accumulate when metrics are disabled");
+    }
+
+    @Test(groups = {"unit"})
+    public void testConnectionTimesRecordedAndDrainedWithMetrics() throws Exception {
+        HttpAPIClientHelper.MeteredManagedHttpClientConnectionFactory factory = newFactory();
+        factory.enableMetrics();
+        for (int i = 0; i < 10; i++) {
+            factory.createConnection(new Socket());
+        }
+        Assert.assertEquals(factory.times.size(), 10);
+        factory.getTime();
+        Assert.assertEquals(factory.times.size(), 0);
+    }
+}

history/latest/3253.md

@@ -0,0 +1,4 @@
+- **[client-v2]** Fixed a memory leak where every new pooled HTTP connection added one entry to an internal queue of
+  connection times that was only drained by the metrics gauge. Without a metrics registry the queue was never emptied
+  and grew for the lifetime of the client. Connection times are now recorded only when metrics are enabled. `jdbc-v2`
+  is affected as well because it uses `client-v2`. (https://github.com/ClickHouse/clickhouse-java/issues/3253)