wvanbergen / kazoo-go

Go library to access Kafka metadata in Zookeeper
http://godoc.org/github.com/wvanbergen/kazoo-go
MIT License
100 stars 48 forks source link

Make claiming the same partition twice not an error #6

Closed horkhe closed 9 years ago

horkhe commented 9 years ago

It is perfectly fine to re-register the same consumer group instance. It is even required to do so when a number of topics an instance is subscribed for has changed. Besides the current registration pattern is rather static (an explicit list of topics) than white_list (a list of regex patterns such that all topics in the kafka cluster that match are subscribed).

EDIT: Ever since this proposal I have been browsing the Kafka documentation and source code trying to understand if the allow resubscription. And I have not found explicit mentioning of that anywhere. What I did found is a list of watches Kafka consumer sets up. To be notified of subscription changes the consumer has to set data watches on all group instances, but it does not do that. So to make the updated subscription visible a client has to deregister and register again.

So the PR title was changed to Make claiming the same partition twice not an error and the content was updated.

horkhe commented 9 years ago

@wvanbergen do you have any comments on this PR?

wvanbergen commented 9 years ago

sorry, I missed this. Will look at it tonight.

horkhe commented 9 years ago

@wvanbergen thanks!

wvanbergen commented 9 years ago

Just on question, but this looks good. Did you test it with the wvanbergen/kafka consumer by any chance?

horkhe commented 9 years ago

We evaluated wvanbergen/kafka, but it did not meet our requirements so we could not use it. There two main issues (those are issues just for us, for any other user they may be not issues at all) that did not let us use it:

wvanbergen commented 9 years ago

OK. I will merge this once I have tested this with wvanbergen/kafka to make sure it's compatible.

horkhe commented 9 years ago

I have reverted the re-registration logic and squashed my changes.

horkhe commented 9 years ago

@wvanbergen have you been able to give it a try?

wvanbergen commented 9 years ago

Looks like it's working!

wvanbergen commented 9 years ago

Looks like you didn't run go fmt but I fixed that on master

horkhe commented 9 years ago

Oops, sorry about that.