Re: Review Request 19957: Patch for KAFKA-1356

2014-04-08 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/#review39840 --- Thanks for the patch. Some additional comments. 1. The introduction

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-08 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/ --- (Updated April 8, 2014, 8:38 a.m.) Review request for kafka. Bugs: KAFKA-1356

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-06 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/#review39652 --- Ship it! Ship It! - Neha Narkhede On April 6, 2014, 8:46 a.m., T

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-06 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/ --- (Updated April 6, 2014, 8:46 a.m.) Review request for kafka. Bugs: KAFKA-1356

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-05 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/#review39635 --- Just one minor comment. Other than that, it looks great! We can chec

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-04 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/ --- (Updated April 5, 2014, 12:45 a.m.) Review request for kafka. Bugs: KAFKA-135

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-04 Thread Timothy Chen
> On April 5, 2014, 12:11 a.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/api/TopicMetadata.scala, line 35 > > > > > > could you confirm why we need to fill the array vs create an empty one > > of size numPart

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-04 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/#review39618 --- core/src/main/scala/kafka/api/TopicMetadata.scala

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-04 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/#review39605 --- Ship it! Ship It! - Guozhang Wang On April 4, 2014, 9:40 p.m., T

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-04 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/ --- (Updated April 4, 2014, 9:40 p.m.) Review request for kafka. Bugs: KAFKA-1356

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-04 Thread Timothy Chen
> On April 4, 2014, 3:59 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/KafkaApis.scala, lines 619-620 > > > > > > If this is expensive, perhaps we can only do that if we require all > > topics? If that mak

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-04 Thread Jun Rao
> On April 4, 2014, 3:59 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/KafkaApis.scala, lines 619-620 > > > > > > If this is expensive, perhaps we can only do that if we require all > > topics? If that mak

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-04 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/#review39546 --- core/src/main/scala/kafka/api/TopicMetadata.scala

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-04 Thread Neha Narkhede
> On April 4, 2014, 3:59 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/KafkaApis.scala, lines 619-620 > > > > > > If this is expensive, perhaps we can only do that if we require all > > topics? If that mak

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-04 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/#review39542 --- core/src/main/scala/kafka/server/KafkaApis.scala

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-03 Thread Guozhang Wang
> On April 3, 2014, 5:12 a.m., Guozhang Wang wrote: > > Just curious: which part of the code is the main contribution to the > > handling overhead? > > Joel Koshy wrote: > Tim can correct me if I'm wrong but I think the overhead was largely in > the sort. > > Thanks again for the

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-03 Thread Timothy Chen
> On April 3, 2014, 5:12 a.m., Guozhang Wang wrote: > > Just curious: which part of the code is the main contribution to the > > handling overhead? > > Joel Koshy wrote: > Tim can correct me if I'm wrong but I think the overhead was largely in > the sort. > > Thanks again for the

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-03 Thread Timothy Chen
> On April 3, 2014, 5:12 a.m., Guozhang Wang wrote: > > Just curious: which part of the code is the main contribution to the > > handling overhead? > > Joel Koshy wrote: > Tim can correct me if I'm wrong but I think the overhead was largely in > the sort. > > Thanks again for the

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-03 Thread Joel Koshy
> On April 3, 2014, 5:12 a.m., Guozhang Wang wrote: > > Just curious: which part of the code is the main contribution to the > > handling overhead? Tim can correct me if I'm wrong but I think the overhead was largely in the sort. Thanks again for the patch. I'll get this checked in. - Joel

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-02 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/#review39402 --- Just curious: which part of the code is the main contribution to the

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-02 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/#review39387 --- Ship it! Ship It! - Joel Koshy On April 3, 2014, 1:39 a.m., Timo

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-02 Thread Joel Koshy
> On April 3, 2014, 1:13 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/TopicMetadata.scala, line 35 > > > > > > Nitpick, but I think this would be a little neater: > > (0 to numPartitions).map( ).toArra

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-02 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/ --- (Updated April 3, 2014, 1:39 a.m.) Review request for kafka. Bugs: KAFKA-1356

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-02 Thread Timothy Chen
> On April 3, 2014, 1:13 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/TopicMetadata.scala, line 35 > > > > > > Nitpick, but I think this would be a little neater: > > (0 to numPartitions).map( ).toArra

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-02 Thread Timothy Chen
> On April 3, 2014, 1:13 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/KafkaApis.scala, line 638 > > > > > > I think this really should have been > > isr.map(aliveBrokers.get).filter(_.isEmpty) this is

Re: Review Request 19957: Patch for KAFKA-1356

2014-04-02 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/#review39378 --- core/src/main/scala/kafka/api/TopicMetadata.scala

Review Request 19957: Patch for KAFKA-1356

2014-04-02 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19957/ --- Review request for kafka. Bugs: KAFKA-1356 https://issues.apache.org/jira/b