Skip to content

fix: Fix connection slot leak when context is cancelled during acquire#1759

Merged
kavirajk merged 10 commits intoClickHouse:mainfrom
hermanschaaf:fix-connection-pooling-bug
Mar 4, 2026
Merged

fix: Fix connection slot leak when context is cancelled during acquire#1759
kavirajk merged 10 commits intoClickHouse:mainfrom
hermanschaaf:fix-connection-pooling-bug

Conversation

@hermanschaaf
Copy link
Copy Markdown
Contributor

Fixes a connection pool leak that happens when a query context is cancelled while acquiring a connection. When acquire() gets a connection slot from the ch.open channel but then ch.idle.Get(ctx) returns a context cancellation error, we were returning immediately without releasing that slot. After enough cancelled queries, the pool would be exhausted even though connections were available.

This fix makes sure the slot gets released. It also adds a test that reproduces the previous issue and shows that is now fixed.

@kavirajk kavirajk self-assigned this Jan 23, 2026
// eventually exhaust the connection pool.
func TestContextCancellationNoConnectionSlotLeak(t *testing.T) {
TestProtocols(t, func(t *testing.T, protocol clickhouse.Protocol) {
SkipOnHTTP(t, protocol, "context cancel slot leak test")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, why do we skip on this on HTTP? both TCP and HTTP uses pooling via these acquire() api.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kavirajk Good point. Updated 👍

@kavirajk kavirajk added Q1-FY-2026 Priorities for Q1 FY 2026 bug labels Mar 3, 2026
in dial_http, we modify the passed in options in-plce. Which restricts
(and cause data race) when end up using multiple go-routines

This PR uses just the local copy of the options's compression and scheme during dial_http

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
@kavirajk kavirajk merged commit fb54f8d into ClickHouse:main Mar 4, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Q1-FY-2026 Priorities for Q1 FY 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants