Re: Review Request 36030: Patch for KAFKA-972

2015-07-07 Thread Ashish Singh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/ --- (Updated July 8, 2015, 6:24 a.m.) Review request for kafka. Bugs: KAFKA-972

Re: Review Request 36030: Patch for KAFKA-972

2015-07-07 Thread Ashish Singh
> On July 8, 2015, 12:52 a.m., Jun Rao wrote: > > Thanks for the patch. Saw the following transient unit test failure. > > > > kafka.integration.TopicMetadataTest > > > testIsrAfterBrokerShutDownAndJoinsBack FAILED > > junit.framework.AssertionFailedError: Topic metadata is not correctly >

Re: Review Request 36030: Patch for KAFKA-972

2015-07-07 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/#review90843 --- Thanks for the patch. Saw the following transient unit test failure.

Re: Review Request 36030: Patch for KAFKA-972

2015-07-07 Thread Ashish Singh
> On July 7, 2015, 4:50 p.m., Jun Rao wrote: > > Thanks for the latest patch. Looks good. Could you rebase? Done. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/#review90721 -

Re: Review Request 36030: Patch for KAFKA-972

2015-07-07 Thread Ashish Singh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/ --- (Updated July 7, 2015, 5:42 p.m.) Review request for kafka. Bugs: KAFKA-972

Re: Review Request 36030: Patch for KAFKA-972

2015-07-07 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/#review90721 --- Thanks for the latest patch. Looks good. Could you rebase? - Jun Ra

Re: Review Request 36030: Patch for KAFKA-972

2015-07-06 Thread Ashish Singh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/ --- (Updated July 7, 2015, 6:07 a.m.) Review request for kafka. Bugs: KAFKA-972

Re: Review Request 36030: Patch for KAFKA-972

2015-07-06 Thread Ashish Singh
> On July 6, 2015, 9:47 p.m., Jun Rao wrote: > > Thanks for the patch. A few more minor comments blow. Thanks for the review again Jun! > On July 6, 2015, 9:47 p.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala, line 147 > >

Re: Review Request 36030: Patch for KAFKA-972

2015-07-06 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/#review90579 --- Thanks for the patch. A few more minor comments blow. core/src/tes

Re: Review Request 36030: Patch for KAFKA-972

2015-07-01 Thread Ashish Singh
> On July 1, 2015, 2:18 p.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala, lines > > 162-172 > > > > > > The propagation of the metadata to different brokers are independant.

Re: Review Request 36030: Patch for KAFKA-972

2015-07-01 Thread Ashish Singh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/ --- (Updated July 1, 2015, 3:06 p.m.) Review request for kafka. Bugs: KAFKA-972

Re: Review Request 36030: Patch for KAFKA-972

2015-07-01 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/#review90045 --- Thanks for the patch. Just one more comment. core/src/test/scala/u

Re: Review Request 36030: Patch for KAFKA-972

2015-07-01 Thread Ashish Singh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/ --- (Updated July 1, 2015, 8:43 a.m.) Review request for kafka. Bugs: KAFKA-972

Re: Review Request 36030: Patch for KAFKA-972

2015-07-01 Thread Ashish Singh
> On July 1, 2015, 4:37 a.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala, line 69 > > > > > > Do we need this? In tearDown(), ZookeeperTestHarness will delete all ZK > > data.

Re: Review Request 36030: Patch for KAFKA-972

2015-07-01 Thread Ashish Singh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/ --- (Updated July 1, 2015, 8:37 a.m.) Review request for kafka. Bugs: KAFKA-972

Re: Review Request 36030: Patch for KAFKA-972

2015-06-30 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/#review90005 --- Thanks for the patch. A couple more comments. core/src/test/scala/

Re: Review Request 36030: Patch for KAFKA-972

2015-06-30 Thread Ashish Singh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/ --- (Updated July 1, 2015, 1:42 a.m.) Review request for kafka. Bugs: KAFKA-972

Re: Review Request 36030: Patch for KAFKA-972

2015-06-30 Thread Ashish Singh
> On June 30, 2015, 4:14 p.m., Jun Rao wrote: > > Thanks for the patch. A few comments below. > > > > 1. Could you add a unit test for this? > > > > 2. Also, we may have a similar issue when handling the failure of the > > broker. Currently, the logic in the controller is that the controller w

Re: Review Request 36030: Patch for KAFKA-972

2015-06-30 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36030/#review89907 --- Thanks for the patch. A few comments below. 1. Could you add a unit