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



##########
File path: 
helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java
##########
@@ -554,12 +554,15 @@ private void subscribeChildChange(String path, 
NotificationContext.Type callback
       if (!childrenSubscribeResult.isInstalled()) {
         logger.info("CallbackHandler {} subscribe data path {} failed!", this, 
path);
       }
+      return childrenSubscribeResult.getChildren();

Review comment:
       I think the main issue here is that line 551
   `_zkClient.subscribeChildChanges(path, this, callbackType != Type.INIT);` we 
have two code path.
   
   if callbackType is init, the parent path of callback handler may not be 
created. Thus, the return would be null, or empty? Please verify.
   
   The potentially issue is that note later at `default`, we re-use the return 
children list from here. 
   Note, original code retrieve children one more time and it may retrieve node 
by then. Here, it may not.
   
   This is different from previous logic. Not sure if this would bring a bug or 
not. 




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