pkuwm commented on a change in pull request #1344:
URL: https://github.com/apache/helix/pull/1344#discussion_r494636237
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java
##########
@@ -646,10 +652,14 @@ private void subscribeForChanges(NotificationContext.Type
callbackType, String p
break;
}
default: {
- List<String> childNames = _zkClient.getChildren(path);
- if (childNames != null) {
- for (String childName : childNames) {
- String childPath = path + "/" + childName;
+ // When callback type is FINALIZE, subscribeChildChange doesn't
+ // get children list. We need to read children to unsubscribe
data change listeners.
+ if (callbackType == Type.FINALIZE) {
+ children = _zkClient.getChildren(path);
+ }
Review comment:
Moved the logic to subscribeChildChange: for FINALIZE, it also returns
the children.
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java
##########
@@ -538,7 +538,12 @@ public void invoke(NotificationContext changeContext)
throws Exception {
}
}
- private void subscribeChildChange(String path, NotificationContext.Type
callbackType) {
+ /*
+ * If callback type is INIT or CALLBACK, subscribes child change listener to
the path
+ * and returns the path's children names. The children list might be null
when the path
+ * doesn't exist or callback type is other than INIT/CALLBACK/FINALIZE.
Review comment:
Resolved comment.
Since we also have `subscribeChildChanges()` in ZkClient that returns
children, so I'd like to keep it as it is to be consistent. Besides, I don't
have a better name for this and I think it is OK to keep it.
----------------------------------------------------------------
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]
