> On May 2, 2014, 12:26 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/utils/ShutdownableThread.scala, lines 35-36 > > <https://reviews.apache.org/r/20745/diff/5/?file=573213#file573213line35> > > > > We probably should check the return value of initiateShutdown().
I was thinking that even with concurrent calls since all calls end up just waiting on the shutdownLatch it should be safe to not check it. Therefore shutdown call regardless of what state will end up waiting until the thread shutdown is completed. > On May 2, 2014, 12:26 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/utils/ShutdownableThread.scala, lines 40-41 > > <https://reviews.apache.org/r/20745/diff/5/?file=573213#file573213line40> > > > > Instead of introducing isShuttingDown, could we just to compareAndSet > > on isRunning? Good point, consolidated it. - Timothy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20745/#review41979 ----------------------------------------------------------- On May 2, 2014, 1:12 a.m., Timothy Chen wrote: > > ----------------------------------------------------------- > 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 > https://issues.apache.org/jira/browse/KAFKA-1397 > > > Repository: kafka > > > Description > ------- > > KAFKA-1397: Fix delete topic tests and deadlock > > > Diffs > ----- > > core/src/main/scala/kafka/admin/AdminUtils.scala > 36ddeb44490e8343a4e8056c45726ac660e4b2f9 > core/src/main/scala/kafka/controller/ControllerChannelManager.scala > 919aeb26f93d2fc34d873cfb3dbfab7235a9d635 > core/src/main/scala/kafka/controller/KafkaController.scala > 933de9dd324c7086efe6aa610335ef370d9e9c12 > core/src/main/scala/kafka/controller/ReplicaStateMachine.scala > 0e47dac8cbf65a86d053a3371a18af467afd70ae > core/src/main/scala/kafka/controller/TopicDeletionManager.scala > e4bc2439ce1933c7c7571d255464ee678226a6cb > core/src/main/scala/kafka/log/LogManager.scala > ac67f081e6219fd2181479e7a2bb88ea6044e6cc > core/src/main/scala/kafka/server/ReplicaManager.scala > 11c20cee83fda9a492156674d351a0096b13fd99 > core/src/main/scala/kafka/utils/ShutdownableThread.scala > cf8adc9f468f4d6f01d1303efe39a3ec6f3d9b53 > core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala > 9c29e144bba2c9bafa91941b6ca5c263490693b3 > core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala > 5c487968014b56490eb2bc876cef1c52efd1cdad > core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala > 014e9644dba1d65142f6a9abc745858870a46230 > core/src/test/scala/unit/kafka/utils/TestUtils.scala > 49c7790c995bb2e79322eb148ab80d0dcccefed4 > > Diff: https://reviews.apache.org/r/20745/diff/ > > > Testing > ------- > > > Thanks, > > Timothy Chen > >
