Fix leaving possibly broken distributed connections in the pool#93171
Closed
azat wants to merge 1 commit intoClickHouse:masterfrom
Closed
Fix leaving possibly broken distributed connections in the pool#93171azat wants to merge 1 commit intoClickHouse:masterfrom
azat wants to merge 1 commit intoClickHouse:masterfrom
Conversation
Previous fix (ClickHouse#92807) introduce another issue, due to which the connection for distributed queries can be preserved in the pool even after some exception (usually it is socket timeout), which leads to a reconnect in the next query, or even query failure. The problem was that `RemoteQueryExecutor` uses `finished` flag to decide should it call `finish()` again in `RemoteSource`, but this `finished` flag is also used in dtor of `RemoteQueryExecutor`, and if the query is not finished it is aborted, which does not work with this new logic. Let's use `got_exception_from_replica` instead, which does not affect dtor. And it is OK to call `finish()` again in `RemoteSource`, since subsequent will be no-op.
azat
force-pushed
the
fix-broken-dist-connections-v3
branch
from
210bdfb to
2a7aa41
Compare
azat
changed the title
Contributor
|
Workflow [PR], commit [2a7aa41] Summary: ❌
|
clickhouse-gh
bot
added
the
pr-bugfix
Member
Author
|
This has been fixed differently in #95466 (via |
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
2 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 broken distributed connections in the pool
Previous fix (#92807) introduce another issue, due to which the connection for distributed queries can be preserved in the pool even after some exception (usually it is socket timeout), which leads to a reconnect in the next query, or even query failure.
The problem was that
RemoteQueryExecutorusesfinishedflag to decide should it callfinish()again inRemoteSource, but thisfinishedflag is also used in dtor ofRemoteQueryExecutor, and if the query is not finished it is aborted, which does not work with this new logic.Let's use
got_exception_from_replicainstead, which does not affect dtor. And it is OK to callfinish()again inRemoteSource, since subsequent will be no-op.Fixes: #92807