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/d124a0a6 Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/d124a0a6 Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/d124a0a6 Branch: refs/heads/CURATOR-3.0 Commit: d124a0a632e7817df69b305d47e4c3dd17429485 Parents: 38de628 Author: Scott Blum <[email protected]> Authored: Thu Jan 28 12:58:14 2016 -0500 Committer: Scott Blum <[email protected]> Committed: Mon Feb 1 14:12:00 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/d124a0a6/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; }
