-
Notifications
You must be signed in to change notification settings - Fork 14.8k
KAFKA-19394: Failure in ConsumerNetworkThread.initializeResources() can cause hangs on AsyncKafkaConsumer.close() #20792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
…an cause hangs on AsyncKafkaConsumer.close() WIP
Use ConsumerUtils.maybeWrapAsKafkaException to possibly reduce an extra layer of exception handling.
|
A label of 'needs-attention' was automatically added to this PR in order to raise the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice fix!
I modified an AsyncKafkaConsumer unit test on trunk to use your invalid login module approach (and a non-mocked ApplicationEventHandler) to cause the network thread to fail and observed that the consumer hung.
I then used the same test on your branch to verify that the fix prevented this.
Only feedback is that it might be good to have a unit test at the AsyncKafkaConsumerTest level along the lines of one for KafkaConsumerTest? But this isn't blocking feedback.
ApplicationEventHandlernow waits for theConsumerNetworkThreadtostart up and complete execution of
initializeResources(). If resourceinitialization fails, the
AsyncKafkaConsumerconstructor will throw anexception. This mimics the behavior of
ClassicKafkaConsumer.