jiajunwang commented on a change in pull request #688: Fix the watcher leakage
issue
URL: https://github.com/apache/helix/pull/688#discussion_r376124517
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/zookeeper/ZkClient.java
##########
@@ -960,10 +956,10 @@ public void process(WatchedEvent event) {
private void fireAllEvents() {
for (Entry<String, Set<IZkChildListener>> entry :
_childListener.entrySet()) {
- fireChildChangedEvents(entry.getKey(), entry.getValue());
+ fireChildChangedEvents(entry.getKey(), entry.getValue(), true);
Review comment:
The first point is for us to simplify the GC. I agree it won't help with the
listener becoming inactive case.
And, about the second part, a behavior change is fine as long as it is
reasonable. However, the main concern is that, even with the current design,
leakage is still possible, right? Since the watcher removing depends on the
notification, if no notification arrived, or the client missed, the watcher
will be leaked. That's why I think this design is not good enough. Feel free to
fix the current design if there is a way to avoid all leakages.
----------------------------------------------------------------
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]
