Fix leaving connection in a broken state after preliminary cancellation distributed queries#93029
Merged
azat merged 1 commit intoClickHouse:masterfrom Dec 25, 2025
Merged
Conversation
…on distributed queries The problem is that some places in the code (i.e. RemoteSource::onUpdatePorts()), can call finish() even before sendQueryAsync() finishes sending the query, and so if after it will try to call finish() again (after sendQueryAsync() finishes) it will be no-op. The problem pops up after ClickHouse#92807, since RemoteSource has these (anti-)pattern.
Contributor
|
Workflow [PR], commit [9ac4c8b] Summary: ❌
|
azat
force-pushed
the
fix-preliminary-dist-query-finish
branch
from
f7243d1 to
9ac4c8b
Compare
clickhouse-gh
bot
added
the
pr-bugfix
This was referenced Dec 24, 2025
rienath
approved these changes
Dec 24, 2025
Member
rienath
left a comment
rienath
left a comment
There was a problem hiding this comment.
Makes sense, thanks for the quick fix and great explanation!
Merged
via the queue into
ClickHouse:master
with commit Dec 25, 2025
076f883
126 of 131 checks passed
robot-clickhouse
added a commit
that referenced
this pull request
Dec 25, 2025
Cherry pick #93029 to 25.8: Fix leaving connection in a broken state after preliminary cancellation distributed queries
robot-clickhouse
added a commit
that referenced
this pull request
Dec 25, 2025
…er preliminary cancellation distributed queries
robot-clickhouse
added a commit
that referenced
this pull request
Dec 25, 2025
Cherry pick #93029 to 25.10: Fix leaving connection in a broken state after preliminary cancellation distributed queries
robot-clickhouse
added a commit
that referenced
this pull request
Dec 25, 2025
…ter preliminary cancellation distributed queries
robot-clickhouse
added a commit
that referenced
this pull request
Dec 25, 2025
Cherry pick #93029 to 25.11: Fix leaving connection in a broken state after preliminary cancellation distributed queries
robot-clickhouse
added a commit
that referenced
this pull request
Dec 25, 2025
…ter preliminary cancellation distributed queries
robot-clickhouse
added a commit
that referenced
this pull request
Dec 25, 2025
Cherry pick #93029 to 25.12: Fix leaving connection in a broken state after preliminary cancellation distributed queries
robot-clickhouse
added a commit
that referenced
this pull request
Dec 25, 2025
…ter preliminary cancellation distributed queries
robot-ch-test-poll4
added
the
pr-must-backport-synced
robot-ch-test-poll
added
the
pr-backports-created
clickhouse-gh bot
added a commit
that referenced
this pull request
Dec 25, 2025
Backport #93029 to 25.11: Fix leaving connection in a broken state after preliminary cancellation distributed queries
clickhouse-gh bot
added a commit
that referenced
this pull request
Dec 25, 2025
Backport #93029 to 25.12: Fix leaving connection in a broken state after preliminary cancellation distributed queries
azat
added a commit
that referenced
this pull request
Dec 25, 2025
Backport #93029 to 25.10: Fix leaving connection in a broken state after preliminary cancellation distributed queries
azat
added a commit
that referenced
this pull request
Dec 25, 2025
Backport #93029 to 25.8: Fix leaving connection in a broken state after preliminary cancellation distributed queries
AVMusorin
pushed a commit
to AVMusorin/ClickHouse
that referenced
this pull request
Jan 28, 2026
The race condition causes a crash when reading from a disconnected
connection in distributed queries.
The `in` buffer:
```
(gdb) frame 1
(gdb) print this->in
$19 = {__ptr_ = 0x0, __cntrl_ = 0x0}
(gdb) print this->connected
$18 = false
```
But the same time:
```
(gdb) frame 2
async_callback=...) at ./ci/tmp/build/./src/Client/MultiplexedConnections.cpp:398
(gdb) print this->sent_query
$15 = true
(gdb) print this->cancelled
$16 = false
(gdb) print this->active_connection_count
$17 = 1
```
Current solution will not fix the issue, it just apply a check like in
`sendCancel()`.
Related PRs:
- ClickHouse#89740
- ClickHouse#92807
- ClickHouse#93029
batkovic75
pushed a commit
to batkovic75/ClickHouse
that referenced
this pull request
Jan 30, 2026
The race condition causes a crash when reading from a disconnected
connection in distributed queries.
The `in` buffer:
```
(gdb) frame 1
(gdb) print this->in
$19 = {__ptr_ = 0x0, __cntrl_ = 0x0}
(gdb) print this->connected
$18 = false
```
But the same time:
```
(gdb) frame 2
async_callback=...) at ./ci/tmp/build/./src/Client/MultiplexedConnections.cpp:398
(gdb) print this->sent_query
$15 = true
(gdb) print this->cancelled
$16 = false
(gdb) print this->active_connection_count
$17 = 1
```
Current solution will not fix the issue, it just apply a check like in
`sendCancel()`.
Related PRs:
- ClickHouse#89740
- ClickHouse#92807
- ClickHouse#93029
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Fix leaving connection in a broken state after preliminary cancellation distributed queries
The problem is that some places in the code (i.e.
RemoteSource::onUpdatePorts()), can call finish() even before sendQueryAsync() finishes sending the query, and so if after it will try to call finish() again (after sendQueryAsync() finishes) it will be no-op.
The problem pops up after #92807, since
RemoteSourcehas these (anti-)pattern.Fixes: #92807
Fixes: #93018