From 21243f5effc23e68c57f9be93558ddb006e5f504 Mon Sep 17 00:00:00 2001 From: Fangmin Lyu Date: Wed, 8 Aug 2018 21:51:55 -0700 Subject: [PATCH 1/3] cache the approximate data size in DataTree --- .../org/apache/zookeeper/server/DataNode.java | 5 ---- .../org/apache/zookeeper/server/DataTree.java | 28 +++++++++++++++++-- .../apache/zookeeper/server/DataTreeBean.java | 6 ++-- .../zookeeper/server/admin/Commands.java | 2 +- .../server/command/MonitorCommand.java | 2 +- 5 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/java/main/org/apache/zookeeper/server/DataNode.java b/src/java/main/org/apache/zookeeper/server/DataNode.java index 0859aab22c8..5922d16584f 100644 --- a/src/java/main/org/apache/zookeeper/server/DataNode.java +++ b/src/java/main/org/apache/zookeeper/server/DataNode.java @@ -135,11 +135,6 @@ public synchronized Set getChildren() { return Collections.unmodifiableSet(children); } - public synchronized long getApproximateDataSize() { - if(null==data) return 0; - return data.length; - } - synchronized public void copyStat(Stat to) { to.setAversion(stat.getAversion()); to.setCtime(stat.getCtime()); diff --git a/src/java/main/org/apache/zookeeper/server/DataTree.java b/src/java/main/org/apache/zookeeper/server/DataTree.java index eb4b7d65f2b..bd95b8e9183 100644 --- a/src/java/main/org/apache/zookeeper/server/DataTree.java +++ b/src/java/main/org/apache/zookeeper/server/DataTree.java @@ -66,6 +66,7 @@ import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicLong; /** * This class maintains the tree data structure. It doesn't have any networking @@ -90,6 +91,9 @@ public class DataTree { private final WatchManager childWatches = new WatchManager(); + /** cached total size of paths and data for all DataNodes */ + private final AtomicLong nodeDataSize = new AtomicLong(0); + /** the root of zookeeper tree */ private static final String rootZookeeper = "/"; @@ -199,13 +203,24 @@ public long approximateDataSize() { for (Map.Entry entry : nodes.entrySet()) { DataNode value = entry.getValue(); synchronized (value) { - result += entry.getKey().length(); - result += value.getApproximateDataSize(); + result += getNodeSize(entry.getKey(), value.data); } } return result; } + /** + * Get the size of the node based on path and data length. + */ + private static long getNodeSize(String path, byte[] data) { + return (path == null ? 0 : path.length()) + + (data == null ? 0 : data.length); + } + + public long cachedApproximateDataSize() { + return nodeDataSize.get(); + } + /** * This is a pointer to the root of the DataTree. It is the source of truth, * but we usually use the nodes hashmap to find nodes in the tree. @@ -236,6 +251,8 @@ public DataTree() { nodes.put(quotaZookeeper, quotaDataNode); addConfigNode(); + + nodeDataSize.set(approximateDataSize()); } /** @@ -468,6 +485,7 @@ public void createNode(final String path, byte data[], List acl, Long longval = aclCache.convertAcls(acl); DataNode child = new DataNode(data, longval, stat); parent.addChild(childName); + nodeDataSize.addAndGet(getNodeSize(path, child.data)); nodes.put(path, child); EphemeralType ephemeralType = EphemeralType.get(ephemeralOwner); if (ephemeralType == EphemeralType.CONTAINER) { @@ -531,6 +549,7 @@ public void deleteNode(String path, long zxid) if (node == null) { throw new KeeperException.NoNodeException(); } + nodeDataSize.addAndGet(-getNodeSize(path, node.data)); nodes.remove(path); synchronized (node) { aclCache.removeUsage(node.acl); @@ -609,6 +628,7 @@ public Stat setData(String path, byte data[], int version, long zxid, this.updateBytes(lastPrefix, (data == null ? 0 : data.length) - (lastdata == null ? 0 : lastdata.length)); } + nodeDataSize.addAndGet(getNodeSize(path, data) - getNodeSize(path, lastdata)); dataWatches.triggerWatch(path, EventType.NodeDataChanged); return s; } @@ -1183,6 +1203,7 @@ public void deserialize(InputArchive ia, String tag) throws IOException { aclCache.deserialize(ia); nodes.clear(); pTrie.clear(); + nodeDataSize.set(0); String path = ia.readString("path"); while (!"/".equals(path)) { DataNode node = new DataNode(); @@ -1220,6 +1241,9 @@ public void deserialize(InputArchive ia, String tag) throws IOException { path = ia.readString("path"); } nodes.put("/", root); + + nodeDataSize.set(approximateDataSize()); + // we are done with deserializing the // the datatree // update the quotas - create path trie diff --git a/src/java/main/org/apache/zookeeper/server/DataTreeBean.java b/src/java/main/org/apache/zookeeper/server/DataTreeBean.java index 433c13fbb61..4626adb6cb9 100644 --- a/src/java/main/org/apache/zookeeper/server/DataTreeBean.java +++ b/src/java/main/org/apache/zookeeper/server/DataTreeBean.java @@ -25,17 +25,17 @@ */ public class DataTreeBean implements DataTreeMXBean, ZKMBeanInfo { DataTree dataTree; - + public DataTreeBean(org.apache.zookeeper.server.DataTree dataTree){ this.dataTree = dataTree; } - + public int getNodeCount() { return dataTree.getNodeCount(); } public long approximateDataSize() { - return dataTree.approximateDataSize(); + return dataTree.cachedApproximateDataSize(); } public int countEphemerals() { diff --git a/src/java/main/org/apache/zookeeper/server/admin/Commands.java b/src/java/main/org/apache/zookeeper/server/admin/Commands.java index 4712261b6c7..73d7b50e815 100644 --- a/src/java/main/org/apache/zookeeper/server/admin/Commands.java +++ b/src/java/main/org/apache/zookeeper/server/admin/Commands.java @@ -324,7 +324,7 @@ public CommandResponse run(ZooKeeperServer zkServer, Map kwargs) response.put("watch_count", zkdb.getDataTree().getWatchCount()); response.put("ephemerals_count", zkdb.getDataTree().getEphemeralsCount()); - response.put("approximate_data_size", zkdb.getDataTree().approximateDataSize()); + response.put("approximate_data_size", zkdb.getDataTree().cachedApproximateDataSize()); OSMXBean osMbean = new OSMXBean(); response.put("open_file_descriptor_count", osMbean.getOpenFileDescriptorCount()); diff --git a/src/java/main/org/apache/zookeeper/server/command/MonitorCommand.java b/src/java/main/org/apache/zookeeper/server/command/MonitorCommand.java index a17fe40e013..232d0bda163 100644 --- a/src/java/main/org/apache/zookeeper/server/command/MonitorCommand.java +++ b/src/java/main/org/apache/zookeeper/server/command/MonitorCommand.java @@ -60,7 +60,7 @@ public void commandRun() { print("watch_count", zkdb.getDataTree().getWatchCount()); print("ephemerals_count", zkdb.getDataTree().getEphemeralsCount()); - print("approximate_data_size", zkdb.getDataTree().approximateDataSize()); + print("approximate_data_size", zkdb.getDataTree().cachedApproximateDataSize()); OSMXBean osMbean = new OSMXBean(); if (osMbean != null && osMbean.getUnix() == true) { From 2e0cbb170c9742d3f1f7a6786a9fe8b9e8f33414 Mon Sep 17 00:00:00 2001 From: Fangmin Lyu Date: Wed, 15 Aug 2018 22:55:08 -0700 Subject: [PATCH 2/3] add test case to test cached approximate data tree size --- .../apache/zookeeper/server/DataTreeTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/java/test/org/apache/zookeeper/server/DataTreeTest.java b/src/java/test/org/apache/zookeeper/server/DataTreeTest.java index 57497cefc9c..8c8240ff47c 100644 --- a/src/java/test/org/apache/zookeeper/server/DataTreeTest.java +++ b/src/java/test/org/apache/zookeeper/server/DataTreeTest.java @@ -281,4 +281,24 @@ public void testReconfigACLClearOnDeserialize() throws Exception { "expected to have the same acl", ZooDefs.Ids.OPEN_ACL_UNSAFE, tree.getACL("/bug", new Stat())); } + + @Test + public void testCachedApproximateDataSize() throws Exception { + DataTree dt = new DataTree(); + long initialSize = dt.approximateDataSize(); + Assert.assertEquals(dt.cachedApproximateDataSize(), dt.approximateDataSize()); + + // create a node + dt.createNode("/testApproximateDataSize", new byte[20], null, -1, 1, 1, 1); + dt.createNode("/testApproximateDataSize1", new byte[20], null, -1, 1, 1, 1); + Assert.assertEquals(dt.cachedApproximateDataSize(), dt.approximateDataSize()); + + // update data + dt.setData("/testApproximateDataSize1", new byte[32], -1, 1, 1); + Assert.assertEquals(dt.cachedApproximateDataSize(), dt.approximateDataSize()); + + // delete a node + dt.deleteNode("/testApproximateDataSize", -1); + Assert.assertEquals(dt.cachedApproximateDataSize(), dt.approximateDataSize()); + } } From ad2ce216a077c41dfef86acd797011976409d4fe Mon Sep 17 00:00:00 2001 From: Fangmin Lyu Date: Wed, 22 Aug 2018 11:54:09 -0700 Subject: [PATCH 3/3] move to synchronize when visiting node.data --- src/java/main/org/apache/zookeeper/server/DataTree.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java/main/org/apache/zookeeper/server/DataTree.java b/src/java/main/org/apache/zookeeper/server/DataTree.java index bd95b8e9183..9a4f1a71151 100644 --- a/src/java/main/org/apache/zookeeper/server/DataTree.java +++ b/src/java/main/org/apache/zookeeper/server/DataTree.java @@ -549,10 +549,10 @@ public void deleteNode(String path, long zxid) if (node == null) { throw new KeeperException.NoNodeException(); } - nodeDataSize.addAndGet(-getNodeSize(path, node.data)); nodes.remove(path); synchronized (node) { aclCache.removeUsage(node.acl); + nodeDataSize.addAndGet(-getNodeSize(path, node.data)); } DataNode parent = nodes.get(parentName); if (parent == null) {