jiajunwang commented on a change in pull request #1109:
URL: https://github.com/apache/helix/pull/1109#discussion_r459093688



##########
File path: 
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
##########
@@ -2216,4 +2253,25 @@ private void validateCurrentThread() {
       throw new IllegalArgumentException("Must not be done in the zookeeper 
event thread.");
     }
   }
+
+  private void checkNumChildren(String path) throws KeeperException, 
InterruptedException {
+    Stat stat = ((ZkConnection) getConnection()).getZookeeper().exists(path, 
false);

Review comment:
       I just noticed this issue. So you only check on connection loss 
exception. What if when you call this exists, the connection has not been 
recovered yet? This is very possible since there is no other operation or waits 
between the error and this exists() call.
   
   In this case, the check will throw an exception, and the retry until connect 
call will also return unexpectedly.
   
   I think we need to wait until connected here.




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