zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lvfangmin <...@git.apache.org>
Subject [GitHub] zookeeper pull request #622: [ZOOKEEPER-3145] Fix potential watch missing is...
Date Fri, 14 Sep 2018 16:30:29 GMT
Github user lvfangmin commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/622#discussion_r217769719
  
    --- Diff: src/java/main/org/apache/zookeeper/server/DataTree.java ---
    @@ -1057,22 +1065,49 @@ void killSession(long session, long zxid) {
             // so there is no need for synchronization. The list is not
             // changed here. Only create and delete change the list which
             // are again called from FinalRequestProcessor in sequence.
    -        Set<String> list = ephemerals.remove(session);
    -        if (list != null) {
    -            for (String path : list) {
    -                try {
    -                    deleteNode(path, zxid);
    -                    if (LOG.isDebugEnabled()) {
    -                        LOG
    -                                .debug("Deleting ephemeral node " + path
    -                                        + " for session 0x"
    -                                        + Long.toHexString(session));
    -                    }
    -                } catch (NoNodeException e) {
    -                    LOG.warn("Ignoring NoNodeException for path " + path
    -                            + " while removing ephemeral for dead session 0x"
    +        killSession(session, zxid, ephemerals.remove(session), null);
    +    }
    +
    +    void killSession(long session, long zxid, Set<String> paths2DeleteLocal,
    +            List<String> paths2DeleteInTxn) {
    +        if (paths2DeleteInTxn != null) {
    +            deleteNodes(session, zxid, paths2DeleteInTxn);
    +        }
    +
    +        if (paths2DeleteLocal == null) {
    +            return;
    +        }
    +
    +        if (paths2DeleteInTxn != null) {
    +            // explicitly check and remove to avoid potential performance
    +            // issue when using removeAll
    +            for (String path: paths2DeleteInTxn) {
    +                paths2DeleteLocal.remove(path);
    +            }
    +            if (!paths2DeleteLocal.isEmpty()) {
    --- End diff --
    
    If there is no bug, I expect we won't hit this line, but I'd like to add the logs here
to make sure this is true for now, we may remove it in the future.


---

Mime
View raw message