Review Board 1.7.22


Upgrading zkclient to verion 0.4. Adds retry logic in case of a failed ZooKeeper session re-establishment.

Review Request #14582 - Created Oct. 10, 2013 and updated

Anatoly Fayngelerin
origin/trunk
Reviewers
kafka
kafka
Upgrading zkclient to verion 0.4. Adds retry logic in case of a failed ZooKeeper session re-establishment.

 
Total:
5
Open:
5
Resolved:
0
Dropped:
0
Status:
From:
Posted (Oct. 13, 2013, 11:59 p.m.)
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.
You are using selftype here. Should we use self or this here?
You are using selftype here. Should we use self or this here?
Could you explain why you have to use "reconnect _", instead of just "reconnect"?
Let's follow the convention and use wait.ms.
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?