jiajunwang commented on a change in pull request #688: Fix the watcher leakage
issue
URL: https://github.com/apache/helix/pull/688#discussion_r376041346
##########
File path:
helix-core/src/test/java/org/apache/helix/manager/zk/client/TestHelixZkClient.java
##########
@@ -163,6 +163,8 @@ public void handleDataDeleted(String s) {
// expected to be here.
}
+ // client B needs to re-install the data watch
+ sharedZkClientB.watchForData(TEST_PATH);
Review comment:
One way is keeping the existing subscription methods the same. But we
created some private/protected methods for the HelixManager to use.
Alternatively, as you said, adding options to the method. It is not optimal
but compared with adding more methods, I would prefer to keep it simple now.
One improvement is that we extend the listeners so the one keeps existing after
delete would be a different listener type internally. Then the callback knows
what to do. It helps to keep the methods simple.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
