kaisun2000 commented on a change in pull request #688: Fix the watcher leakage
issue
URL: https://github.com/apache/helix/pull/688#discussion_r376126881
##########
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:
If I understand correctly, @jiajunwang concern is that reconnect happen and
fireAllEvent would be triggered and install the watcher for all the paths in
subscriber list. The thing is that there can be case that in the disconnecting
period, the zkpath can be deleted. This can still leak another path. This is
very good point.
The first propose fix by @jiajunwang is:
1/ use exists to re-subscribe in re-connecting phase anyway.
2/ if path is not there, delete the watcher for this path.
This require 3.5 zk feature though. Not feasible now.
For the GC approach, what I can see is that this may still be a problem.
First you check if the subscribe path is there. If it is not there, you delete
it. The problem is that these are two steps operation to ZK. The problem is
between the two steps, the user may create the path and install watcher as one
operation. Now you delete it. This is even worse than leaking.
The second step of the first proposal has the same problem.
Actually I thought about this before, if Zookeeper does not have atomic
check if the path not there then delete watcher, it is hard for us to implement
this correctly.
@jiajunwang, what is your take?
Maybe let us note this issue and have an enhancement task to track 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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
