ZOOKEEPER-1269. Multi deserialization issues. (Camille Fournier via mahadev)

git-svn-id: https://svn.apache.org/repos/asf/zookeeper/trunk@1212663 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -76,6 +76,8 @@ BUGFIXES:
 
   ZOOKEEPER-1319. Missing data after restarting+expanding a cluster.
   (phunt and breed via mahadev)
+
+  ZOOKEEPER-1269. Multi deserialization issues. (Camille Fournier via mahadev)
  
 IMPROVEMENTS:
 

src/java/main/org/apache/zookeeper/server/DataTree.java

@@ -872,6 +872,39 @@ public class DataTree {
         if (rc.zxid > lastProcessedZxid) {
             lastProcessedZxid = rc.zxid;
         }
+        /**
+         * Snapshots are taken lazily. It can happen that the child
+         * znodes of a parent are created after the parent
+         * is serialized. Therefore, while replaying logs during restore, a
+         * 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
+         * before its children.
+         *
+         * Note, such failures on DT should be seen only during
+         * restore.
+         */
+        if (header.getType() == OpCode.create &&
+                rc.err == Code.NODEEXISTS.intValue()) {
+            LOG.debug("Adjusting parent cversion for Txn: " + header.getType() +
+                    " path:" + rc.path + " err: " + rc.err);
+            int lastSlash = rc.path.lastIndexOf('/');
+            String parentName = rc.path.substring(0, lastSlash);
+            CreateTxn cTxn = (CreateTxn)txn;
+            try {
+                setCversionPzxid(parentName, cTxn.getParentCVersion(),
+                        header.getZxid());
+            } catch (KeeperException.NoNodeException e) {
+                LOG.error("Failed to set parent cversion for: " +
+                      parentName, e);
+                rc.err = e.code().intValue();
+            }
+        } else if (rc.err != Code.OK.intValue()) {
+            LOG.debug("Ignoring processTxn failure hdr: " + header.getType() +
+                  " : error: " + rc.err);
+        }
         return rc;
     }
 

src/java/main/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java

@@ -197,39 +197,20 @@ public class FileTxnSnapLog {
             rc = dt.processTxn(hdr, txn);
         }
 
-        /**
-         * Snapshots are taken lazily. It can happen that the child
-         * znodes of a parent are created after the parent
-         * is serialized. Therefore, while replaying logs during restore, a
-         * 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
-         * before its children.
-         *
-         * Note, such failures on DT should be seen only during
-         * restore.
-         */
-        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.setCversionPzxid(parentName, cTxn.getParentCVersion(),
-                        hdr.getZxid());
-            } catch (KeeperException.NoNodeException e) {
+              
+        if(rc.err !=  Code.OK.intValue()) {          
+            if(rc.err == Code.NONODE.intValue()) {
+                int lastSlash = rc.path.lastIndexOf('/');
+                String parentName = rc.path.substring(0, lastSlash);
                 LOG.error("Failed to set parent cversion for: " +
-                      parentName, e);
-                throw e;
+                        parentName);
+                  throw new KeeperException.NoNodeException(parentName);
             }
-        } else if (rc.err != Code.OK.intValue()) {
-            LOG.debug("Ignoring processTxn failure hdr: " + hdr.getType() +
-                  " : error: " + rc.err);
-        }
+            else {
+                LOG.debug("Ignoring processTxn failure hdr: " + hdr.getType() +
+                        " : error: " + rc.err);
+            }
+        }      
     }
 
     /**

src/java/test/org/apache/zookeeper/test/LoadFromLogTest.java

@@ -18,12 +18,15 @@
 
 package org.apache.zookeeper.test;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.util.ArrayList;
 import java.util.List;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.MultiTransactionRecord;
 import org.apache.zookeeper.PortAssignment;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
@@ -43,9 +46,13 @@ import org.apache.zookeeper.server.DataTree;
 import org.apache.zookeeper.server.DataNode;
 import org.apache.zookeeper.txn.CreateTxn;
 import org.apache.zookeeper.txn.DeleteTxn;
+import org.apache.zookeeper.txn.MultiTxn;
+import org.apache.zookeeper.txn.Txn;
 import org.apache.zookeeper.ZooDefs.OpCode;
+import org.apache.jute.BinaryOutputArchive;
 import org.apache.jute.Record;
 import java.io.FileInputStream;
+import java.nio.ByteBuffer;
 
 import org.apache.jute.BinaryInputArchive;
 import org.apache.zookeeper.server.persistence.FileHeader;
@@ -157,6 +164,14 @@ public class LoadFromLogTest extends ZKTestCase implements  Watcher {
         LOG.info("Attempting to create " + "/test/" + (count - 1));
         doOp(logFile, OpCode.create, "/test/" + (count - 1), dt, zk,
                 zk.stat.getCversion() + 1);
+        
+        LOG.info("Attempting to create " + "/test/" + (count - 1));
+        doOp(logFile, OpCode.multi, "/test/" + (count - 1), dt, zk,
+                zk.stat.getCversion() + 1);
+        
+        LOG.info("Attempting to create " + "/test/" + (count - 1));
+        doOp(logFile, OpCode.multi, "/test/" + (count - 1), dt, zk,
+                -1);
 
         // Make delete fo fail, then verify cversion.
         // this doesn't happen anymore, we only set the cversion on create
@@ -193,6 +208,21 @@ public class LoadFromLogTest extends ZKTestCase implements  Watcher {
                     System.currentTimeMillis(), OpCode.create);
             txn = new CreateTxn(path, new byte[0], null, false, cversion);
         }
+        else if (type == OpCode.multi) {
+            txnHeader = new TxnHeader(0xabcd, 0x123, prevPzxid + 1,
+                    System.currentTimeMillis(), OpCode.create);
+            txn = new CreateTxn(path, new byte[0], null, false, cversion);                       
+            ArrayList txnList = new ArrayList();
+            ByteArrayOutputStream baos = new ByteArrayOutputStream();
+            BinaryOutputArchive boa = BinaryOutputArchive.getArchive(baos);
+            txn.serialize(boa, "request") ;
+            ByteBuffer bb = ByteBuffer.wrap(baos.toByteArray());
+            Txn txact = new Txn(OpCode.create,  bb.array());
+            txnList.add(txact);
+            txn = new MultiTxn(txnList);
+            txnHeader = new TxnHeader(0xabcd, 0x123, prevPzxid + 1,
+                    System.currentTimeMillis(), OpCode.multi);
+        }
         logFile.processTransaction(txnHeader, dt, null, txn);
 
         int newCversion = parent.stat.getCversion();