Author: ivank
Date: Thu Aug 22 15:15:09 2013
New Revision: 1516482
URL: http://svn.apache.org/r1516482
Log:
BOOKKEEPER-649: Race condition in sync ZKUtils.createFullPathOptimistic() (ivank)
Modified:
zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt
zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java
zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java
Modified: zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt?rev=1516482&r1=1516481&r2=1516482&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt (original)
+++ zookeeper/bookkeeper/branches/branch-4.2/CHANGES.txt Thu Aug 22 15:15:09 2013
@@ -66,6 +66,8 @@ Release 4.2.2 - Unreleased
BOOKKEEPER-632: AutoRecovery should consider read only bookies (vinay via ivank)
+ BOOKKEEPER-649: Race condition in sync ZKUtils.createFullPathOptimistic() (ivank)
+
hedwig-server:
BOOKKEEPER-579: TestSubAfterCloseSub was put in a wrong package (sijie via ivank)
Modified: zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java?rev=1516482&r1=1516481&r2=1516482&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
(original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java
Thu Aug 22 15:15:09 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/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java?rev=1516482&r1=1516481&r2=1516482&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
(original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java
Thu Aug 22 15:15:09 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/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java?rev=1516482&r1=1516481&r2=1516482&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
(original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java
Thu Aug 22 15:15:09 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/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java?rev=1516482&r1=1516481&r2=1516482&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java
(original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerUnderreplicationManager.java
Thu Aug 22 15:15:09 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(),
- 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/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java
URL: http://svn.apache.org/viewvc/zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java?rev=1516482&r1=1516481&r2=1516482&view=diff
==============================================================================
--- zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java
(original)
+++ zookeeper/bookkeeper/branches/branch-4.2/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/ZkUtils.java
Thu Aug 22 15:15:09 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);
- }
- }
}
|