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

Switch to Threaded View
Kafka >> mail # dev >> Re: 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: