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 2013/10/10 20:06:59 UTC

svn commit: r1531063 - in /zookeeper/branches/branch-3.4: CHANGES.txt src/c/tests/TestMulti.cc src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java

Author: camille
Date: Thu Oct 10 18:06:58 2013
New Revision: 1531063

URL: http://svn.apache.org/r1531063
Log:
ZOOKEEPER-1624. PrepRequestProcessor abort multi-operation incorrectly. (thawan via camille)

Modified:
    zookeeper/branches/branch-3.4/CHANGES.txt
    zookeeper/branches/branch-3.4/src/c/tests/TestMulti.cc
    zookeeper/branches/branch-3.4/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java

Modified: zookeeper/branches/branch-3.4/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/branches/branch-3.4/CHANGES.txt?rev=1531063&r1=1531062&r2=1531063&view=diff
==============================================================================
--- zookeeper/branches/branch-3.4/CHANGES.txt (original)
+++ zookeeper/branches/branch-3.4/CHANGES.txt Thu Oct 10 18:06:58 2013
@@ -127,6 +127,8 @@ BUGFIXES:
 
   ZOOKEEPER-877. zkpython does not work with python3.1
   (Daniel Enman via phunt)
+  
+  ZOOKEEPER-1624. PrepRequestProcessor abort multi-operation incorrectly. (thawan via camille)
 
 IMPROVEMENTS:
 

Modified: zookeeper/branches/branch-3.4/src/c/tests/TestMulti.cc
URL: http://svn.apache.org/viewvc/zookeeper/branches/branch-3.4/src/c/tests/TestMulti.cc?rev=1531063&r1=1531062&r2=1531063&view=diff
==============================================================================
--- zookeeper/branches/branch-3.4/src/c/tests/TestMulti.cc (original)
+++ zookeeper/branches/branch-3.4/src/c/tests/TestMulti.cc Thu Oct 10 18:06:58 2013
@@ -177,6 +177,7 @@ class Zookeeper_multi : public CPPUNIT_N
     CPPUNIT_TEST(testMultiFail);
     CPPUNIT_TEST(testCheck);
     CPPUNIT_TEST(testWatch);
+    CPPUNIT_TEST(testSequentialNodeCreateInAsyncMulti);
 #endif
     CPPUNIT_TEST_SUITE_END();
 
@@ -244,6 +245,10 @@ public:
         count++;
     }
 
+    static void multi_completion_fn_no_assert(int rc, const void *data) {
+        count++;
+    }
+
     static void waitForMultiCompletion(int seconds) {
         time_t expires = time(0) + seconds;
         while(count == 0 && time(0) < expires) {
@@ -252,6 +257,10 @@ public:
         count--;
     }
 
+    static void resetCounter() {
+        count = 0;
+    }
+
     /**
      * Test basic multi-op create functionality 
      */
@@ -646,6 +655,47 @@ public:
         // wait for multi completion in doMultiInWatch
         waitForMultiCompletion(5);
      }
+
+     /**
+      * ZOOKEEPER-1624: PendingChanges of create sequential node request didn't
+      * get rollbacked correctly when multi-op failed. This caused
+      * create sequential node request in subsequent multi-op to failed because
+      * sequential node name generation is incorrect.
+      *
+      * The check is to make sure that each request in multi-op failed with
+      * the correct reason.
+      */
+     void testSequentialNodeCreateInAsyncMulti() {
+         int rc;
+         watchctx_t ctx;
+         zhandle_t *zk = createClient(&ctx);
+
+         int iteration = 4;
+         int nops = 2;
+
+         zoo_op_result_t results[iteration][nops];
+         zoo_op_t ops[nops];
+         zoo_create_op_init(&ops[0], "/node-",   "", 0, &ZOO_OPEN_ACL_UNSAFE, ZOO_SEQUENCE, NULL, 0);
+         zoo_create_op_init(&ops[1], "/dup", "", 0, &ZOO_OPEN_ACL_UNSAFE, 0, NULL, 0);
+         for (int i = 0; i < iteration ; ++i) {
+           rc = zoo_amulti(zk, nops, ops, results[i], multi_completion_fn_no_assert, 0);
+           CPPUNIT_ASSERT_EQUAL((int)ZOK, rc);
+         }
+
+         waitForMultiCompletion(10);
+
+         CPPUNIT_ASSERT_EQUAL((int)ZOK, results[0][0].err);
+         CPPUNIT_ASSERT_EQUAL((int)ZOK, results[1][0].err);
+         CPPUNIT_ASSERT_EQUAL((int)ZOK, results[2][0].err);
+         CPPUNIT_ASSERT_EQUAL((int)ZOK, results[3][0].err);
+
+         CPPUNIT_ASSERT_EQUAL((int)ZOK, results[0][1].err);
+         CPPUNIT_ASSERT_EQUAL((int)ZNODEEXISTS, results[1][1].err);
+         CPPUNIT_ASSERT_EQUAL((int)ZNODEEXISTS, results[2][1].err);
+         CPPUNIT_ASSERT_EQUAL((int)ZNODEEXISTS, results[3][1].err);
+
+         resetCounter();
+     }
 };
 
 volatile int Zookeeper_multi::count;

Modified: zookeeper/branches/branch-3.4/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
URL: http://svn.apache.org/viewvc/zookeeper/branches/branch-3.4/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java?rev=1531063&r1=1531062&r2=1531063&view=diff
==============================================================================
--- zookeeper/branches/branch-3.4/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java (original)
+++ zookeeper/branches/branch-3.4/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java Thu Oct 10 18:06:58 2013
@@ -198,10 +198,28 @@ public class PrepRequestProcessor extend
     		String path = op.getPath();
 
     		try {
-    			ChangeRecord cr = getRecordForPath(path);
-    			if (cr != null) {
-    				pendingChangeRecords.put(path, cr);
-    			}
+    		    ChangeRecord cr = getRecordForPath(path);
+    		    if (cr != null) {
+    		        pendingChangeRecords.put(path, cr);
+    		    }
+    		    /*
+    		     * ZOOKEEPER-1624 - We need to store for parent's ChangeRecord
+    		     * of the parent node of a request. So that if this is a
+    		     * sequential node creation request, rollbackPendingChanges()
+    		     * can restore previous parent's ChangeRecord correctly.
+    		     *
+    		     * Otherwise, sequential node name generation will be incorrect
+    		     * for a subsequent request.
+    		     */
+    		    int lastSlash = path.lastIndexOf('/');
+    		    if (lastSlash == -1 || path.indexOf('\0') != -1) {
+    		        continue;
+    		    }
+    		    String parentPath = path.substring(0, lastSlash);
+    		    ChangeRecord parentCr = getRecordForPath(parentPath);
+    		    if (parentCr != null) {
+    		        pendingChangeRecords.put(parentPath, parentCr);
+    		    }
     		} catch (KeeperException.NoNodeException e) {
     			// ignore this one
     		}