> On Nov. 14, 2014, 11:36 p.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/javaapi/consumer/ConsumerRebalanceListener.java, 
> > line 29
> > <https://reviews.apache.org/r/28025/diff/1/?file=763200#file763200line29>
> >
> >     It would be worth elaborating the use-case for these hooks (i.e., in 
> > the context of mirror maker data loss and how these will be used). Do we 
> > need all of these hooks?
> 
> Jiangjie Qin wrote:
>     I checked the new consumer. The rebalance callback has 2 hooks. 
> onPartitionRevoked and onPartitionAssigned. I think we can keep them just for 
> future compatibility. It seems people potentially needs before and after hook 
> (that's why this ticket is initially created). For mirror maker we need the 
> hook after fetchers stopped and before the partition ownerships are released. 
> So it seems we still need 5 hooks...

It may be better for the purposes of code review, to explain the motivation by 
also including the patch for the mirror-maker, or a summary of how the hooks 
will be used. I'm more inclined to just have the hooks that are absolutely 
necessary. Furthermore, if the mirror-maker use-case warrants we can consider 
whether any changes/additions are required in the new consumer hooks as well.


- Joel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28025/#review61556
-----------------------------------------------------------


On Nov. 15, 2014, 9:20 a.m., Jiangjie Qin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28025/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2014, 9:20 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-345
>     https://issues.apache.org/jira/browse/KAFKA-345
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Added new unit test.
> 
> 
> Incorporated Joel's comments
> 
> 
> Incorporated Joel's comments
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala 
> f476973eeff653473a60c3ecf36e870e386536bc 
>   core/src/main/scala/kafka/javaapi/consumer/ConsumerRebalanceListener.java 
> PRE-CREATION 
>   core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 
> 1f98db5d692adc113189ec8c75a4fad29d6b6ffe 
>   
> core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala 
> e1d87112a2a587aa3a2f5875f278b276c32f45ac 
> 
> Diff: https://reviews.apache.org/r/28025/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jiangjie Qin
> 
>

Reply via email to