zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From iv...@apache.org
Subject svn commit: r1516465 - in /zookeeper/bookkeeper/trunk: ./ bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ bookkeeper-server/src/main/java/org/apache/bookkeeper/util/
Date Thu, 22 Aug 2013 14:40:10 GMT
Author: ivank
Date: Thu Aug 22 14:40:10 2013
New Revision: 1516465

URL: http://svn.apache.org/r1516465
Log:
BOOKKEEPER-649: Race condition in sync ZKUtils.createFullPathOptimistic() (ivank)

Modified:
    zookeeper/bookkeeper/trunk/CHANGES.txt
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java
    zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java

Modified: zookeeper/bookkeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/CHANGES.txt?rev=1516465&r1=1516464&r2=1516465&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/CHANGES.txt (original)
+++ zookeeper/bookkeeper/trunk/CHANGES.txt Thu Aug 22 14:40:10 2013
@@ -94,6 +94,8 @@ Trunk (unreleased changes)
 
         BOOKKEEPER-632: AutoRecovery should consider read only bookies (vinay via ivank)
 
+        BOOKKEEPER-649: Race condition in sync ZKUtils.createFullPathOptimistic() (ivank)
+
       hedwig-server:
 
         BOOKKEEPER-601: readahead cache size isn't updated correctly (sijie via fpj)

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java?rev=1516465&r1=1516464&r2=1516465&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
Thu Aug 22 14:40:10 2013
@@ -93,7 +93,7 @@ class FlatLedgerManager extends Abstract
                 }
             }
         };
-        ZkUtils.createFullPathOptimistic(zk, ledgerPrefix, metadata.serialize(),
+        ZkUtils.asyncCreateFullPathOptimistic(zk, ledgerPrefix, metadata.serialize(),
             Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL, scb, null);
     }
 

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java?rev=1516465&r1=1516464&r2=1516465&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
Thu Aug 22 14:40:10 2013
@@ -102,7 +102,7 @@ class HierarchicalLedgerManager extends 
 
     @Override
     public void createLedger(final LedgerMetadata metadata, final GenericCallback<Long>
ledgerCb) {
-        ZkUtils.createFullPathOptimistic(zk, idGenPath, new byte[0], Ids.OPEN_ACL_UNSAFE,
+        ZkUtils.asyncCreateFullPathOptimistic(zk, idGenPath, new byte[0], Ids.OPEN_ACL_UNSAFE,
             CreateMode.EPHEMERAL_SEQUENTIAL, new StringCallback() {
             @Override
             public void processResult(int rc, String path, Object ctx, final String idPathName)
{
@@ -140,7 +140,7 @@ class HierarchicalLedgerManager extends 
                         }
                     }
                 };
-                ZkUtils.createFullPathOptimistic(zk, ledgerPath, metadata.serialize(),
+                ZkUtils.asyncCreateFullPathOptimistic(zk, ledgerPath, metadata.serialize(),
                     Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT, scb, null);
                 // delete the znode for id generation
                 scheduler.submit(new Runnable() {

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java?rev=1516465&r1=1516464&r2=1516465&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
Thu Aug 22 14:40:10 2013
@@ -216,7 +216,7 @@ public class MSLedgerManagerFactory exte
 
         @Override
         public void createLedger(final LedgerMetadata metadata, final GenericCallback<Long>
ledgerCb) {
-            ZkUtils.createFullPathOptimistic(zk, idGenPath, new byte[0], Ids.OPEN_ACL_UNSAFE,
+            ZkUtils.asyncCreateFullPathOptimistic(zk, idGenPath, new byte[0], Ids.OPEN_ACL_UNSAFE,
                     CreateMode.EPHEMERAL_SEQUENTIAL, new StringCallback() {
                         @Override
                         public void processResult(int rc, String path, Object ctx, final
String idPathName) {

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java?rev=1516465&r1=1516464&r2=1516465&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java
Thu Aug 22 14:40:10 2013
@@ -42,7 +42,6 @@ import com.google.protobuf.TextFormat;
 import com.google.common.base.Joiner;
 import static com.google.common.base.Charsets.UTF_8;
 
-import java.net.InetAddress;
 import java.net.UnknownHostException;
 
 import java.util.concurrent.CountDownLatch;
@@ -198,7 +197,7 @@ public class ZkLedgerUnderreplicationMan
         String subdir2 = String.format("%04x", ledgerId >> 32 & 0xffff);
         String subdir3 = String.format("%04x", ledgerId >> 16 & 0xffff);
         String subdir4 = String.format("%04x", ledgerId & 0xffff);
-        
+
         return String.format("%s/%s/%s/%s/%s",
                              base, subdir1, subdir2, subdir3, subdir4);
     }
@@ -430,7 +429,7 @@ public class ZkLedgerUnderreplicationMan
             cb.await();
         }
     }
-    
+
     @Override
     public void releaseUnderreplicatedLedger(long ledgerId) throws ReplicationException.UnavailableException
{
         LOG.debug("releaseLedger(ledgerId={})", ledgerId);
@@ -473,9 +472,8 @@ public class ZkLedgerUnderreplicationMan
             throws ReplicationException.UnavailableException {
         LOG.debug("disableLedegerReplication()");
         try {
-            ZkUtils.createFullPathOptimistic(zkc, basePath + '/'
-                    + BookKeeperConstants.DISABLE_NODE, "".getBytes(UTF_8),
-                    Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+            String znode = basePath + '/' + BookKeeperConstants.DISABLE_NODE;
+            zkc.create(znode, "".getBytes(UTF_8), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
             LOG.info("Auto ledger re-replication is disabled!");
         } catch (KeeperException.NodeExistsException ke) {
             LOG.warn("AutoRecovery is already disabled!", ke);

Modified: zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java?rev=1516465&r1=1516464&r2=1516465&view=diff
==============================================================================
--- zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java
(original)
+++ zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java
Thu Aug 22 14:40:10 2013
@@ -25,6 +25,9 @@ import java.io.File;
 import java.io.IOException;
 import java.util.List;
 
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicInteger;
+
 import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallback;
 import org.apache.bookkeeper.zookeeper.ZooKeeperWatcherBase;
 import org.apache.zookeeper.CreateMode;
@@ -42,8 +45,11 @@ import org.slf4j.LoggerFactory;
  */
 public class ZkUtils {
     private static final Logger LOG = LoggerFactory.getLogger(ZkUtils.class);
+
     /**
-     * Create zookeeper path recursively
+     * Asynchronously create zookeeper path recursively and optimistically.
+     *
+     * @see #createFullPathOptimistic(ZooKeeper,String,byte[],List<ACL>,CreateMode)
      *
      * @param zk
      *          Zookeeper client
@@ -60,7 +66,7 @@ public class ZkUtils {
      * @param ctx
      *          Context object
      */
-    public static void createFullPathOptimistic(
+    public static void asyncCreateFullPathOptimistic(
         final ZooKeeper zk, final String originalPath, final byte[] data,
         final List<ACL> acl, final CreateMode createMode,
         final AsyncCallback.StringCallback callback, final Object ctx) {
@@ -77,7 +83,8 @@ public class ZkUtils {
                 // Since I got a nonode, it means that my parents don't exist
                 // create mode is persistent since ephemeral nodes can't be
                 // parents
-                createFullPathOptimistic(zk, new File(originalPath).getParent().replace("\\",
"/"), new byte[0], acl,
+                String parent = new File(originalPath).getParent().replace("\\", "/");
+                asyncCreateFullPathOptimistic(zk, parent, new byte[0], acl,
                         CreateMode.PERSISTENT, new StringCallback() {
 
                             @Override
@@ -85,8 +92,8 @@ public class ZkUtils {
                                 if (rc == Code.OK.intValue() || rc == Code.NODEEXISTS.intValue())
{
                                     // succeeded in creating the parent, now
                                     // create the original path
-                                    createFullPathOptimistic(zk, originalPath, data, acl,
createMode, callback,
-                                            ctx);
+                                    asyncCreateFullPathOptimistic(zk, originalPath, data,
+                                            acl, createMode, callback, ctx);
                                 } else {
                                     callback.processResult(rc, path, ctx, name);
                                 }
@@ -94,7 +101,48 @@ public class ZkUtils {
                         }, ctx);
             }
         }, ctx);
+    }
 
+    /**
+     * Create zookeeper path recursively and optimistically. This method can throw
+     * any of the KeeperExceptions which can be thrown by ZooKeeper#create.
+     * KeeperException.NodeExistsException will only be thrown if the full path specified
+     * by _path_ already exists. The existence of any parent znodes is not an error
+     * condition.
+     *
+     * @param zkc
+     *            - ZK instance
+     * @param path
+     *            - znode path
+     * @param data
+     *            - znode data
+     * @param acl
+     *            - Acl of the zk path
+     * @param createMode
+     *            - Create mode of zk path
+     * @throws KeeperException
+     *             if the server returns a non-zero error code, or invalid ACL
+     * @throws InterruptedException
+     *             if the transaction is interrupted
+     */
+    public static void createFullPathOptimistic(ZooKeeper zkc, String path,
+            byte[] data, final List<ACL> acl, final CreateMode createMode)
+            throws KeeperException, InterruptedException {
+        final CountDownLatch latch = new CountDownLatch(1);
+        final AtomicInteger rc = new AtomicInteger(Code.OK.intValue());
+        asyncCreateFullPathOptimistic(zkc, path, data, acl, createMode,
+                                      new StringCallback() {
+                                          @Override
+                                          public void processResult(int rc2, String path,
+                                                                    Object ctx, String name)
{
+                                              rc.set(rc2);
+                                              latch.countDown();
+                                          }
+                                      }, null);
+        latch.await();
+        if (rc.get() != Code.OK.intValue()) {
+            throw KeeperException.create(Code.get(rc.get()));
+        }
     }
 
     private static class GetChildrenCtx {
@@ -206,38 +254,4 @@ public class ZkUtils {
         }
         return newZk;
     }
-
-    /**
-     * Utility to create the complete znode path synchronously
-     * 
-     * @param zkc
-     *            - ZK instance
-     * @param path
-     *            - znode path
-     * @param data
-     *            - znode data
-     * @param acl
-     *            - Acl of the zk path
-     * @param createMode
-     *            - Create mode of zk path
-     * @throws KeeperException
-     *             if the server returns a non-zero error code, or invalid ACL
-     * @throws InterruptedException
-     *             if the transaction is interrupted
-     */
-    public static void createFullPathOptimistic(ZooKeeper zkc, String path,
-            byte[] data, final List<ACL> acl, final CreateMode createMode)
-            throws KeeperException, InterruptedException {
-        try {
-            zkc.create(path, data, acl, createMode);
-        } catch (KeeperException.NoNodeException nne) {
-            int lastSlash = path.lastIndexOf('/');
-            if (lastSlash <= 0) {
-                throw nne;
-            }
-            String parent = path.substring(0, lastSlash);
-            createFullPathOptimistic(zkc, parent, new byte[0], acl, createMode);
-            zkc.create(path, data, acl, createMode);
-        }
-    }
 }



Mime
View raw message