kaisun2000 commented on a change in pull request #1119:
URL: https://github.com/apache/helix/pull/1119#discussion_r447903378



##########
File path: 
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
##########
@@ -1165,20 +1177,92 @@ private void reconnect() {
     }
   }
 
+  private class SyncContext {

Review comment:
       Before this pull request, I already had a look. THe reason not to use 
ZkAsyncCallbacks are two fold:
   1/ Upon failure, the syncContext need to block the current ZkEvent queue. 
ZkAsyncCallbacks does not block the ZkEvent queue or the _asycThread event 
queue, but retry from _asyncThread. ---- Thread modeling difference.
   2/ ZkAsyncCallbacks upon session expiration, would still retry. For this 
SyncContext, it did not retry session expiration. -- Error handling difference.
   
   Especially for thread modeling difference, it is hard to merge them together.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to