----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14582/#review26969 -----------------------------------------------------------
The way that we have a ZkNonReconnector and a ZkReconnector is a bit awkward. The reason that we have both of them is that for a given ZK connection, we could register multiple session expiration listeners, only one of which needs to deal with reconnect error. However, it's a bit hard for a developer to remember which listener should wire in ZkNonReconnector, instead of ZkReconnector. Not sure what's the best way to address this. One way is to create a single instance of ZkReconnector and reuse it in all session expiration listeners for the same ZK connection. In ZkReconnector, we remember the last instance of the passed-in throwable and only schedule connection tasks when we see new instances of throwable. core/src/main/scala/kafka/utils/ZkReconnector.scala <https://reviews.apache.org/r/14582/#comment52511> You are using selftype here. Should we use self or this here? core/src/main/scala/kafka/utils/ZkReconnector.scala <https://reviews.apache.org/r/14582/#comment52512> You are using selftype here. Should we use self or this here? core/src/main/scala/kafka/utils/ZkReconnector.scala <https://reviews.apache.org/r/14582/#comment52513> Could you explain why you have to use "reconnect _", instead of just "reconnect"? core/src/main/scala/kafka/utils/ZkUtils.scala <https://reviews.apache.org/r/14582/#comment52514> Let's follow the convention and use wait.ms. core/src/test/scala/unit/kafka/zk/ZkReconnecterTest.scala <https://reviews.apache.org/r/14582/#comment52515> This class seems to be just testing the reconnect logic in zkclient. Assuming there is already a unit test in zkclient itself, is there any value in including this unit test in Kafka? - Jun Rao On Oct. 10, 2013, 10:14 p.m., Anatoly Fayngelerin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14582/ > ----------------------------------------------------------- > > (Updated Oct. 10, 2013, 10:14 p.m.) > > > Review request for kafka. > > > Repository: kafka > > > Description > ------- > > Upgrading zkclient to verion 0.4. Adds retry logic in case of a failed > ZooKeeper session re-establishment. > > > Diffs > ----- > > core/build.sbt b5bcb44f783ae6fe2cfdc091e53e867a56a1e592 > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala > 77449f53dc7a8aef14b29790ee6ac62cbafadda7 > core/src/main/scala/kafka/consumer/ZookeeperTopicEventWatcher.scala > a67c193df9f7cbfc52f75dc1b71dc017de1b5fe2 > core/src/main/scala/kafka/controller/KafkaController.scala > 88d130f55997b72a8590e4cfe92857a7320e70d5 > core/src/main/scala/kafka/server/KafkaHealthcheck.scala > 84ea17a068c84592d3d0b007982f7be8fbc231b7 > core/src/main/scala/kafka/server/KafkaServer.scala > c148fdff61b13c2f06d8d048365daee30a286842 > core/src/main/scala/kafka/utils/ZkReconnector.scala PRE-CREATION > core/src/main/scala/kafka/utils/ZkUtils.scala > d1c4b3d7b3d11c88a1d1474aadbb727704cfb759 > core/src/test/scala/unit/kafka/zk/ZkReconnecterTest.scala PRE-CREATION > > Diff: https://reviews.apache.org/r/14582/diff/ > > > Testing > ------- > > > Thanks, > > Anatoly Fayngelerin > >
