Re: Review Request 31706: Patch for KAFKA-1997

2015-03-17 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/#review76815 --- Ship it! Ship It! - Guozhang Wang On March 17, 2015, 8:47 p.m.,

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-17 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/ --- (Updated March 17, 2015, 8:47 p.m.) Review request for kafka. Bugs: KAFKA-199

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-13 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/ --- (Updated March 13, 2015, 9:43 p.m.) Review request for kafka. Bugs: KAFKA-199

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-12 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/#review76226 --- Ship it! LGTM. Joel, do you want to take another look at it? - Guo

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-11 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/ --- (Updated March 12, 2015, 2:10 a.m.) Review request for kafka. Bugs: KAFKA-199

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-11 Thread Jiangjie Qin
> On March 12, 2015, 1:22 a.m., Guozhang Wang wrote: > > Hit this unit test failure, is this relevant? > > > > -- > > > > kafka.consumer.ZookeeperConsumerConnectorTest > > > testConsumerRebalanceListener FAILED > > junit.framework.AssertionFailedError: > > expected: bu

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-11 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/#review76189 --- Hit this unit test failure, is this relevant? -

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-11 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/ --- (Updated March 11, 2015, 10:20 p.m.) Review request for kafka. Bugs: KAFKA-19

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-11 Thread Jiangjie Qin
> On March 11, 2015, 8:39 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 195 > > > > > > Should we add the index as the suffix to the consumer id in the > > consumerConfig to

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-11 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/#review76121 --- LGTM overall except one potential issue on consumer metrics collidin

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-10 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/ --- (Updated March 11, 2015, 1:31 a.m.) Review request for kafka. Bugs: KAFKA-199

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-10 Thread Jiangjie Qin
> On March 10, 2015, 7:10 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 45 > > > > > > I am still a bit concerned about bounding ourselves with one producer > > per MM, becausin

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-10 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/#review75921 --- core/src/main/scala/kafka/consumer/PartitionAssignor.scala

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-09 Thread Jiangjie Qin
> On March 9, 2015, 10:34 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 668 > > > > > > Failed to send message to ... > > > > Also, it is probably better to _not_ log the

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-09 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/ --- (Updated March 10, 2015, 1:55 a.m.) Review request for kafka. Bugs: KAFKA-199

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-09 Thread Joel Koshy
> On March 6, 2015, 12:14 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala, line > > 729 > > > > > > partitionOwnerships -> topicPartitionAssignment > > Jiangjie Qin wro

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-09 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/#review75785 --- Patch does not apply. Can you rebase? core/src/main/scala/kafka/co

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-05 Thread Jiangjie Qin
> On March 6, 2015, 12:14 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/consumer/PartitionAssignor.scala, line 74 > > > > > > Why does this need to be a pool? i.e., rebalance is done while holding > > a lock.

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-05 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/ --- (Updated March 6, 2015, 4:15 a.m.) Review request for kafka. Bugs: KAFKA-1997

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-05 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/#review75409 --- Sorry this is not a thorough review but a first pass. I can dig deep

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-04 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/ --- (Updated March 4, 2015, 11:42 p.m.) Review request for kafka. Bugs: KAFKA-199

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-04 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/ --- (Updated March 4, 2015, 11:07 p.m.) Review request for kafka. Bugs: KAFKA-199

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-03 Thread Jiangjie Qin
> On March 4, 2015, midnight, Guozhang Wang wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 67 > > > > > > Are we sure that one producer io thread is sufficient for all cases? > > Jiangjie Qin wrote:

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-03 Thread Jiangjie Qin
> On March 4, 2015, 12:48 a.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 524 > > > > > > On the other hand we should avoid checking for each message as > > System.currentTimeMil

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-03 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/#review75110 --- core/src/main/scala/kafka/tools/MirrorMaker.scala

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-03 Thread Guozhang Wang
> On March 4, 2015, midnight, Guozhang Wang wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 67 > > > > > > Are we sure that one producer io thread is sufficient for all cases? > > Jiangjie Qin wrote:

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-03 Thread Jiangjie Qin
> On March 4, 2015, midnight, Guozhang Wang wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 682 > > > > > > Instead of a wrapper rebalancer, I think it is cleaner to just > > instantiate the Consume

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-03 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/ --- (Updated March 4, 2015, 12:28 a.m.) Review request for kafka. Bugs: KAFKA-199

Re: Review Request 31706: Patch for KAFKA-1997

2015-03-03 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/#review75071 --- clients/src/main/java/org/apache/kafka/clients/producer/internals/R

Review Request 31706: Patch for KAFKA-1997

2015-03-03 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31706/ --- Review request for kafka. Bugs: KAFKA-1997 https://issues.apache.org/jira/b