Skip to content

Commit

Permalink
ZOOKEEPER-1046: Creating a new sequential node results in a ZNODEEXIS…
Browse files Browse the repository at this point in the history 
…TS error

git-svn-id: https://svn.apache.org/repos/asf/zookeeper/trunk@1146025 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@skamille
skamille committed Jul 13, 2011
1 parent 9afd3a2 commit 18aae19
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 35 deletions.
2 changes: 2 additions & 0 deletions CHANGES.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,8 @@ BUGFIXES:
ZOOKEEPER-1046. Creating a new sequential node results in a ZNODEEXISTS error. (breed via camille)

ZOOKEEPER-1097. Quota is not correctly rehydrated on snapshot reload (camille via henryr)

ZOOKEEPER-1046. Small fix: Creating a new sequential node results in a ZNODEEXISTS error. (Vishal K via camille)

IMPROVEMENTS:
ZOOKEEPER-724. Improve junit test integration - log harness information
Expand Down
25 changes: 16 additions & 9 deletions src/java/main/org/apache/zookeeper/server/DataTree.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1299,20 +1299,22 @@ public void setWatches(long relativeZxid, List<String> dataWatches,
}

/**
* 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.
* This method sets the Cversion and Pzxid for the specified node to the
* values passed as arguments. The values are modified only if newCversion
* is greater than the current Cversion. A NoNodeException is thrown if
* a znode for the specified path is not found.
*
* @param path
* Full path to the znode whose cversion needs to be incremented.
* Full path to the znode whose Cversion needs to be modified.
* A "/" at the end of the path is ignored.
* @param newCversion
* Value to be assigned to Cversion
* @param zxid
* Value to be assigned to pzxid
* Value to be assigned to Pzxid
* @throws KeeperException.NoNodeException
* If znode not found.
**/
public void incrementCversion(String path, long zxid)
public void setCversionPzxid(String path, int newCversion, long zxid)
throws KeeperException.NoNodeException {
if (path.endsWith("/")) {
path = path.substring(0, path.length() - 1);
Expand All @@ -1322,8 +1324,13 @@ public void incrementCversion(String path, long zxid)
throw new KeeperException.NoNodeException(path);
}
synchronized (node) {
node.stat.setCversion(node.stat.getCversion() + 1);
node.stat.setPzxid(zxid);
if(newCversion == -1) {
newCversion = node.stat.getCversion() + 1;
}
if (newCversion > node.stat.getCversion()) {
node.stat.setCversion(newCversion);
node.stat.setPzxid(zxid);
}
}
}
}
33 changes: 16 additions & 17 deletions src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,19 @@
import java.util.concurrent.ConcurrentHashMap;

import org.apache.jute.Record;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.KeeperException.Code;
import org.apache.zookeeper.ZooDefs.OpCode;
import org.apache.zookeeper.server.DataTree;
import org.apache.zookeeper.server.DataTree.ProcessTxnResult;
import org.apache.zookeeper.server.Request;
import org.apache.zookeeper.server.ZooTrace;
import org.apache.zookeeper.server.persistence.TxnLog.TxnIterator;
import org.apache.zookeeper.txn.CreateSessionTxn;
import org.apache.zookeeper.txn.CreateTxn;
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;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* This is a helper class
Expand Down Expand Up @@ -201,10 +199,10 @@ public void processTransaction(TxnHeader hdr,DataTree dt,

/**
* Snapshots are taken lazily. It can happen that the child
* znodes of a parent are modified (deleted or created) after the parent
* znodes of a parent are created after the parent
* is serialized. Therefore, while replaying logs during restore, a
* delete/create might fail because the node was already
* deleted/created.
* create might fail because the node was already
* created.
*
* After seeing this failure, we should increment
* the cversion of the parent znode since the parent was serialized
Expand All @@ -213,17 +211,18 @@ public void processTransaction(TxnHeader hdr,DataTree dt,
* Note, such failures on DT should be seen only during
* restore.
*/
if ((hdr.getType() == OpCode.create &&
rc.err == Code.NODEEXISTS.intValue()) &&
((CreateTxn)txn).getParentCVersion() == -1) {
LOG.debug("Failed Txn: " + hdr.getType() + " path:" +
rc.path + " err: " + rc.err);
if (hdr.getType() == OpCode.create &&
rc.err == Code.NODEEXISTS.intValue()) {
LOG.debug("Adjusting parent cversion for Txn: " + hdr.getType() +
" path:" + rc.path + " err: " + rc.err);
int lastSlash = rc.path.lastIndexOf('/');
String parentName = rc.path.substring(0, lastSlash);
CreateTxn cTxn = (CreateTxn)txn;
try {
dt.incrementCversion(parentName, hdr.getZxid());
dt.setCversionPzxid(parentName, cTxn.getParentCVersion(),
hdr.getZxid());
} catch (KeeperException.NoNodeException e) {
LOG.error("Failed to increment parent cversion for: " +
LOG.error("Failed to set parent cversion for: " +
parentName, e);
throw e;
}
Expand Down
6 changes: 3 additions & 3 deletions src/java/test/org/apache/zookeeper/test/DataTreeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ public void process(WatchedEvent event) {
public void testIncrementCversion() throws Exception {
dt.createNode("/test", new byte[0], null, 0, dt.getNode("/").stat.getCversion()+1, 1, 1);
DataNode zk = dt.getNode("/test");
long prevCversion = zk.stat.getCversion();
int prevCversion = zk.stat.getCversion();
long prevPzxid = zk.stat.getPzxid();
dt.incrementCversion("/test/", prevPzxid + 1);
long newCversion = zk.stat.getCversion();
dt.setCversionPzxid("/test/", prevCversion + 1, prevPzxid + 1);
int newCversion = zk.stat.getCversion();
long newPzxid = zk.stat.getPzxid();
Assert.assertTrue("<cversion, pzxid> verification failed. Expected: <" +
(prevCversion + 1) + ", " + (prevPzxid + 1) + ">, found: <" +
Expand Down
16 changes: 10 additions & 6 deletions src/java/test/org/apache/zookeeper/test/LoadFromLogTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,11 @@ public void testTxnFailure() throws Exception {

// Make create to fail, then verify cversion.
LOG.info("Attempting to create " + "/test/" + (count - 1));
doOp(logFile, OpCode.create, "/test/" + (count - 1), dt, zk);
doOp(logFile, OpCode.create, "/test/" + (count - 1), dt, zk, -1);

LOG.info("Attempting to create " + "/test/" + (count - 1));
doOp(logFile, OpCode.create, "/test/" + (count - 1), dt, zk,
zk.stat.getCversion() + 1);

// Make delete fo fail, then verify cversion.
// this doesn't happen anymore, we only set the cversion on create
Expand All @@ -148,7 +152,7 @@ public void testTxnFailure() throws Exception {
* 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 {
DataTree dt, DataNode parent, int cversion) throws Exception {
int lastSlash = path.lastIndexOf('/');
String parentName = path.substring(0, lastSlash);

Expand All @@ -171,11 +175,11 @@ private void doOp(FileTxnSnapLog logFile, int type, String path,
} 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, -1);
txn = new CreateTxn(path, new byte[0], null, false, cversion);
}
logFile.processTransaction(txnHeader, dt, null, txn);

long newCversion = parent.stat.getCversion();
int newCversion = parent.stat.getCversion();
long newPzxid = parent.stat.getPzxid();
child = dt.getChildren(parentName, null, null);
childStr = "";
Expand Down Expand Up @@ -206,8 +210,8 @@ public void testPad() throws Exception {
BinaryInputArchive ia = BinaryInputArchive.getArchive(in);
FileHeader header = new FileHeader();
header.deserialize(ia, "fileheader");
LOG.info("Expected header :" + header.getMagic() +
" Received : " + FileTxnLog.TXNLOG_MAGIC);
LOG.info("Received magic : " + header.getMagic() +
" Expected : " + FileTxnLog.TXNLOG_MAGIC);
Assert.assertTrue("Missing magic number ",
header.getMagic() == FileTxnLog.TXNLOG_MAGIC);
}
Expand Down

0 comments on commit 18aae19

Please sign in to comment.