Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Kafka >> mail # dev >> Review Request 14582: Upgrading zkclient to verion 0.4. Adds retry logic in case of a failed ZooKeeper session re-establishment.


Copy link to this message
-
Re: Review Request 14582: Upgrading zkclient to verion 0.4. Adds retry logic in case of a failed ZooKeeper session re-establishment.

-----------------------------------------------------------
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
>
>