Review Request 20745: Patch for KAFKA-1397

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

Re: Review Request 20745: Patch for KAFKA-1397

2014-04-28 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/#review41626 --- core/src/main/scala/kafka/controller/ControllerChannelManager.scala

Re: Review Request 20745: Patch for KAFKA-1397

2014-04-28 Thread Timothy Chen
> On April 28, 2014, 7:43 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/controller/ControllerChannelManager.scala, line > > 210 > > > > > > better to not leak the logic from delete topic manager here. It is

Re: Review Request 20745: Patch for KAFKA-1397

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

Re: Review Request 20745: Patch for KAFKA-1397

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

Re: Review Request 20745: Patch for KAFKA-1397

2014-04-29 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/#review41714 --- Some comments: 1. Should we restore the commented out test in Serve

Re: Review Request 20745: Patch for KAFKA-1397

2014-04-29 Thread Jun Rao
> On April 28, 2014, 7:43 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/controller/ControllerChannelManager.scala, line > > 210 > > > > > > better to not leak the logic from delete topic manager here. It is

Re: Review Request 20745: Patch for KAFKA-1397

2014-04-30 Thread Timothy Chen
> On April 29, 2014, 5:02 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/controller/ControllerChannelManager.scala, lines > > 209-212 > > > > > > Is it better to do the check here or in the caller? Hmm, at first

Re: Review Request 20745: Patch for KAFKA-1397

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

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-01 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/#review41904 --- 1. Should we restore the commented out test in ServerShutdownTest.sc

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-01 Thread Timothy Chen
> On May 1, 2014, 6:26 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/controller/TopicDeletionManager.scala, lines > > 394-396 > > > > > > It seems this test is not necessary since the outer loop will detect > >

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-01 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/ --- (Updated May 1, 2014, 10:54 p.m.) Review request for kafka. Bugs: KAFKA-1397

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-01 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/#review41979 --- core/src/main/scala/kafka/admin/AdminUtils.scala

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-01 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/ --- (Updated May 2, 2014, 1:12 a.m.) Review request for kafka. Bugs: KAFKA-1397

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-02 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/#review42015 --- Not all comments from the previous round of review have been address

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-02 Thread Timothy Chen
> On May 2, 2014, 12:26 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/utils/ShutdownableThread.scala, lines 35-36 > > > > > > We probably should check the return value of initiateShutdown(). I was thinking that ev

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-02 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/ --- (Updated May 2, 2014, 8:38 p.m.) Review request for kafka. Bugs: KAFKA-1397

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-05 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/#review42058 --- core/src/main/scala/kafka/controller/TopicDeletionManager.scala

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-05 Thread Timothy Chen
> On May 5, 2014, 5:12 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/utils/ShutdownableThread.scala, lines 28-33 > > > > > > I thought you plan to remove isShuttingDown? Odd I don't have this anymore in my box, bu

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-05 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/ --- (Updated May 5, 2014, 6:17 p.m.) Review request for kafka. Bugs: KAFKA-1397

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-05 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/#review42195 --- core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-05 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/#review42199 --- core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala

Re: Review Request 20745: Patch for KAFKA-1397

2014-05-05 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/ --- (Updated May 5, 2014, 9 p.m.) Review request for kafka. Bugs: KAFKA-1397