wvanbergen / kafka

Load-balancing, resuming Kafka consumer for go, backed by Zookeeper.
MIT License
373 stars 141 forks source link

Handle reconnects #99

Closed warebot closed 8 years ago

warebot commented 8 years ago

@wvanbergen

Re: issue #95 and #81

Clearly this a work in progress. I tested this approach locally and it seems to do the job. The idea is to "reconnect" the consumption for a topic+partition when encountering upstream failures e.g. sarama library closing channel.

Would like to get your initial thoughts. (clearly error handling would be added/implemented in the final commit)

wvanbergen commented 8 years ago

I think this makes sense (even though I still don't understand why we trigger the nil messages on channels situation).

What I'd also like to see is a log line that tells the user the consumer will reconnect.

warebot commented 8 years ago

We should definitely log this scenario when it occurs.

Based on logs (will re:run and share), this usually happens when the consumer erros out on requests e.g. offset requested being out of bounds (which can happen in a troubled cluster).

As you mentioned in the previous issue, if the upstream consumer closes the channel for whatever reason, all poll requests from the channel will result in nil objects being consumed.

warebot commented 8 years ago

@wvanbergen Been running strong for the past few hours and handling re-connects as expected. Below are some logs. Let me know your thoughts.

[Sarama] 2016/06/16 10:46:32 kafka: error while consuming prometrics/29: kafka server: The requested offset is outside the range of offsets maintained by the server for the given topic/partition. [Sarama] 2016/06/16 10:46:32 consumer/prometrics/29 shutting down because kafka server: The requested offset is outside the range of offsets maintained by the server for the given topic/partition. [Sarama] 2016/06/16 10:46:32 [prometrics_test_group/710bd94a996c] prometrics/29 :: Consumer encountered an invalid state: re-establishing consumption of partition. [Sarama] 2016/06/16 10:46:33 [prometrics_test_group/710bd94a996c] prometrics/29 :: Partition consumer offset out of Range. [Sarama] 2016/06/16 10:46:33 [prometrics_test_group/710bd94a996c] prometrics/29 :: Partition consumer offset reset to oldest available offset. [Sarama] 2016/06/16 10:46:34 consumer/broker/10015 added subscription to prometrics/29

wvanbergen commented 8 years ago

I think this looks good. Is it still a WIP, or can I merge?

warebot commented 8 years ago

Unless you have further changes/requests or concerns, good to go on my end.

On Jun 17, 2016, at 8:29 AM, Willem van Bergen notifications@github.com wrote:

I think this looks good. Is it still a WIP, or can I merge?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/wvanbergen/kafka/pull/99#issuecomment-226756558, or mute the thread https://github.com/notifications/unsubscribe/ACYDqe7pRGYguB2WXJvCa8mqgvQZpoLRks5qMpNFgaJpZM4I20h4.