Skip to content

[jdbc-v2] Reproduce socket timeout loss after connection validation - #3262

Closed
ssouris-te wants to merge 1 commit into
ClickHouse:mainfrom
ssouris-te:codex/reproduce-jdbc-network-timeout
Closed

ssouris-te wants to merge 1 commit into
ClickHouse:mainfrom
ssouris-te:codex/reproduce-jdbc-network-timeout

Conversation

@ssouris-te

@ssouris-te ssouris-te commented Oct 29, 2026 •

Copy link
Copy Markdown

Summary

Adds one reproduction test for #3260. This draft contains only the test, with no production code changes or proposed fix.

The test configures socket_timeout=500 ms and performs the timeout sequence used by connection pools: save getNetworkTimeout(), temporarily set 5000 ms, then restore the saved value. A local HTTP server returns a valid response to SELECT 1 after 1800 ms.

The configured 500 ms timeout should still apply after restoration. On current upstream, the saved value is zero, restoring it overrides the configured socket timeout, and the delayed query succeeds instead of timing out.

The test intentionally fails on upstream. This draft is for confirming the bug and agreeing on the intended behavior before proposing a fix. It does not close the issue.

Validation

Run from the repository root on JDK 21 (the toolchain-enforcement profile is disabled for this local environment):

mvn -B -ntp -P'!build11' -pl jdbc-v2 -am verify \
  -DskipITs -Dtest=ConnectionNetworkTimeoutTest \
  -Dsurefire.failIfNoSpecifiedTests=false

Observed result: 1 test, 1 expected failure, 0 errors, 0 skipped:

ConnectionNetworkTimeoutTest.testRestoringNetworkTimeoutPreservesConfiguredSocketTimeout
Expected SQLException to be thrown, but nothing was thrown

The test uses the real JDBC driver and a loopback WireMock server. It requires no running ClickHouse server or Docker. The response is delayed for a bounded period, the test has a 15-second limit, and JDBC/server resources are closed. Read-only Codex review found no actionable issues with the reproduction.

Checklist

  • Linked the existing issue without closing it.
  • Added a focused reproduction through JDBC and the actual HTTP transport.
  • Ran the test locally and confirmed the expected failure.
  • Kept this draft limited to one test file; no production, dependency, or documentation changes.

Note

Low Risk
Test-only change with no runtime behavior modifications; the new test is expected to fail on current upstream.

Overview
Adds ConnectionNetworkTimeoutTest, a WireMock-based integration test for issue #3260. There are no production or dependency changes—only this test.

The test sets socket_timeout=500, then runs the sequence connection pools use before validation: read getNetworkTimeout(), temporarily setNetworkTimeout(..., 5000), and restore the saved value. It then runs SELECT 1 against a stub that responds after 1800 ms with a valid RowBinary payload.

Expected behavior: after restore, the 500 ms socket timeout still applies and the query throws SQLException with a SocketTimeoutException root cause.

Current upstream: the saved timeout is 0, so restore clears the configured limit and the delayed query succeeds—the test is written to fail until a fix lands.

Reviewed by Cursor Bugbot for commit 4d970c2. Bugbot is set up for automated code reviews on this repo. Configure here.

Add a delayed-response test for the JDBC timeout save/set/restore sequence used by pools.
The test intentionally fails on upstream because restoring the reported zero timeout disables
the configured socket timeout. Keep the reproduction separate from any proposed fix.

Refs ClickHouse#3260
Co-authored-by: codex (0.153.2)
@ssouris-te
ssouris-te marked this pull request as ready for review October 29, 2026 11:40
@chernser

Copy link
Copy Markdown
Contributor

@ssouris-te thank you for the code!
We will create another PR and fix the issue

@polyglotAI-bot please take this PR as base and fix the issue. Original tests should be preserved and new tests should be added.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Stathis Souris seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@chernser

chernser commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

move to #3266
Closing

@chernser chernser closed this Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants