----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34050/#review83278 -----------------------------------------------------------
Thanks for the patch. A couple of comments blow. core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala <https://reviews.apache.org/r/34050/#comment134178> This method throws exception instead of RuntimeException. core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/34050/#comment134183> We register two StateListeners on the same zkclient instance in the broker. If we can't establish a new ZK session, both listeners will be called. However, we only need to exit in one of the listners. So, we can just do the logging and exit in handleSessionEstablishedmentError() in KafkaHealthcheck and add a comment in the listener in KafkaController that the actual logic is done in the other listener. Ditto to the two listeners in the consumer. - Jun Rao On May 11, 2015, 6:34 p.m., Parth Brahmbhatt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34050/ > ----------------------------------------------------------- > > (Updated May 11, 2015, 6:34 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2169 > https://issues.apache.org/jira/browse/KAFKA-2169 > > > Repository: kafka > > > Description > ------- > > System.exit instead of throwing RuntimeException when zokeeper session > establishment fails. > > > Diffs > ----- > > build.gradle fef515b3b2276b1f861e7cc2e33e74c3ce5e405b > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala > aa8d9404a3e78a365df06404b79d0d8f694b4bd6 > core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala > 38f4ec0bd1b388cc8fc04b38bbb2e7aaa1c3f43b > core/src/main/scala/kafka/controller/KafkaController.scala > a6351163f5b6f080d6fa50bcc3533d445fcbc067 > core/src/main/scala/kafka/server/KafkaHealthcheck.scala > 861b7f644941f88ce04a4e95f6b28d18bf1db16d > > Diff: https://reviews.apache.org/r/34050/diff/ > > > Testing > ------- > > > Thanks, > > Parth Brahmbhatt > >
