[GitHub] zookeeper pull request #136: [ZOOKEEPER-1416] Persistent Recursive Watch

[skamille <gi...@git.apache.org>]

Github user skamille commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/136#discussion_r136637514
  
    --- Diff: src/java/main/org/apache/zookeeper/server/WatchManager.java ---
    @@ -40,50 +40,110 @@
     class WatchManager {
         private static final Logger LOG = LoggerFactory.getLogger(WatchManager.class);
     
    -    private final HashMap<String, HashSet<Watcher>> watchTable =
    -        new HashMap<String, HashSet<Watcher>>();
    +    enum Type {
    +        STANDARD() {
    +            @Override
    +            boolean isPersistent() {
    +                return false;
    +            }
    +
    +            @Override
    +            boolean isRecursive() {
    +                return false;
    +            }
    +        },
    +        PERSISTENT() {
    +            @Override
    +            boolean isPersistent() {
    +                return true;
    +            }
    +
    +            @Override
    +            boolean isRecursive() {
    +                return false;
    +            }
    +        },
    +        PERSISTENT_RECURSIVE() {
    +            @Override
    +            boolean isPersistent() {
    +                return true;
    +            }
    +
    +            @Override
    +            boolean isRecursive() {
    +                return true;
    +            }
    +        }
    +        ;
    +
    +        abstract boolean isPersistent();
    +        abstract boolean isRecursive();
    +    }
    +
    +    private final Map<String, Map<Watcher, Type>> watchTable =
    +        new HashMap<>();
    +
    +    private final Map<Watcher, Set<String>> watch2Paths =
    +        new HashMap<>();
     
    -    private final HashMap<Watcher, HashSet<String>> watch2Paths =
    -        new HashMap<Watcher, HashSet<String>>();
    +    private int recursiveWatchQty = 0;    // guarded by sync
    +
    +    // visible for testing
    +    synchronized int getRecursiveWatchQty() {
    +        return recursiveWatchQty;
    +    }
     
         synchronized int size(){
             int result = 0;
    -        for(Set<Watcher> watches : watchTable.values()) {
    +        for(Map<Watcher, Type> watches : watchTable.values()) {
                 result += watches.size();
             }
             return result;
         }
     
    -    synchronized void addWatch(String path, Watcher watcher) {
    -        HashSet<Watcher> list = watchTable.get(path);
    +    synchronized void addWatch(String path, Watcher watcher, WatchManager.Type type) {
    +        Map<Watcher, Type> list = watchTable.get(path);
             if (list == null) {
                 // don't waste memory if there are few watches on a node
                 // rehash when the 4th entry is added, doubling size thereafter
                 // seems like a good compromise
    -            list = new HashSet<Watcher>(4);
    +            list = new HashMap<>(4);
    --- End diff --
    
    Does the assumption still hold re: memory management now that we have a hashmap instead of a hashset?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---