You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by ca...@apache.org on 2011/06/16 16:00:03 UTC
svn commit: r1136440 - in /zookeeper/branches/branch-3.3: ./
src/java/main/org/apache/zookeeper/server/
src/java/main/org/apache/zookeeper/server/persistence/
src/java/test/org/apache/zookeeper/test/
Author: camille
Date: Thu Jun 16 14:00:03 2011
New Revision: 1136440
URL: http://svn.apache.org/viewvc?rev=1136440&view=rev
Log:
ZOOKEEPER-1046: Creating a new sequential node results in a ZNODEEXISTS error
Added:
zookeeper/branches/branch-3.3/src/java/test/org/apache/zookeeper/test/LoadFromLogTest.java (with props)
Modified:
zookeeper/branches/branch-3.3/CHANGES.txt
zookeeper/branches/branch-3.3/src/java/main/org/apache/zookeeper/server/DataTree.java
zookeeper/branches/branch-3.3/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
zookeeper/branches/branch-3.3/src/java/test/org/apache/zookeeper/test/DataTreeTest.java
Modified: zookeeper/branches/branch-3.3/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/branches/branch-3.3/CHANGES.txt?rev=1136440&r1=1136439&r2=1136440&view=diff
==============================================================================
--- zookeeper/branches/branch-3.3/CHANGES.txt (original)
+++ zookeeper/branches/branch-3.3/CHANGES.txt Thu Jun 16 14:00:03 2011
@@ -26,6 +26,8 @@ BUGFIXES:
ZOOKEEPER-985. Test BookieRecoveryTest fails on trunk. (fpj via breed)
ZOOKEEPER-880. QuorumCnxManager$SendWorker grows without bounds (vishal via breed)
+
+ ZOOKEEPER-1046. Creating a new sequential node results in a ZNODEEXISTS error. (Vishal K via camille)
IMPROVEMENTS:
ZOOKEEPER-963. Make Forrest work with JDK6 (Carl Steinbach via henryr)
@@ -102,7 +104,7 @@ BUGFIXES:
(jared cantwell via mahadev)
ZOOKEEPER-907. Spurious "KeeperErrorCode = Session moved" messages (vishal k via breed)
-
+
IMPROVEMENTS:
ZOOKEEPER-789. Improve FLE log messages (flavio via phunt)
Modified: zookeeper/branches/branch-3.3/src/java/main/org/apache/zookeeper/server/DataTree.java
URL: http://svn.apache.org/viewvc/zookeeper/branches/branch-3.3/src/java/main/org/apache/zookeeper/server/DataTree.java?rev=1136440&r1=1136439&r2=1136440&view=diff
==============================================================================
--- zookeeper/branches/branch-3.3/src/java/main/org/apache/zookeeper/server/DataTree.java (original)
+++ zookeeper/branches/branch-3.3/src/java/main/org/apache/zookeeper/server/DataTree.java Thu Jun 16 14:00:03 2011
@@ -754,17 +754,18 @@ public class DataTree {
case OpCode.create:
CreateTxn createTxn = (CreateTxn) txn;
debug = "Create transaction for " + createTxn.getPath();
+ rc.path = createTxn.getPath();
createNode(
createTxn.getPath(),
createTxn.getData(),
createTxn.getAcl(),
createTxn.getEphemeral() ? header.getClientId() : 0,
header.getZxid(), header.getTime());
- rc.path = createTxn.getPath();
break;
case OpCode.delete:
DeleteTxn deleteTxn = (DeleteTxn) txn;
debug = "Delete transaction for " + deleteTxn.getPath();
+ rc.path = deleteTxn.getPath();
deleteNode(deleteTxn.getPath(), header.getZxid());
break;
case OpCode.setData:
@@ -791,11 +792,8 @@ public class DataTree {
break;
}
} catch (KeeperException e) {
- // These are expected errors since we take a lazy snapshot
- if (initialized
- || (e.code() != Code.NONODE && e.code() != Code.NODEEXISTS)) {
- LOG.warn("Failed:" + debug, e);
- }
+ LOG.debug("Failed: " + debug, e);
+ rc.err = e.code().intValue();
}
return rc;
}
@@ -1182,4 +1180,33 @@ public class DataTree {
}
}
}
+
+ /**
+ * If the znode for the specified path is found, then this method
+ * increments the cversion and sets its pzxid to the zxid passed
+ * in the second argument. A NoNodeException is thrown if the znode is
+ * not found.
+ *
+ * @param path
+ * Full path to the znode whose cversion needs to be incremented.
+ * A "/" at the end of the path is ignored.
+ * @param zxid
+ * Value to be assigned to pzxid
+ * @throws KeeperException.NoNodeException
+ * If znode not found.
+ **/
+ public void incrementCversion(String path, long zxid)
+ throws KeeperException.NoNodeException {
+ if (path.endsWith("/")) {
+ path = path.substring(0, path.length() - 1);
+ }
+ DataNode node = nodes.get(path);
+ if (node == null) {
+ throw new KeeperException.NoNodeException(path);
+ }
+ synchronized (node) {
+ node.stat.setCversion(node.stat.getCversion() + 1);
+ node.stat.setPzxid(zxid);
+ }
+ }
}
Modified: zookeeper/branches/branch-3.3/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
URL: http://svn.apache.org/viewvc/zookeeper/branches/branch-3.3/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java?rev=1136440&r1=1136439&r2=1136440&view=diff
==============================================================================
--- zookeeper/branches/branch-3.3/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java (original)
+++ zookeeper/branches/branch-3.3/src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java Thu Jun 16 14:00:03 2011
@@ -33,6 +33,11 @@ import org.apache.zookeeper.server.ZooTr
import org.apache.zookeeper.server.persistence.TxnLog.TxnIterator;
import org.apache.zookeeper.txn.CreateSessionTxn;
import org.apache.zookeeper.txn.TxnHeader;
+import org.apache.zookeeper.KeeperException.Code;
+import org.apache.zookeeper.KeeperException.NoNodeException;
+import org.apache.zookeeper.server.DataTree.ProcessTxnResult;
+import org.apache.zookeeper.KeeperException.NoNodeException;
+import org.apache.zookeeper.KeeperException;
/**
* This is a helper class
@@ -141,7 +146,12 @@ public class FileTxnSnapLog {
} else {
highestZxid = hdr.getZxid();
}
- processTransaction(hdr,dt,sessions, itr.getTxn());
+ try {
+ processTransaction(hdr,dt,sessions, itr.getTxn());
+ } catch(KeeperException.NoNodeException e) {
+ throw new IOException("Failed to process transaction type: " +
+ hdr.getType() + " error: " + e.getMessage());
+ }
if (!itr.next())
break;
}
@@ -155,8 +165,10 @@ public class FileTxnSnapLog {
* @param sessions the sessions to be restored
* @param txn the transaction to be applied
*/
- private void processTransaction(TxnHeader hdr,DataTree dt,
- Map<Long, Integer> sessions, Record txn){
+ public void processTransaction(TxnHeader hdr,DataTree dt,
+ Map<Long, Integer> sessions, Record txn)
+ throws KeeperException.NoNodeException {
+ ProcessTxnResult rc;
switch (hdr.getType()) {
case OpCode.createSession:
sessions.put(hdr.getClientId(),
@@ -169,7 +181,7 @@ public class FileTxnSnapLog {
+ ((CreateSessionTxn) txn).getTimeOut());
}
// give dataTree a chance to sync its lastProcessedZxid
- dt.processTxn(hdr, txn);
+ rc = dt.processTxn(hdr, txn);
break;
case OpCode.closeSession:
sessions.remove(hdr.getClientId());
@@ -178,11 +190,45 @@ public class FileTxnSnapLog {
"playLog --- close session in log: "
+ Long.toHexString(hdr.getClientId()));
}
- dt.processTxn(hdr, txn);
+ rc = dt.processTxn(hdr, txn);
break;
default:
- dt.processTxn(hdr, txn);
- }
+ rc = dt.processTxn(hdr, txn);
+ }
+
+ /**
+ * Snapshots are taken lazily. It can happen that the child
+ * znodes of a parent are modified (deleted or created) after the parent
+ * is serialized. Therefore, while replaying logs during restore, a
+ * delete/create might fail because the node was already
+ * deleted/created.
+ *
+ * After seeing this failure, we should increment
+ * the cversion of the parent znode since the parent was serialized
+ * before its children.
+ *
+ * Note, such failures on DT should be seen only during
+ * restore.
+ */
+ if ((hdr.getType() == OpCode.delete &&
+ rc.err == Code.NONODE.intValue()) ||
+ (hdr.getType() == OpCode.create &&
+ rc.err == Code.NODEEXISTS.intValue())) {
+ LOG.debug("Failed Txn: " + hdr.getType() + " path:" +
+ rc.path + " err: " + rc.err);
+ int lastSlash = rc.path.lastIndexOf('/');
+ String parentName = rc.path.substring(0, lastSlash);
+ try {
+ dt.incrementCversion(parentName, hdr.getZxid());
+ } catch (KeeperException.NoNodeException e) {
+ LOG.error("Failed to increment parent cversion for: " +
+ parentName, e);
+ throw e;
+ }
+ } else if (rc.err != Code.OK.intValue()) {
+ LOG.debug("Ignoring processTxn failure hdr: " + hdr.getType() +
+ " : error: " + rc.err);
+ }
}
/**
Modified: zookeeper/branches/branch-3.3/src/java/test/org/apache/zookeeper/test/DataTreeTest.java
URL: http://svn.apache.org/viewvc/zookeeper/branches/branch-3.3/src/java/test/org/apache/zookeeper/test/DataTreeTest.java?rev=1136440&r1=1136439&r2=1136440&view=diff
==============================================================================
--- zookeeper/branches/branch-3.3/src/java/test/org/apache/zookeeper/test/DataTreeTest.java (original)
+++ zookeeper/branches/branch-3.3/src/java/test/org/apache/zookeeper/test/DataTreeTest.java Thu Jun 16 14:00:03 2011
@@ -18,6 +18,7 @@
package org.apache.zookeeper.test;
+import junit.framework.Assert;
import junit.framework.TestCase;
import org.apache.log4j.Logger;
@@ -26,6 +27,7 @@ import org.apache.zookeeper.Watcher;
import org.apache.zookeeper.data.Stat;
import org.apache.zookeeper.server.DataTree;
import org.junit.Test;
+import org.apache.zookeeper.server.DataNode;
public class DataTreeTest extends TestCase {
protected static final Logger LOG = Logger.getLogger(DataTreeTest.class);
@@ -44,6 +46,23 @@ public class DataTreeTest extends TestCa
LOG.info("FINISHED " + getName());
}
+ /**
+ * For ZOOKEEPER-1046 test if cversion is getting incremented correctly.
+ */
+ @Test
+ public void testIncrementCversion() throws Exception {
+ dt.createNode("/test", new byte[0], null, 0, 1, 1);
+ DataNode zk = dt.getNode("/test");
+ long prevCversion = zk.stat.getCversion();
+ long prevPzxid = zk.stat.getPzxid();
+ dt.incrementCversion("/test/", prevPzxid + 1);
+ long newCversion = zk.stat.getCversion();
+ long newPzxid = zk.stat.getPzxid();
+ Assert.assertTrue("<cversion, pzxid> verification failed. Expected: <" +
+ (prevCversion + 1) + ", " + (prevPzxid + 1) + ">, found: <" +
+ newCversion + ", " + newPzxid + ">",
+ (newCversion == prevCversion + 1 && newPzxid == prevPzxid + 1));
+ }
@Test
public void testRootWatchTriggered() throws Exception {
class MyWatcher implements Watcher{
Added: zookeeper/branches/branch-3.3/src/java/test/org/apache/zookeeper/test/LoadFromLogTest.java
URL: http://svn.apache.org/viewvc/zookeeper/branches/branch-3.3/src/java/test/org/apache/zookeeper/test/LoadFromLogTest.java?rev=1136440&view=auto
==============================================================================
--- zookeeper/branches/branch-3.3/src/java/test/org/apache/zookeeper/test/LoadFromLogTest.java (added)
+++ zookeeper/branches/branch-3.3/src/java/test/org/apache/zookeeper/test/LoadFromLogTest.java Thu Jun 16 14:00:03 2011
@@ -0,0 +1,104 @@
+package org.apache.zookeeper.test;
+
+import java.io.File;
+import java.util.List;
+
+import junit.framework.TestCase;
+
+import org.apache.jute.Record;
+import org.apache.zookeeper.PortAssignment;
+import org.apache.zookeeper.WatchedEvent;
+import org.apache.zookeeper.Watcher;
+import org.apache.zookeeper.ZooDefs.OpCode;
+import org.apache.zookeeper.server.DataNode;
+import org.apache.zookeeper.server.DataTree;
+import org.apache.zookeeper.server.persistence.FileTxnSnapLog;
+import org.apache.zookeeper.txn.CreateTxn;
+import org.apache.zookeeper.txn.DeleteTxn;
+import org.apache.zookeeper.txn.TxnHeader;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class LoadFromLogTest extends TestCase implements Watcher {
+ private static String HOSTPORT = "127.0.0.1:" + PortAssignment.unique();
+ private static final int CONNECTION_TIMEOUT = 3000;
+ private static final int NUM_MESSAGES = 300;
+
+
+ // setting up the quorum has a transaction overhead for creating and closing the session
+ private static final int TRANSACTION_OVERHEAD = 2;
+ private static final int TOTAL_TRANSACTIONS = NUM_MESSAGES + TRANSACTION_OVERHEAD;
+
+
+ public void process(WatchedEvent event) {
+ // do nothing
+ }
+
+ /**
+ * For ZOOKEEPER-1046. Verify if cversion and pzxid if incremented
+ * after create/delete failure during restore.
+ */
+ @Test
+ public void testTxnFailure() throws Exception {
+ long count = 1;
+ File tmpDir = ClientBase.createTmpDir();
+ FileTxnSnapLog logFile = new FileTxnSnapLog(tmpDir, tmpDir);
+ DataTree dt = new DataTree();
+ dt.createNode("/test", new byte[0], null, 0, 1, 1);
+ for (count = 1; count <= 3; count++) {
+ dt.createNode("/test/" + count, new byte[0], null, 0, count,
+ System.currentTimeMillis());
+ }
+ DataNode zk = dt.getNode("/test");
+
+ // Make create to fail, then verify cversion.
+
+ doOp(logFile, OpCode.create, "/test/" + (count - 1), dt, zk);
+
+ // Make delete fo fail, then verify cversion.
+
+ doOp(logFile, OpCode.delete, "/test/" + (count + 1), dt, zk);
+ }
+ /*
+ * Does create/delete depending on the type and verifies
+ * if cversion before the operation is 1 less than cversion afer.
+ */
+ private void doOp(FileTxnSnapLog logFile, int type, String path,
+ DataTree dt, DataNode parent) throws Exception {
+ int lastSlash = path.lastIndexOf('/');
+ String parentName = path.substring(0, lastSlash);
+
+ long prevCversion = parent.stat.getCversion();
+ long prevPzxid = parent.stat.getPzxid();
+ List<String> child = dt.getChildren(parentName, null, null);
+ String childStr = "";
+ for (String s : child) {
+ childStr += s + " ";
+ }
+
+ Record txn = null;
+ TxnHeader txnHeader = null;
+ if (type == OpCode.delete) {
+ txn = new DeleteTxn(path);
+ txnHeader = new TxnHeader(0xabcd, 0x123, prevPzxid + 1,
+ System.currentTimeMillis(), OpCode.delete);
+ } else if (type == OpCode.create) {
+ txnHeader = new TxnHeader(0xabcd, 0x123, prevPzxid + 1,
+ System.currentTimeMillis(), OpCode.create);
+ txn = new CreateTxn(path, new byte[0], null, false);
+ }
+ logFile.processTransaction(txnHeader, dt, null, txn);
+
+ long newCversion = parent.stat.getCversion();
+ long newPzxid = parent.stat.getPzxid();
+ child = dt.getChildren(parentName, null, null);
+ childStr = "";
+ for (String s : child) {
+ childStr += s + " ";
+ }
+ Assert.assertTrue("<cversion, pzxid> verification failed. Expected: <" +
+ (prevCversion + 1) + ", " + (prevPzxid + 1) + ">, found: <" +
+ newCversion + ", " + newPzxid + ">",
+ (newCversion == prevCversion + 1 && newPzxid == prevPzxid + 1));
+ }
+}
Propchange: zookeeper/branches/branch-3.3/src/java/test/org/apache/zookeeper/test/LoadFromLogTest.java
------------------------------------------------------------------------------
svn:mime-type = text/plain