Skip to content
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

avoid consumer Close() hang #61

Merged
merged 1 commit into from
Jun 5, 2018
Merged

avoid consumer Close() hang #61

merged 1 commit into from
Jun 5, 2018

Conversation

dtheodor
Copy link
Contributor

@dtheodor dtheodor commented May 23, 2018

unsubscribe and poll before closing the consumer to avoid Close()
hang

Closes #59

Workaround for confluentinc/confluent-kafka-go#189

@dtheodor dtheodor requested a review from agis May 23, 2018 14:28
@agis
Copy link
Contributor

agis commented May 23, 2018

Let's wait for confluentinc/confluent-kafka-go#189 before we proceed with this. Not sure if it's the suggested solution.

Copy link
Contributor

@agis agis left a comment

Choose a reason for hiding this comment

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

We should probably do what edenhill suggests: confluentinc/confluent-kafka-go#189 (comment).

consumer.go Outdated
err := c.consumer.Close()
// need to unassign to be sure the following Close()
// will return
// https://github.com/confluentinc/confluent-kafka-go/issues/189
Copy link
Contributor

Choose a reason for hiding this comment

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

Better add an explicit TODO so that we're reminded to get rid of this when the issue is fixed.

unsubscribe and poll before closing the consumer to avoid Close()
hang

Closes #59
@dtheodor dtheodor changed the title unassign before closing the consumer to avoid Close() hang avoid consumer Close() hang Jun 5, 2018
@dtheodor
Copy link
Contributor Author

dtheodor commented Jun 5, 2018

PR ready

@dtheodor dtheodor merged commit 76a0abd into master Jun 5, 2018
@dtheodor dtheodor deleted the fix-exit-hang branch June 5, 2018 10:33
agis added a commit that referenced this pull request May 24, 2019
This enables us to revert the workaround introduced in #61, which was
needed because of confluentinc/confluent-kafka-go#189.

Since this bug is fixed in librdkafka 0.11.5, so we can now remove our
workaround.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Close() may hung indefinitely
2 participants