Fix for the hang during deletion of engine=Kafka (one more time)#11145
Merged
alexey-milovidov merged 4 commits intoClickHouse:masterfrom May 25, 2020
Merged
Fix for the hang during deletion of engine=Kafka (one more time)#11145alexey-milovidov merged 4 commits intoClickHouse:masterfrom
alexey-milovidov merged 4 commits intoClickHouse:masterfrom
Conversation
…nother hang case, which has no straigt solutions and can be workarounded currenly only by draining the consumer queue, so destructor is back
azat
reviewed
May 22, 2020
azat
reviewed
May 23, 2020
azat
approved these changes
May 23, 2020
Member
azat
left a comment
There was a problem hiding this comment.
LGTM (squashing before merging is preferable)
But
Fix for the hang during deletion of engine=Kafka (one more time) #11145
"one more time" looks icky, let's change PR title (and the commit subject) so that it will include "RD_KAFKA_DESTROY_F_NO_CONSUMER_CLOSE" and "get back poll" or something similar
Besides commit subject is too long (I guess you missed newline between subject and description)
KochetovNicolai
pushed a commit
that referenced
this pull request
Jun 8, 2020
) * Added flag for safer rdkafka destruction, but more testing detected another hang case, which has no straigt solutions and can be workarounded currenly only by draining the consumer queue, so destructor is back * After review fixes * After review fixes2 * After review fixes3 (cherry picked from commit 73a0ef6)
filimonov
added a commit
to filimonov/ClickHouse
that referenced
this pull request
Jun 8, 2020
…ckHouse#11145) * Added flag for safer rdkafka destruction, but more testing detected another hang case, which has no straigt solutions and can be workarounded currenly only by draining the consumer queue, so destructor is back * After review fixes * After review fixes2 * After review fixes3 (cherry picked from commit 73a0ef6)
filimonov
added a commit
to filimonov/ClickHouse
that referenced
this pull request
Jun 8, 2020
…ckHouse#11145) * Added flag for safer rdkafka destruction, but more testing detected another hang case, which has no straigt solutions and can be workarounded currenly only by draining the consumer queue, so destructor is back * After review fixes * After review fixes2 * After review fixes3 (cherry picked from commit 73a0ef6)
This was referenced Mar 24, 2026
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
6 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.
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix for the hang which was happening sometimes during DROP of table engine=Kafka (or during server restarts).
Detailed description / Documentation draft:
Continuation of #10910
After #10910 it still can hang, but chances get lower and the test stopped detecting that.
librdkafka have a flag that should prevent that, so I've added it to cppkafka (mfontanini/cppkafka#247)
But after extra testing, I've found that there are scenarios when that flag don't work (confluentinc/librdkafka#2898) and we still need to get back to queue draining after unsubscribe.
Fixes #7260 #10740
Thanks to @azat for help & inspiration.