Re: Review Request 35820: Patch for KAFKA-1367

2015-07-07 Thread Ashish Singh
> On July 7, 2015, 5:53 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/controller/KafkaController.scala, lines 898-903 > > > > > > I think we need to add the de-registration functions as other listeners >

Re: Review Request 35820: Patch for KAFKA-1367

2015-07-07 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35820/#review90734 --- core/src/main/scala/kafka/controller/KafkaController.scala (lines 8

Re: Review Request 35820: Patch for KAFKA-1367

2015-07-07 Thread Ashish Singh
> On July 7, 2015, 4:43 p.m., Jun Rao wrote: > > +1. Will fix the following during commit. Thanks Jun! - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35820/#review90717

Re: Review Request 35820: Patch for KAFKA-1367

2015-07-07 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35820/#review90717 --- Ship it! +1. Will fix the following during commit. core/src/main/

Re: Review Request 35820: Patch for KAFKA-1367

2015-07-06 Thread Ashish Singh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35820/#review90630 --- core/src/main/scala/kafka/controller/KafkaController.scala (line 42

Re: Review Request 35820: Patch for KAFKA-1367

2015-07-06 Thread Ashish Singh
> On July 7, 2015, 12:36 a.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala, line 50 > > > > > > This seems to be specific to testIsrAfterBrokerShutDownAndJoinsBack. > > Could w

Re: Review Request 35820: Patch for KAFKA-1367

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

Re: Review Request 35820: Patch for KAFKA-1367

2015-07-06 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35820/#review90605 --- Thanks for the patch. A few more comments below. core/src/main/sca

Re: Review Request 35820: Patch for KAFKA-1367

2015-07-01 Thread Ashish Singh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35820/ --- (Updated July 2, 2015, 12:23 a.m.) Review request for kafka. Bugs: KAFKA-1367

Re: Review Request 35820: Patch for KAFKA-1367

2015-07-01 Thread Ashish Singh
> On June 30, 2015, 4:42 p.m., Jun Rao wrote: > > Thanks for the patch. A few comments below. > > > > Also, could we add a unit test for this? Thanks for the review Jun! Addressed your concerns and added a test that re-produces the issue and verifies the fix. - Ashish -

Re: Review Request 35820: Patch for KAFKA-1367

2015-06-30 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35820/#review89911 --- Thanks for the patch. A few comments below. Also, could we add a un

Re: Review Request 35820: Patch for KAFKA-1367

2015-06-23 Thread Ashish Singh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35820/ --- (Updated June 24, 2015, 5:10 a.m.) Review request for kafka. Bugs: KAFKA-1367

Review Request 35820: Patch for KAFKA-1367

2015-06-23 Thread Ashish Singh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35820/ --- Review request for kafka. Bugs: KAFKA-1367 https://issues.apache.org/jira/b