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

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.

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 Ashish Singh
On July 7, 2015, 5:53 p.m., Guozhang Wang wrote: core/src/main/scala/kafka/controller/KafkaController.scala, lines 898-903 https://reviews.apache.org/r/35820/diff/3/?file=1000895#file1000895line898 I think we need to add the de-registration functions as other listeners upon

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.

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 https://reviews.apache.org/r/35820/diff/2/?file=997781#file997781line50 This seems to be specific to testIsrAfterBrokerShutDownAndJoinsBack. Could we move it

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 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

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:

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

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

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: