[GitHub] [helix] xyuanlu opened a new issue, #2509: race condition in ZkClient may leave untracked watcher on ZK

["xyuanlu (via GitHub)" <gi...@apache.org>]

xyuanlu opened a new issue, #2509:
URL: https://github.com/apache/helix/issues/2509

   ### Describe the bug
   `  
   public ChildrenSubscribeResult subscribeChildChanges(String path, IZkChildListener listener, boolean skipWatchingNonExistNode) {
       synchronized (_childListener) {
         Set<IZkChildListener> listeners = _childListener.get(path);
         if (listeners == null) {
           listeners = new CopyOnWriteArraySet<>();
           _childListener.put(path, listeners);
         }
         listeners.add(listener);
       }
   
       List<String> children = watchForChilds(path, skipWatchingNonExistNode);
       if (children == null && skipWatchingNonExistNode) {
         unsubscribeChildChanges(path, listener);
         LOG.info("zkclient{}, watchForChilds failed to install no-existing watch and add listener. Path: {}", _uid, path);
         return new ChildrenSubscribeResult(children, false);
       }
   
       return new ChildrenSubscribeResult(children, true);
     }
   
     public void unsubscribeChildChanges(String path, IZkChildListener childListener) {
       synchronized (_childListener) {
         final Set<IZkChildListener> listeners = _childListener.get(path);
         if (listeners != null) {
           listeners.remove(childListener);
         }
       }
     }
   `
   
   Current on Helix master (May 24 2023,  TOT on commit 07b1bb8), subscribe child/data change does 
   1. Lock in memory map,
   2. add new listener to map,
   3. unlock map
   4. Subscribe an one time listener to ZK
   5. If subscribe failed, call  unsubscribe child/data change (lock, remove, unlock)
   
   unsubscribe child/data change does 
   1.Lock in memory map
   2. remove listener from map
   3. unlock map
   
   When multiple user calling subscribe and unsubscribe at the same time, unsubscribe child/data change may happen between subscribe step 3 and 4. As a result, the listener is not added to the map but the ZkClient is registered as watcher in ZK. Leaving a untracked watcher.
   
   
   ### Expected behavior
   When multiple user calling subscribe and unsubscribe at the same time, the final result is undetermined. However, all registered ZK watcher should be tracked.
   
   ### Additional context
   Add any other context about the problem here.
   


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

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org

[GitHub] [helix] jiajunwang commented on issue #2509: race condition in ZkClient may leave untracked watcher on ZK

["jiajunwang (via GitHub)" <gi...@apache.org>]

jiajunwang commented on issue #2509:
URL: https://github.com/apache/helix/issues/2509#issuecomment-1595054197

   Since the watcher is one-time, I don't think we consider this a leakage, right?
   
   Moreover, I suspect that if we can track all ZK watchers, and do we really need to do it? If user calls unsubscribeChildChanges 10 mins after subscribeChildChanges, the watcher will still become untracked, right?
   
   I assume the goal is to ensure all the existing watchers are tracked by _childListener map? For achieving this goal, I guess some GC mechanism works better. And in reality, the watchers only have a long life when they are watching non-exist paths (so no update), or very stable paths. Seems low risk to me. Do we have more detail about the potential user impact?


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

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org