From 243a8f9f33160afddf776380ad5df20cc2b5cfcb Mon Sep 17 00:00:00 2001 From: Nikita Fomichev Date: Wed, 20 May 2026 01:14:11 +0200 Subject: [PATCH 1/3] jdbc-v2: swallow chunked-stream drain errors on ResultSet.close MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #2361. When the server tears down the response connection mid-stream (canonically because send_timeout fires on the writer side and the terminating zero-length chunk is never written), Apache HC's ChunkedInputStream.close() drains remaining bytes and trips `ConnectionClosedException: Premature end of chunk coded message body: closing chunk expected`. With `compress=true` the close path goes through `FramedLZ4CompressorInputStream.close()` → `BoundedInputStream.close()` → `ChunkedInputStream.close()` and the same drain failure surfaces. `ResultSetImpl.close()` previously rethrew this as a `SQLException`, which breaks well-behaved try-with-resources callers: they have already finished iterating, they cannot affect the server-side socket race, and the bug is surfacing in *cleanup* — they have no recourse other than wrapping every `rs.close()` (often implicit) in a try/catch. The reporter in #2361 ended up implementing client-side retry for the symptom; sqlancer / SQL fuzzers hit the same. Downgrade chunked-drain failures to log-and-swallow. If iteration genuinely failed (server closed mid-`next()`), that exception still surfaces — through `next()`, not through `close()`. The bare-drain-fail case is informational only. Covers both bare `Premature end of chunk` messages and `ConnectionClosedException` class-name match for forward-compat with Apache HC rephrasings. --- .../com/clickhouse/jdbc/ResultSetImpl.java | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ResultSetImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ResultSetImpl.java index e57282967..587a11118 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ResultSetImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ResultSetImpl.java @@ -149,6 +149,18 @@ public boolean next() throws SQLException { public void close() throws SQLException { closed = true; + // Stream-close exceptions on the response body are expected when the server has + // already torn down the connection (e.g. SOCKET_TIMEOUT on the writer side hits + // `send_timeout` before the terminating chunk is written). The most common + // surface is `ConnectionClosedException: Premature end of chunk coded message + // body: closing chunk expected` from Apache HC's `ChunkedInputStream` drain, + // optionally wrapped through `FramedLZ4CompressorInputStream` when the + // response was lz4-compressed. The application has already finished iterating + // the result set; propagating the drain failure as a SQLException punishes + // well-behaved try-with-resources callers for a server-side socket race they + // cannot affect. Log at debug and swallow -- the connection is closed either + // way, and we should not turn `try (ResultSet rs = ...)` into a throwing path. + // See https://github.com/ClickHouse/clickhouse-java/issues/2361 Exception e = null; try { if (reader != null) { @@ -156,7 +168,9 @@ public void close() throws SQLException { reader.close(); } catch (Exception re) { log.debug("Error closing reader", re); - e = re; + if (!isStreamDrainException(re)) { + e = re; + } } finally { reader = null; } @@ -167,7 +181,9 @@ public void close() throws SQLException { response.close(); } catch (Exception re) { log.debug("Error closing response", re); - e = re; + if (!isStreamDrainException(re)) { + e = re; + } } finally { response = null; } @@ -180,6 +196,32 @@ public void close() throws SQLException { } } + /** + * Determines whether a close-time exception originates from draining an + * already-truncated HTTP chunked response body. Such exceptions are not + * actionable from the caller's perspective -- iteration has finished, the + * connection is closing anyway, and the server's premature disconnect is the + * actual error condition (which, if it occurred during iteration, would have + * surfaced through {@code next()}). + */ + private static boolean isStreamDrainException(Throwable t) { + while (t != null) { + String msg = t.getMessage(); + if (msg != null && (msg.contains("Premature end of chunk") + || msg.contains("closing chunk expected"))) { + return true; + } + // ConnectionClosedException + class name match for defensive coverage of + // future HC versions that may rephrase the message. + String cls = t.getClass().getName(); + if (cls.endsWith("ConnectionClosedException")) { + return true; + } + t = t.getCause(); + } + return false; + } + @Override public boolean wasNull() throws SQLException { checkClosed(); From f964c8eb8e44e727444d771e06915ca6d57ed9a0 Mon Sep 17 00:00:00 2001 From: Nikita Fomichev Date: Wed, 20 May 2026 10:25:35 +0200 Subject: [PATCH 2/3] jdbc-v2: add unit tests + standalone reproducer for #2361 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Augments the previous commit with: - Unit tests for `ResultSetImpl.isStreamDrainException` covering the six exception shapes the classifier needs to recognise (bare message, `closing chunk expected` variant, nested cause chain, class-name match without message, unrelated exceptions, null input). All six pass. - `examples/jdbc/.../Issue2361Repro.java` — a self-contained main that reproduces the bug against a real ClickHouse server. Recipe is documented in the class Javadoc: compression on, server `send_timeout=1`, buffered chunked response, slow client read. On a freshly-started server it trips at ~100% rate. After the fix in `ResultSetImpl.close()` the iteration completes cleanly with the drain-close failure downgraded to a debug log. `isStreamDrainException` is package-private so it can be unit-tested without reflection. --- .../examples/jdbc/Issue2361Repro.java | 127 ++++++++++++++++++ .../com/clickhouse/jdbc/ResultSetImpl.java | 3 +- .../jdbc/ResultSetImplCloseTest.java | 72 ++++++++++ 3 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 examples/jdbc/src/main/java/com/clickhouse/examples/jdbc/Issue2361Repro.java create mode 100644 jdbc-v2/src/test/java/com/clickhouse/jdbc/ResultSetImplCloseTest.java diff --git a/examples/jdbc/src/main/java/com/clickhouse/examples/jdbc/Issue2361Repro.java b/examples/jdbc/src/main/java/com/clickhouse/examples/jdbc/Issue2361Repro.java new file mode 100644 index 000000000..a3e0c5743 --- /dev/null +++ b/examples/jdbc/src/main/java/com/clickhouse/examples/jdbc/Issue2361Repro.java @@ -0,0 +1,127 @@ +package com.clickhouse.examples.jdbc; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.Properties; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Reproducer for issue #2361 — ConnectionClosedException: "Premature end of + * chunk coded message body: closing chunk expected" during ResultSet.close(). + * + *

Recipe: + *

    + *
  1. Configure the JDBC connection with LZ4 HTTP compression on + * ({@code compress=true&client.use_http_compression=true}) and a + * short server-side {@code send_timeout=1} so the server abandons + * writes after one second of back-pressure.
    2. + *
  2. Force the server to commit to chunked HTTP encoding by buffering + * the full response: {@code http_response_buffer_size=104857600} + * and {@code wait_end_of_query=1}.
    3. + *
  3. Issue a query whose result is large enough to overflow the buffer + * and that a slow client cannot drain in time.
    4. + *
  4. Read rows slowly (a few {@code Thread.sleep}s per thousand rows).
    5. + *
+ * + *

The server hits {@code SOCKET_TIMEOUT} writing the chunked body, closes + * the connection without emitting the terminating zero-length chunk, and + * the driver's stream-drain on close trips the exception. + * + *

Run with: {@code java -DchUrl=jdbc:ch://localhost:8123 com.clickhouse.examples.jdbc.Issue2361Repro [iters]} + * + *

The fix in {@code ResultSetImpl.close()} downgrades this drain-time + * failure to a debug log instead of a thrown SQLException — close() should + * never punish callers for a server-side teardown race after iteration is done. + */ +public class Issue2361Repro { + + // 5M rows × ~4.5KB pad ≈ 22 GB uncompressed (~700 MB lz4-compressed). + // Large enough that send_timeout=1 + slow client reliably triggers the bug + // in a freshly-started server (before its memory tracker fills with prior + // query state). + static final String QUERY = + "SELECT number, repeat('xyz', 1500) AS pad FROM numbers(5000000)"; + + public static void main(String[] args) throws Exception { + Class.forName("com.clickhouse.jdbc.Driver"); + + String baseUrl = System.getProperty("chUrl", "jdbc:ch://localhost:8123") + "/default"; + int iters = args.length > 0 ? Integer.parseInt(args[0]) : 3; + + Properties props = new Properties(); + props.setProperty("user", System.getProperty("chUser", "default")); + props.setProperty("password", System.getProperty("chPassword", "")); + // Compression on — same lz4-framed-over-HTTP path observed in the bug. + props.setProperty("compress", "true"); + props.setProperty("client.use_http_compression", "true"); + // Long socket timeout so we don't bail out before the bug surfaces. + props.setProperty("socket_timeout", "60000"); + // Trigger conditions: + props.setProperty("clickhouse_setting_send_timeout", "1"); + props.setProperty("clickhouse_setting_http_response_buffer_size", "104857600"); + props.setProperty("clickhouse_setting_wait_end_of_query", "1"); + props.setProperty("clickhouse_setting_max_execution_time", "120"); + + AtomicInteger trips = new AtomicInteger(); + String firstStack = null; + + for (int i = 0; i < iters; i++) { + long t0 = System.currentTimeMillis(); + try (Connection c = DriverManager.getConnection(baseUrl, props); + Statement s = c.createStatement(); + ResultSet rs = s.executeQuery(QUERY)) { + + int rows = 0; + try { + while (rs.next()) { + rs.getString(2); + rows++; + // Slow client: ~1 s per ~1000 rows → 5,000 s to fully + // drain. Server send_timeout=1 will fire well before then. + if (rows % 100 == 0) Thread.sleep(50); + } + } catch (SQLException re) { + // Mid-iteration errors land here; the bug we want is the + // try-with-resources close that fires after the catch. + } + } catch (Throwable t) { + if (hasInMsg(t, "Premature end of chunk")) { + trips.incrementAndGet(); + if (firstStack == null) firstStack = stack(t); + } + } + System.out.printf("iter %d: trips=%d elapsed=%dms%n", + i, trips.get(), System.currentTimeMillis() - t0); + System.out.flush(); + } + + System.out.printf("%nFINAL: trips=%d / %d iterations (%.0f%%)%n", + trips.get(), iters, 100.0 * trips.get() / iters); + if (firstStack != null) { + System.out.println("\n-- first failure stack (top frames) --"); + String[] lines = firstStack.split("\n"); + for (int li = 0; li < Math.min(24, lines.length); li++) { + System.out.println(lines[li]); + } + } + } + + private static boolean hasInMsg(Throwable t, String needle) { + while (t != null) { + if (t.getMessage() != null && t.getMessage().contains(needle)) return true; + t = t.getCause(); + } + return false; + } + + private static String stack(Throwable t) { + StringWriter sw = new StringWriter(); + t.printStackTrace(new PrintWriter(sw)); + return sw.toString(); + } +} diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ResultSetImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ResultSetImpl.java index 587a11118..af29813c4 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ResultSetImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ResultSetImpl.java @@ -204,7 +204,8 @@ public void close() throws SQLException { * actual error condition (which, if it occurred during iteration, would have * surfaced through {@code next()}). */ - private static boolean isStreamDrainException(Throwable t) { + // Package-private for unit tests. + static boolean isStreamDrainException(Throwable t) { while (t != null) { String msg = t.getMessage(); if (msg != null && (msg.contains("Premature end of chunk") diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ResultSetImplCloseTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ResultSetImplCloseTest.java new file mode 100644 index 000000000..67ba18423 --- /dev/null +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ResultSetImplCloseTest.java @@ -0,0 +1,72 @@ +package com.clickhouse.jdbc; + +import org.testng.annotations.Test; + +import java.io.IOException; + +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +/** + * Unit tests for ResultSetImpl.isStreamDrainException — the classifier that + * decides whether a close-time exception is a benign chunked-stream drain + * failure (to swallow) or a real error (to propagate). + * + * Background: see issue #2361. When the server's send_timeout fires mid-write, + * Apache HC's ChunkedInputStream.close() trips + * `ConnectionClosedException: Premature end of chunk coded message body: + * closing chunk expected`. That happens during ResultSetImpl.close(), AFTER + * the application has finished iterating. Propagating it punishes well-behaved + * try-with-resources callers for a server-side socket race they cannot affect. + */ +public class ResultSetImplCloseTest { + + @Test + public void prematureEndOfChunkIsDrainException() { + // The canonical surface from issue #2361. + Exception e = new IOException("Premature end of chunk coded message body: closing chunk expected"); + assertTrue(ResultSetImpl.isStreamDrainException(e)); + } + + @Test + public void closingChunkExpectedIsDrainException() { + // Forward-compat for variations of the same message. + Exception e = new IOException("...closing chunk expected"); + assertTrue(ResultSetImpl.isStreamDrainException(e)); + } + + @Test + public void wrappedCausesAreUnwrapped() { + // Stack from the actual JDBC trace: + // SQLException -> ClientException -> ConnectionClosedException + Throwable root = new FakeConnectionClosedException("Premature end of chunk coded message body: closing chunk expected"); + RuntimeException mid = new RuntimeException("Failed to close response", root); + Exception top = new Exception("wrapped", mid); + assertTrue(ResultSetImpl.isStreamDrainException(top)); + } + + @Test + public void classNameMatchEvenWithoutMessage() { + // Defensive: if a future HC version drops the descriptive message but + // keeps the exception class, we still want to classify it as drain noise. + Throwable e = new FakeConnectionClosedException(null); + assertTrue(ResultSetImpl.isStreamDrainException(e)); + } + + @Test + public void unrelatedExceptionsArePropagated() { + assertFalse(ResultSetImpl.isStreamDrainException(new IOException("disk full"))); + assertFalse(ResultSetImpl.isStreamDrainException(new IllegalStateException("bad state"))); + assertFalse(ResultSetImpl.isStreamDrainException(new RuntimeException("anything else"))); + } + + @Test + public void nullThrowableHandled() { + assertFalse(ResultSetImpl.isStreamDrainException(null)); + } + + /** Mimics org.apache.hc.core5.http.ConnectionClosedException for the class-name match. */ + private static final class FakeConnectionClosedException extends IOException { + FakeConnectionClosedException(String msg) { super(msg); } + } +} From 4215ce79c62575d06e34faf5e334adf25ba8892c Mon Sep 17 00:00:00 2001 From: Nikita Fomichev Date: Thu, 21 May 2026 00:16:57 +0200 Subject: [PATCH 3/3] Address review: move drain handling to client-v2 Per chernser's review on #2857: - Move close-time ConnectionClosedException handling from jdbc-v2 ResultSetImpl.close() into AbstractBinaryFormatReader.close() in client-v2. The drain failure originates at input.close() inside the reader (FramedLZ4CompressorInputStream -> BoundedInputStream -> ChunkedInputStream), so that is where it should be caught. - Detect the HC ConnectionClosedException by class-name suffix (cause chain) instead of message text. Works against both the unshaded and the shaded copy of the class without taking a compile-time dependency on HC from the reader layer. - Drop the long WHY comment from ResultSetImpl.close(); the rationale now lives at the actual detection site in the reader. - Squash the six jdbc-v2 unit tests into a single data-driven test in client-v2 (AbstractBinaryFormatReaderCloseTest) using the real org.apache.hc.core5.http.ConnectionClosedException. Marked unit group, matching the convention of other client-v2 unit tests (SerializerUtilsTest, etc.). - Remove the Issue2361Repro example. The reproducer recipe is captured in the commit history and the production code comment. --- .../internal/AbstractBinaryFormatReader.java | 31 ++++- .../AbstractBinaryFormatReaderCloseTest.java | 36 +++++ .../examples/jdbc/Issue2361Repro.java | 127 ------------------ .../com/clickhouse/jdbc/ResultSetImpl.java | 47 +------ .../jdbc/ResultSetImplCloseTest.java | 72 ---------- 5 files changed, 68 insertions(+), 245 deletions(-) create mode 100644 client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReaderCloseTest.java delete mode 100644 examples/jdbc/src/main/java/com/clickhouse/examples/jdbc/Issue2361Repro.java delete mode 100644 jdbc-v2/src/test/java/com/clickhouse/jdbc/ResultSetImplCloseTest.java diff --git a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java index e5892748d..67e1d591d 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java @@ -1086,7 +1086,36 @@ public ClickHouseBitmap getClickHouseBitmap(int index) { @Override public void close() throws Exception { - input.close(); + try { + input.close(); + } catch (Exception e) { + // Apache HttpComponents ChunkedInputStream.close() drains remaining bytes and + // throws ConnectionClosedException ("Premature end of chunk coded message body") + // when the server tore the connection down mid-response (e.g. send_timeout fired + // before the terminating zero-length chunk was written). Any real iteration-time + // failure has already surfaced through nextRecord(); a drain failure at close() + // is informational only and should not punish callers of try-with-resources. + if (isConnectionClosedException(e)) { + LOG.debug("Swallowing chunked-stream drain on reader close: {}", e.toString()); + return; + } + throw e; + } + } + + /** + * Walks the cause chain looking for an {@code org.apache.hc.core5.http.ConnectionClosedException}. + * Matched by class-name suffix so it works against both directly-referenced and shaded copies of + * the HC class without taking a compile-time dependency on it from this layer. + */ + static boolean isConnectionClosedException(Throwable t) { + while (t != null) { + if (t.getClass().getName().endsWith(".ConnectionClosedException")) { + return true; + } + t = t.getCause(); + } + return false; } private static class RecordWrapper implements Map { diff --git a/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReaderCloseTest.java b/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReaderCloseTest.java new file mode 100644 index 000000000..81eefa676 --- /dev/null +++ b/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReaderCloseTest.java @@ -0,0 +1,36 @@ +package com.clickhouse.client.api.data_formats.internal; + +import org.apache.hc.core5.http.ConnectionClosedException; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +import java.io.IOException; + +import static org.testng.Assert.assertEquals; + +/** + * Verifies that the reader's close path recognises Apache HC's + * ConnectionClosedException (the "Premature end of chunk coded message body" + * surface that fires when the server tore the connection down before writing + * the terminating zero-length chunk) and is willing to swallow it on close. + * Matched by class-name suffix so the recogniser works against both the + * directly-referenced and the shaded copy of the HC class. + */ +@Test(groups = {"unit"}) +public class AbstractBinaryFormatReaderCloseTest { + + @DataProvider(name = "cases") + public Object[][] cases() { + return new Object[][] { + { new ConnectionClosedException("Premature end of chunk coded message body: closing chunk expected"), true }, + { new IOException("close failed", new ConnectionClosedException("closing chunk expected")), true }, + { new IOException("disk full"), false }, + { null, false }, + }; + } + + @Test(dataProvider = "cases") + public void recognisesHcConnectionClosed(Throwable t, boolean expected) { + assertEquals(AbstractBinaryFormatReader.isConnectionClosedException(t), expected); + } +} diff --git a/examples/jdbc/src/main/java/com/clickhouse/examples/jdbc/Issue2361Repro.java b/examples/jdbc/src/main/java/com/clickhouse/examples/jdbc/Issue2361Repro.java deleted file mode 100644 index a3e0c5743..000000000 --- a/examples/jdbc/src/main/java/com/clickhouse/examples/jdbc/Issue2361Repro.java +++ /dev/null @@ -1,127 +0,0 @@ -package com.clickhouse.examples.jdbc; - -import java.io.PrintWriter; -import java.io.StringWriter; -import java.sql.Connection; -import java.sql.DriverManager; -import java.sql.ResultSet; -import java.sql.SQLException; -import java.sql.Statement; -import java.util.Properties; -import java.util.concurrent.atomic.AtomicInteger; - -/** - * Reproducer for issue #2361 — ConnectionClosedException: "Premature end of - * chunk coded message body: closing chunk expected" during ResultSet.close(). - * - *

Recipe: - *

    - *
  1. Configure the JDBC connection with LZ4 HTTP compression on - * ({@code compress=true&client.use_http_compression=true}) and a - * short server-side {@code send_timeout=1} so the server abandons - * writes after one second of back-pressure.
    2. - *
  2. Force the server to commit to chunked HTTP encoding by buffering - * the full response: {@code http_response_buffer_size=104857600} - * and {@code wait_end_of_query=1}.
    3. - *
  3. Issue a query whose result is large enough to overflow the buffer - * and that a slow client cannot drain in time.
    4. - *
  4. Read rows slowly (a few {@code Thread.sleep}s per thousand rows).
    5. - *
- * - *

The server hits {@code SOCKET_TIMEOUT} writing the chunked body, closes - * the connection without emitting the terminating zero-length chunk, and - * the driver's stream-drain on close trips the exception. - * - *

Run with: {@code java -DchUrl=jdbc:ch://localhost:8123 com.clickhouse.examples.jdbc.Issue2361Repro [iters]} - * - *

The fix in {@code ResultSetImpl.close()} downgrades this drain-time - * failure to a debug log instead of a thrown SQLException — close() should - * never punish callers for a server-side teardown race after iteration is done. - */ -public class Issue2361Repro { - - // 5M rows × ~4.5KB pad ≈ 22 GB uncompressed (~700 MB lz4-compressed). - // Large enough that send_timeout=1 + slow client reliably triggers the bug - // in a freshly-started server (before its memory tracker fills with prior - // query state). - static final String QUERY = - "SELECT number, repeat('xyz', 1500) AS pad FROM numbers(5000000)"; - - public static void main(String[] args) throws Exception { - Class.forName("com.clickhouse.jdbc.Driver"); - - String baseUrl = System.getProperty("chUrl", "jdbc:ch://localhost:8123") + "/default"; - int iters = args.length > 0 ? Integer.parseInt(args[0]) : 3; - - Properties props = new Properties(); - props.setProperty("user", System.getProperty("chUser", "default")); - props.setProperty("password", System.getProperty("chPassword", "")); - // Compression on — same lz4-framed-over-HTTP path observed in the bug. - props.setProperty("compress", "true"); - props.setProperty("client.use_http_compression", "true"); - // Long socket timeout so we don't bail out before the bug surfaces. - props.setProperty("socket_timeout", "60000"); - // Trigger conditions: - props.setProperty("clickhouse_setting_send_timeout", "1"); - props.setProperty("clickhouse_setting_http_response_buffer_size", "104857600"); - props.setProperty("clickhouse_setting_wait_end_of_query", "1"); - props.setProperty("clickhouse_setting_max_execution_time", "120"); - - AtomicInteger trips = new AtomicInteger(); - String firstStack = null; - - for (int i = 0; i < iters; i++) { - long t0 = System.currentTimeMillis(); - try (Connection c = DriverManager.getConnection(baseUrl, props); - Statement s = c.createStatement(); - ResultSet rs = s.executeQuery(QUERY)) { - - int rows = 0; - try { - while (rs.next()) { - rs.getString(2); - rows++; - // Slow client: ~1 s per ~1000 rows → 5,000 s to fully - // drain. Server send_timeout=1 will fire well before then. - if (rows % 100 == 0) Thread.sleep(50); - } - } catch (SQLException re) { - // Mid-iteration errors land here; the bug we want is the - // try-with-resources close that fires after the catch. - } - } catch (Throwable t) { - if (hasInMsg(t, "Premature end of chunk")) { - trips.incrementAndGet(); - if (firstStack == null) firstStack = stack(t); - } - } - System.out.printf("iter %d: trips=%d elapsed=%dms%n", - i, trips.get(), System.currentTimeMillis() - t0); - System.out.flush(); - } - - System.out.printf("%nFINAL: trips=%d / %d iterations (%.0f%%)%n", - trips.get(), iters, 100.0 * trips.get() / iters); - if (firstStack != null) { - System.out.println("\n-- first failure stack (top frames) --"); - String[] lines = firstStack.split("\n"); - for (int li = 0; li < Math.min(24, lines.length); li++) { - System.out.println(lines[li]); - } - } - } - - private static boolean hasInMsg(Throwable t, String needle) { - while (t != null) { - if (t.getMessage() != null && t.getMessage().contains(needle)) return true; - t = t.getCause(); - } - return false; - } - - private static String stack(Throwable t) { - StringWriter sw = new StringWriter(); - t.printStackTrace(new PrintWriter(sw)); - return sw.toString(); - } -} diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ResultSetImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ResultSetImpl.java index af29813c4..e57282967 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ResultSetImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ResultSetImpl.java @@ -149,18 +149,6 @@ public boolean next() throws SQLException { public void close() throws SQLException { closed = true; - // Stream-close exceptions on the response body are expected when the server has - // already torn down the connection (e.g. SOCKET_TIMEOUT on the writer side hits - // `send_timeout` before the terminating chunk is written). The most common - // surface is `ConnectionClosedException: Premature end of chunk coded message - // body: closing chunk expected` from Apache HC's `ChunkedInputStream` drain, - // optionally wrapped through `FramedLZ4CompressorInputStream` when the - // response was lz4-compressed. The application has already finished iterating - // the result set; propagating the drain failure as a SQLException punishes - // well-behaved try-with-resources callers for a server-side socket race they - // cannot affect. Log at debug and swallow -- the connection is closed either - // way, and we should not turn `try (ResultSet rs = ...)` into a throwing path. - // See https://github.com/ClickHouse/clickhouse-java/issues/2361 Exception e = null; try { if (reader != null) { @@ -168,9 +156,7 @@ public void close() throws SQLException { reader.close(); } catch (Exception re) { log.debug("Error closing reader", re); - if (!isStreamDrainException(re)) { - e = re; - } + e = re; } finally { reader = null; } @@ -181,9 +167,7 @@ public void close() throws SQLException { response.close(); } catch (Exception re) { log.debug("Error closing response", re); - if (!isStreamDrainException(re)) { - e = re; - } + e = re; } finally { response = null; } @@ -196,33 +180,6 @@ public void close() throws SQLException { } } - /** - * Determines whether a close-time exception originates from draining an - * already-truncated HTTP chunked response body. Such exceptions are not - * actionable from the caller's perspective -- iteration has finished, the - * connection is closing anyway, and the server's premature disconnect is the - * actual error condition (which, if it occurred during iteration, would have - * surfaced through {@code next()}). - */ - // Package-private for unit tests. - static boolean isStreamDrainException(Throwable t) { - while (t != null) { - String msg = t.getMessage(); - if (msg != null && (msg.contains("Premature end of chunk") - || msg.contains("closing chunk expected"))) { - return true; - } - // ConnectionClosedException + class name match for defensive coverage of - // future HC versions that may rephrase the message. - String cls = t.getClass().getName(); - if (cls.endsWith("ConnectionClosedException")) { - return true; - } - t = t.getCause(); - } - return false; - } - @Override public boolean wasNull() throws SQLException { checkClosed(); diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ResultSetImplCloseTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ResultSetImplCloseTest.java deleted file mode 100644 index 67ba18423..000000000 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ResultSetImplCloseTest.java +++ /dev/null @@ -1,72 +0,0 @@ -package com.clickhouse.jdbc; - -import org.testng.annotations.Test; - -import java.io.IOException; - -import static org.testng.Assert.assertFalse; -import static org.testng.Assert.assertTrue; - -/** - * Unit tests for ResultSetImpl.isStreamDrainException — the classifier that - * decides whether a close-time exception is a benign chunked-stream drain - * failure (to swallow) or a real error (to propagate). - * - * Background: see issue #2361. When the server's send_timeout fires mid-write, - * Apache HC's ChunkedInputStream.close() trips - * `ConnectionClosedException: Premature end of chunk coded message body: - * closing chunk expected`. That happens during ResultSetImpl.close(), AFTER - * the application has finished iterating. Propagating it punishes well-behaved - * try-with-resources callers for a server-side socket race they cannot affect. - */ -public class ResultSetImplCloseTest { - - @Test - public void prematureEndOfChunkIsDrainException() { - // The canonical surface from issue #2361. - Exception e = new IOException("Premature end of chunk coded message body: closing chunk expected"); - assertTrue(ResultSetImpl.isStreamDrainException(e)); - } - - @Test - public void closingChunkExpectedIsDrainException() { - // Forward-compat for variations of the same message. - Exception e = new IOException("...closing chunk expected"); - assertTrue(ResultSetImpl.isStreamDrainException(e)); - } - - @Test - public void wrappedCausesAreUnwrapped() { - // Stack from the actual JDBC trace: - // SQLException -> ClientException -> ConnectionClosedException - Throwable root = new FakeConnectionClosedException("Premature end of chunk coded message body: closing chunk expected"); - RuntimeException mid = new RuntimeException("Failed to close response", root); - Exception top = new Exception("wrapped", mid); - assertTrue(ResultSetImpl.isStreamDrainException(top)); - } - - @Test - public void classNameMatchEvenWithoutMessage() { - // Defensive: if a future HC version drops the descriptive message but - // keeps the exception class, we still want to classify it as drain noise. - Throwable e = new FakeConnectionClosedException(null); - assertTrue(ResultSetImpl.isStreamDrainException(e)); - } - - @Test - public void unrelatedExceptionsArePropagated() { - assertFalse(ResultSetImpl.isStreamDrainException(new IOException("disk full"))); - assertFalse(ResultSetImpl.isStreamDrainException(new IllegalStateException("bad state"))); - assertFalse(ResultSetImpl.isStreamDrainException(new RuntimeException("anything else"))); - } - - @Test - public void nullThrowableHandled() { - assertFalse(ResultSetImpl.isStreamDrainException(null)); - } - - /** Mimics org.apache.hc.core5.http.ConnectionClosedException for the class-name match. */ - private static final class FakeConnectionClosedException extends IOException { - FakeConnectionClosedException(String msg) { super(msg); } - } -}