----------------------------------------------------------- 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 <https://reviews.apache.org/r/20745/#comment75675> Could we add a comment that checkBrokerAvailable is only used for testing purpose? core/src/main/scala/kafka/controller/ReplicaStateMachine.scala <https://reviews.apache.org/r/20745/#comment75680> Could you add some comments on why we need to send updateMetadata request in the case? core/src/main/scala/kafka/server/ReplicaManager.scala <https://reviews.apache.org/r/20745/#comment75697> Could we just use logManager.getLog() instead of introducing doesLogExists()? core/src/main/scala/kafka/utils/ShutdownableThread.scala <https://reviews.apache.org/r/20745/#comment75685> We probably should check the return value of initiateShutdown(). core/src/main/scala/kafka/utils/ShutdownableThread.scala <https://reviews.apache.org/r/20745/#comment75684> Instead of introducing isShuttingDown, could we just to compareAndSet on isRunning? core/src/main/scala/kafka/utils/ShutdownableThread.scala <https://reviews.apache.org/r/20745/#comment75687> lower case "Shutting". core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala <https://reviews.apache.org/r/20745/#comment75701> It may be possible to design a reliable version of the test. We can (1) create a partition with 2 replicas, (2) shutdown one of the replicas that's not the controller, (3) initiate topic deletion (which will block since 1 replica is down), (4) failed the controller, (5) bring both brokers up, (6) verify topic is deleted. core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala <https://reviews.apache.org/r/20745/#comment75702> The message should be "partition reassignment shouldn't complete". - Jun Rao On May 1, 2014, 10:54 p.m., Timothy Chen wrote: > > ----------------------------------------------------------- > 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 > 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/utils/TestUtils.scala > 49c7790c995bb2e79322eb148ab80d0dcccefed4 > > Diff: https://reviews.apache.org/r/20745/diff/ > > > Testing > ------- > > > Thanks, > > Timothy Chen > >
