You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by ha...@apache.org on 2017/06/18 17:23:08 UTC
zookeeper git commit: ZOOKEEPER-2808: fix miss-referenced ACL count
issue when upgrading the ZooKeeper
Repository: zookeeper
Updated Branches:
refs/heads/master c38febe2d -> 111ae5a5b
ZOOKEEPER-2808: fix miss-referenced ACL count issue when upgrading the ZooKeeper
Author: Fangmin Lyu <al...@fb.com>
Reviewers: Michael Han <ha...@apache.org>
Closes #284 from lvfangmin/ZOOKEEPER-2808
Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/111ae5a5
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/111ae5a5
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/111ae5a5
Branch: refs/heads/master
Commit: 111ae5a5b67999925f1921736a14fcfbdb1b7348
Parents: c38febe
Author: Fangmin Lyu <al...@fb.com>
Authored: Sun Jun 18 10:23:04 2017 -0700
Committer: Michael Han <ha...@apache.org>
Committed: Sun Jun 18 10:23:04 2017 -0700
----------------------------------------------------------------------
.../org/apache/zookeeper/server/DataTree.java | 56 ++++++++++----------
.../org/apache/zookeeper/server/ZKDatabase.java | 12 ++---
.../apache/zookeeper/server/DataTreeTest.java | 50 ++++++++++++++++-
3 files changed, 83 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zookeeper/blob/111ae5a5/src/java/main/org/apache/zookeeper/server/DataTree.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/DataTree.java b/src/java/main/org/apache/zookeeper/server/DataTree.java
index f0ab2b3..b6fb61d 100644
--- a/src/java/main/org/apache/zookeeper/server/DataTree.java
+++ b/src/java/main/org/apache/zookeeper/server/DataTree.java
@@ -118,7 +118,7 @@ public class DataTree {
/** this will be the string thats stored as a child of /zookeeper */
private static final String configChildZookeeper = configZookeeper
.substring(procZookeeper.length() + 1);
-
+
/**
* the path trie that keeps track fo the quota nodes in this datatree
*/
@@ -223,13 +223,6 @@ public class DataTree {
*/
private final DataNode quotaDataNode = new DataNode(new byte[0], -1L, new StatPersisted());
- /**
- * create a /zookeeper/config node for maintaining the configuration (membership and quorum system) info for
- * zookeeper
- */
- private DataNode configDataNode = new DataNode(new byte[0], -1L, new StatPersisted());
-
-
public DataTree() {
/* Rather than fight it, let root have an alias */
nodes.put("", root);
@@ -241,10 +234,14 @@ public class DataTree {
procDataNode.addChild(quotaChildZookeeper);
nodes.put(quotaZookeeper, quotaDataNode);
-
+
addConfigNode();
}
+ /**
+ * create a /zookeeper/config node for maintaining the configuration (membership and quorum system) info for
+ * zookeeper
+ */
public void addConfigNode() {
DataNode zookeeperZnode = nodes.get(procZookeeper);
if (zookeeperZnode != null) { // should always be the case
@@ -253,7 +250,7 @@ public class DataTree {
assert false : "There's no /zookeeper znode - this should never happen.";
}
- nodes.put(configZookeeper, configDataNode);
+ nodes.put(configZookeeper, new DataNode(new byte[0], -1L, new StatPersisted()));
try {
// Reconfig node is access controlled by default (ZOOKEEPER-2014).
setACL(configZookeeper, ZooDefs.Ids.READ_ACL_UNSAFE, -1);
@@ -406,8 +403,8 @@ public class DataTree {
* @param zxid
* Transaction ID
* @param time
- * @throws NodeExistsException
- * @throws NoNodeException
+ * @throws NodeExistsException
+ * @throws NoNodeException
* @throws KeeperException
*/
public void createNode(final String path, byte data[], List<ACL> acl,
@@ -415,7 +412,7 @@ public class DataTree {
throws NoNodeException, NodeExistsException {
createNode(path, data, acl, ephemeralOwner, parentCVersion, zxid, time, null);
}
-
+
/**
* Add a new node to the DataTree.
* @param path
@@ -432,8 +429,8 @@ public class DataTree {
* @param time
* @param outputStat
* A Stat object to store Stat output results into.
- * @throws NodeExistsException
- * @throws NoNodeException
+ * @throws NodeExistsException
+ * @throws NoNodeException
* @throws KeeperException
*/
public void createNode(final String path, byte data[], List<ACL> acl,
@@ -1327,37 +1324,37 @@ public class DataTree {
DataNode node = getNode(path);
WatchedEvent e = null;
if (node == null) {
- watcher.process(new WatchedEvent(EventType.NodeDeleted,
+ watcher.process(new WatchedEvent(EventType.NodeDeleted,
KeeperState.SyncConnected, path));
} else if (node.stat.getMzxid() > relativeZxid) {
- watcher.process(new WatchedEvent(EventType.NodeDataChanged,
+ watcher.process(new WatchedEvent(EventType.NodeDataChanged,
KeeperState.SyncConnected, path));
} else {
this.dataWatches.addWatch(path, watcher);
- }
- }
+ }
+ }
for (String path : existWatches) {
DataNode node = getNode(path);
if (node != null) {
- watcher.process(new WatchedEvent(EventType.NodeCreated,
+ watcher.process(new WatchedEvent(EventType.NodeCreated,
KeeperState.SyncConnected, path));
} else {
this.dataWatches.addWatch(path, watcher);
- }
- }
+ }
+ }
for (String path : childWatches) {
DataNode node = getNode(path);
if (node == null) {
- watcher.process(new WatchedEvent(EventType.NodeDeleted,
+ watcher.process(new WatchedEvent(EventType.NodeDeleted,
KeeperState.SyncConnected, path));
} else if (node.stat.getPzxid() > relativeZxid) {
- watcher.process(new WatchedEvent(EventType.NodeChildrenChanged,
+ watcher.process(new WatchedEvent(EventType.NodeChildrenChanged,
KeeperState.SyncConnected, path));
} else {
this.childWatches.addWatch(path, watcher);
- }
- }
- }
+ }
+ }
+ }
/**
* This method sets the Cversion and Pzxid for the specified node to the
@@ -1436,4 +1433,9 @@ public class DataTree {
}
return removed;
}
+
+ // visible for testing
+ public ReferenceCountedACLCache getReferenceCountedAclCache() {
+ return aclCache;
+ }
}
http://git-wip-us.apache.org/repos/asf/zookeeper/blob/111ae5a5/src/java/main/org/apache/zookeeper/server/ZKDatabase.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/ZKDatabase.java b/src/java/main/org/apache/zookeeper/server/ZKDatabase.java
index 05bbb91..2845d28 100644
--- a/src/java/main/org/apache/zookeeper/server/ZKDatabase.java
+++ b/src/java/main/org/apache/zookeeper/server/ZKDatabase.java
@@ -75,13 +75,13 @@ public class ZKDatabase {
protected ConcurrentHashMap<Long, Integer> sessionsWithTimeouts;
protected FileTxnSnapLog snapLog;
protected long minCommittedLog, maxCommittedLog;
-
+
/**
* Default value is to use snapshot if txnlog size exceeds 1/3 the size of snapshot
*/
public static final String SNAPSHOT_SIZE_FACTOR = "zookeeper.snapshotSizeFactor";
private double snapshotSizeFactor = 0.33;
-
+
public static final int commitLogCount = 500;
protected static int commitLogBuffer = 700;
protected LinkedList<Proposal> committedLog = new LinkedList<Proposal>();
@@ -268,7 +268,7 @@ public class ZKDatabase {
wl.unlock();
}
}
-
+
public double getSnapshotSizeFactor() {
return snapshotSizeFactor;
}
@@ -570,7 +570,7 @@ public class ZKDatabase {
try {
if (this.dataTree.getNode(ZooDefs.CONFIG_NODE) == null) {
// should only happen during upgrade
- LOG.warn("configuration znode missing (hould only happen during upgrade), creating the node");
+ LOG.warn("configuration znode missing (should only happen during upgrade), creating the node");
this.dataTree.addConfigNode();
}
this.dataTree.setData(ZooDefs.CONFIG_NODE, qv.toString().getBytes(), -1, qv.getVersion(), Time.currentWallTime());
@@ -578,7 +578,7 @@ public class ZKDatabase {
System.out.println("configuration node missing - should not happen");
}
}
-
+
/**
* Use for unit testing, so we can turn this feature on/off
* @param snapshotSizeFactor Set to minus value to turn this off.
@@ -603,7 +603,7 @@ public class ZKDatabase {
/**
* Remove watch from the datatree
- *
+ *
* @param path
* node to remove watches from
* @param type
http://git-wip-us.apache.org/repos/asf/zookeeper/blob/111ae5a5/src/java/test/org/apache/zookeeper/server/DataTreeTest.java
----------------------------------------------------------------------
diff --git a/src/java/test/org/apache/zookeeper/server/DataTreeTest.java b/src/java/test/org/apache/zookeeper/server/DataTreeTest.java
index 8b2bd80..57497ce 100644
--- a/src/java/test/org/apache/zookeeper/server/DataTreeTest.java
+++ b/src/java/test/org/apache/zookeeper/server/DataTreeTest.java
@@ -25,6 +25,8 @@ import org.apache.zookeeper.KeeperException.NodeExistsException;
import org.apache.zookeeper.WatchedEvent;
import org.apache.zookeeper.Watcher;
import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.data.ACL;
import org.apache.zookeeper.data.Stat;
import org.junit.After;
import org.junit.Assert;
@@ -46,6 +48,8 @@ import java.lang.reflect.*;
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.List;
+import java.util.ArrayList;
public class DataTreeTest extends ZKTestCase {
protected static final Logger LOG = LoggerFactory.getLogger(DataTreeTest.class);
@@ -113,7 +117,7 @@ public class DataTreeTest extends ZKTestCase {
dataTree.getNode("/").stat.getCversion() + 1, 1, 1);
}
}
-
+
@Test(timeout = 60000)
public void testRootWatchTriggered() throws Exception {
class MyWatcher implements Watcher{
@@ -177,7 +181,7 @@ public class DataTreeTest extends ZKTestCase {
PathTrie pTrie = (PathTrie)pfield.get(dserTree);
//Check that the node path is removed from pTrie
- Assert.assertEquals("/bug is still in pTrie", "", pTrie.findMaxPrefix("/bug"));
+ Assert.assertEquals("/bug is still in pTrie", "", pTrie.findMaxPrefix("/bug"));
}
/*
@@ -235,4 +239,46 @@ public class DataTreeTest extends ZKTestCase {
//Let's make sure that we hit the code that ran the real assertion above
Assert.assertTrue("Didn't find the expected node", ranTestCase.get());
}
+
+ @Test(timeout = 60000)
+ public void testReconfigACLClearOnDeserialize() throws Exception {
+
+ DataTree tree = new DataTree();
+ // simulate the upgrading scenario, where the reconfig znode
+ // doesn't exist and the acl cache is empty
+ tree.deleteNode(ZooDefs.CONFIG_NODE, 1);
+ tree.getReferenceCountedAclCache().aclIndex = 0;
+
+ Assert.assertEquals(
+ "expected to have 1 acl in acl cache map", 0, tree.aclCacheSize());
+
+ // serialize the data with one znode with acl
+ tree.createNode(
+ "/bug", new byte[20], ZooDefs.Ids.OPEN_ACL_UNSAFE, -1, 1, 1, 1);
+
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ BinaryOutputArchive oa = BinaryOutputArchive.getArchive(baos);
+ tree.serialize(oa, "test");
+ baos.flush();
+
+ ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
+ BinaryInputArchive ia = BinaryInputArchive.getArchive(bais);
+ tree.deserialize(ia, "test");
+
+ Assert.assertEquals(
+ "expected to have 1 acl in acl cache map", 1, tree.aclCacheSize());
+ Assert.assertEquals(
+ "expected to have the same acl", ZooDefs.Ids.OPEN_ACL_UNSAFE,
+ tree.getACL("/bug", new Stat()));
+
+ // simulate the upgrading case where the config node will be created
+ // again after leader election
+ tree.addConfigNode();
+
+ Assert.assertEquals(
+ "expected to have 2 acl in acl cache map", 2, tree.aclCacheSize());
+ Assert.assertEquals(
+ "expected to have the same acl", ZooDefs.Ids.OPEN_ACL_UNSAFE,
+ tree.getACL("/bug", new Stat()));
+ }
}