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

Raise NotNotReadyError from KafkaConsumer.poll after a retry timeout if the KafkaBroker is not available #1000

Closed
wants to merge 49 commits into from

Conversation

zembunia
Copy link

@zembunia zembunia commented Mar 1, 2017

KafkaConsumer.poll never returns in case of no connection with the KafkaBroker. This exception gives a chance to the outer program to know about the problem.

@zembunia
Copy link
Author

zembunia commented Mar 1, 2017

Please only review the comparison of the files because the intermediate 48 commits are for getting to know what to change.

@dpkp
Copy link
Owner

dpkp commented Mar 1, 2017

you know there's always git rebase -i master -- squash those commits :)

@dpkp
Copy link
Owner

dpkp commented Mar 1, 2017

I appreciate the PR, but I'm not a fan of this change. We try to stay fairly consistent with the java client configuration options so I dont like to add new options that dont exist upstream unless absolutely necessary.

It looks like the problem you are trying to solve is that ensure_coordinator_known is blocking and does not support a timeout. Have you checked how the java client handles this? Perhaps a simpler solution would be to add an optional timeout to the function itself and check it during the while loop. Is there a particular upstream function call that you are having problems with vis-a-vis blocking?

@zembunia
Copy link
Author

zembunia commented Mar 2, 2017

Java client has the same behavior reported as an issue. (https://issues.apache.org/jira/browse/KAFKA-1894) I will also test it with the same scenario.

To get the messages in a topic I use KafkaConsumer.poll and it never returns if the broker is not there. How can I understand it is not there?
I have seen that iterator interface also calls ``ensure_coordinator_known``` which will also block.

@zembunia
Copy link
Author

zembunia commented Mar 9, 2017

Java consumer is has a "consumer.timeout.ms" configuration option which lets ConsumerIterator.hasNext() to timeout and raise ConsumerTimeoutException. Its default value is -1 so that it never times out.

@jeffwidman
Copy link
Contributor

@zembunia if you're still interested in this PR, can you rebase and cleanup the commit history (see comment above about squashing) so that it's easier to review/discuss?

@zembunia
Copy link
Author

#1209 replaces this PR

@zembunia zembunia closed this Sep 11, 2017
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.

3 participants