> On Nov. 14, 2014, 11:36 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/javaapi/consumer/ConsumerRebalanceListener.java, > > line 24 > > <https://reviews.apache.org/r/28025/diff/1/?file=763200#file763200line24> > > > > Can we instead just have an interface for this? We don't need to > > provide any default implementation since by default the rebalance listener > > can be null.
I was thinking about this. The reason I finally used class is that the user can just override the method they want without implementing the other methods. But an interface is probably more clean. > 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? 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... > On Nov. 14, 2014, 11:36 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/javaapi/consumer/ConsumerRebalanceListener.java, > > line 48 > > <https://reviews.apache.org/r/28025/diff/1/?file=763200#file763200line48> > > > > It seems this would be more useful if we pass in the assignment itself. > > Also, afterPartitionAssignment would be a better name. Yes, I realized this... Actually I will need the assignment for later on design in KET for topic deletion. - Jiangjie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28025/#review61556 ----------------------------------------------------------- On Nov. 15, 2014, 9:01 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:01 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 > > > 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 > >
