You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by dr...@apache.org on 2016/01/28 19:01:30 UTC

[2/2] curator git commit: CURATOR-294: Optimize TreeCache, fix possible concurrency issue

CURATOR-294: Optimize TreeCache, fix possible concurrency issue


Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/81067f5b
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/81067f5b
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/81067f5b

Branch: refs/heads/CURATOR-294
Commit: 81067f5b35937e4415b1d11e8d06b99a21f67926
Parents: cc27e34
Author: Scott Blum <dr...@apache.org>
Authored: Thu Jan 28 12:58:14 2016 -0500
Committer: Scott Blum <dr...@apache.org>
Committed: Thu Jan 28 12:58:14 2016 -0500

----------------------------------------------------------------------
 .../framework/recipes/cache/TreeCache.java      | 37 +++++++++++---------
 1 file changed, 20 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/81067f5b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCache.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCache.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCache.java
index 4d00266..3f7d8d4 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCache.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCache.java
@@ -220,8 +220,7 @@ public class TreeCache implements Closeable
         final AtomicReference<NodeState> nodeState = new AtomicReference<NodeState>(NodeState.PENDING);
         final TreeNode parent;
         final String path;
-        final AtomicReference<Stat> stat = new AtomicReference<Stat>();
-        final AtomicReference<byte[]> data = new AtomicReference<byte[]>();
+        final AtomicReference<ChildData> childData = new AtomicReference<ChildData>();
         final AtomicReference<ConcurrentMap<String, TreeNode>> children = new AtomicReference<ConcurrentMap<String, TreeNode>>();
         final int depth;
 
@@ -296,8 +295,7 @@ public class TreeCache implements Closeable
 
         void wasDeleted() throws Exception
         {
-            Stat oldStat = stat.getAndSet(null);
-            byte[] oldData = data.getAndSet(null);
+            ChildData oldChildData = childData.getAndSet(null);
             client.clearWatcherReferences(this);
             ConcurrentMap<String, TreeNode> childMap = children.getAndSet(null);
             if ( childMap != null )
@@ -318,7 +316,7 @@ public class TreeCache implements Closeable
             NodeState oldState = nodeState.getAndSet(NodeState.DEAD);
             if ( oldState == NodeState.LIVE )
             {
-                publishEvent(TreeCacheEvent.Type.NODE_REMOVED, new ChildData(path, oldStat, oldData));
+                publishEvent(TreeCacheEvent.Type.NODE_REMOVED, oldChildData);
             }
 
             if ( parent == null )
@@ -383,12 +381,12 @@ public class TreeCache implements Closeable
             case CHILDREN:
                 if ( event.getResultCode() == KeeperException.Code.OK.intValue() )
                 {
-                    Stat oldStat = stat.get();
-                    if ( oldStat != null && oldStat.getMzxid() == newStat.getMzxid() )
+                    ChildData oldChildData = childData.get();
+                    if ( oldChildData != null && oldChildData.getStat().getMzxid() == newStat.getMzxid() )
                     {
-                        // Only update stat if mzxid is different, otherwise we might obscure
+                        // Only update stat if mzxid is same, otherwise we might obscure
                         // GET_DATA event updates.
-                        stat.set(newStat);
+                        childData.compareAndSet(oldChildData, new ChildData(oldChildData.getPath(), newStat, oldChildData.getData()));
                     }
 
                     if ( event.getChildren().isEmpty() )
@@ -435,22 +433,27 @@ public class TreeCache implements Closeable
             case GET_DATA:
                 if ( event.getResultCode() == KeeperException.Code.OK.intValue() )
                 {
+                    ChildData toPublish = new ChildData(event.getPath(), newStat, event.getData());
+                    ChildData oldChildData;
                     if ( cacheData )
                     {
-                        data.set(event.getData());
+                        oldChildData = childData.getAndSet(toPublish);
+                    }
+                    else
+                    {
+                        oldChildData = childData.getAndSet(new ChildData(event.getPath(), newStat, null));
                     }
 
-                    Stat oldStat = stat.getAndSet(newStat);
                     NodeState oldState = nodeState.getAndSet(NodeState.LIVE);
                     if ( oldState != NodeState.LIVE )
                     {
-                        publishEvent(TreeCacheEvent.Type.NODE_ADDED, new ChildData(event.getPath(), newStat, event.getData()));
+                        publishEvent(TreeCacheEvent.Type.NODE_ADDED, toPublish);
                     }
                     else
                     {
-                        if ( oldStat == null || oldStat.getMzxid() != newStat.getMzxid() )
+                        if ( oldChildData == null || oldChildData.getStat().getMzxid() != newStat.getMzxid() )
                         {
-                            publishEvent(TreeCacheEvent.Type.NODE_UPDATED, new ChildData(event.getPath(), newStat, event.getData()));
+                            publishEvent(TreeCacheEvent.Type.NODE_UPDATED, toPublish);
                         }
                     }
                 }
@@ -681,9 +684,9 @@ public class TreeCache implements Closeable
             for ( Map.Entry<String, TreeNode> entry : map.entrySet() )
             {
                 TreeNode childNode = entry.getValue();
-                ChildData childData = new ChildData(childNode.path, childNode.stat.get(), childNode.data.get());
+                ChildData childData = childNode.childData.get();
                 // Double-check liveness after retreiving data.
-                if ( childNode.nodeState.get() == NodeState.LIVE )
+                if ( childData != null && childNode.nodeState.get() == NodeState.LIVE )
                 {
                     builder.put(entry.getKey(), childData);
                 }
@@ -710,7 +713,7 @@ public class TreeCache implements Closeable
         {
             return null;
         }
-        ChildData result = new ChildData(node.path, node.stat.get(), node.data.get());
+        ChildData result = node.childData.get();
         // Double-check liveness after retreiving data.
         return node.nodeState.get() == NodeState.LIVE ? result : null;
     }