curator-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From randg...@apache.org
Subject curator git commit: CURATOR-45 added findAndDeleteProtectedNodeInBackground to handle cases where a protected node can get lost. However, the code wasn't correctly handling namespaces
Date Tue, 22 Sep 2015 19:59:53 GMT
Repository: curator
Updated Branches:
  refs/heads/CURATOR-264 [created] f8f05be2e


CURATOR-45 added findAndDeleteProtectedNodeInBackground to handle cases where a protected
node can get lost. However, the code wasn't correctly handling namespaces


Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/f8f05be2
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/f8f05be2
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/f8f05be2

Branch: refs/heads/CURATOR-264
Commit: f8f05be2e097c4c9be65e5110a376d461fd9cd9a
Parents: c8cc3f4
Author: randgalt <randgalt@apache.org>
Authored: Tue Sep 22 14:59:41 2015 -0500
Committer: randgalt <randgalt@apache.org>
Committed: Tue Sep 22 14:59:41 2015 -0500

----------------------------------------------------------------------
 .../framework/imps/CreateBuilderImpl.java       | 24 +++++------
 .../framework/imps/TestFrameworkEdges.java      | 42 +++++++++++++++++++-
 2 files changed, 53 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/f8f05be2/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
b/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
index 7a4a96f..b72b7b6 100644
--- a/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
+++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
@@ -464,13 +464,13 @@ class CreateBuilderImpl implements CreateBuilder, BackgroundOperation<PathAndByt
         }
         else
         {
-            String path = protectedPathInForeground(adjustedPath, data);
+            String path = protectedPathInForeground(givenPath, adjustedPath, data);
             returnPath = client.unfixForNamespace(path);
         }
         return returnPath;
     }
 
-    private String protectedPathInForeground(String adjustedPath, byte[] data) throws Exception
+    private String protectedPathInForeground(String givenPath, String adjustedPath, byte[]
data) throws Exception
     {
         try
         {
@@ -486,7 +486,7 @@ class CreateBuilderImpl implements CreateBuilder, BackgroundOperation<PathAndByt
                  * register the znode to be sure it is deleted later.
                  */
                 String localProtectedId = protectedId;
-                findAndDeleteProtectedNodeInBackground(adjustedPath, localProtectedId, null);
+                findAndDeleteProtectedNodeInBackground(givenPath, localProtectedId, null);
                 /*
                 * The current UUID is scheduled to be deleted, it is not safe to use it again.
                 * If this builder is used again later create a new UUID
@@ -635,7 +635,7 @@ class CreateBuilderImpl implements CreateBuilder, BackgroundOperation<PathAndByt
                     if ( doProtected )
                     {
                         // all retries have failed, findProtectedNodeInForeground(..) included,
schedule a clean up
-                        findAndDeleteProtectedNodeInBackground(path, protectedId, null);
+                        findAndDeleteProtectedNodeInBackground(givenPath, protectedId, null);
                         // assign a new id if this builder is used again later
                         protectedId = UUID.randomUUID().toString();
                     }
@@ -795,25 +795,25 @@ class CreateBuilderImpl implements CreateBuilder, BackgroundOperation<PathAndByt
     /**
      * Attempt to delete a protected znode
      *
-     * @param path        the path
-     * @param protectedId the protected id
-     * @param callback    callback to use, <code>null</code> to create a new
one
+     * @param unAdjustedPath the path - raw without namespace resolution
+     * @param protectedId    the protected id
+     * @param callback       callback to use, <code>null</code> to create a new
one
      */
-    private void findAndDeleteProtectedNodeInBackground(String path, String protectedId,
FindProtectedNodeCB callback)
+    private void findAndDeleteProtectedNodeInBackground(String unAdjustedPath, String protectedId,
FindProtectedNodeCB callback)
     {
         if ( client.getState() == CuratorFrameworkState.STARTED )
         {
             if ( callback == null )
             {
-                callback = new FindProtectedNodeCB(path, protectedId);
+                callback = new FindProtectedNodeCB(unAdjustedPath, protectedId);
             }
             try
             {
-                client.getChildren().inBackground(callback).forPath(ZKPaths.getPathAndNode(path).getPath());
+                client.getChildren().inBackground(callback).forPath(ZKPaths.getPathAndNode(unAdjustedPath).getPath());
             }
             catch ( Exception e )
             {
-                findAndDeleteProtectedNodeInBackground(path, protectedId, callback);
+                findAndDeleteProtectedNodeInBackground(unAdjustedPath, protectedId, callback);
             }
         }
     }
@@ -830,7 +830,7 @@ class CreateBuilderImpl implements CreateBuilder, BackgroundOperation<PathAndByt
         }
 
         @Override
-        public void processResult(CuratorFramework client, CuratorEvent event) throws Exception
+        public void processResult(CuratorFramework ignoreClient, CuratorEvent event) throws
Exception
         {
             if ( event.getResultCode() == KeeperException.Code.OK.intValue() )
             {

http://git-wip-us.apache.org/repos/asf/curator/blob/f8f05be2/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java
b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java
index cd3ae77..95c3792 100644
--- a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java
+++ b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java
@@ -29,10 +29,10 @@ import org.apache.curator.framework.api.CuratorEventType;
 import org.apache.curator.framework.api.CuratorListener;
 import org.apache.curator.framework.state.ConnectionState;
 import org.apache.curator.framework.state.ConnectionStateListener;
+import org.apache.curator.retry.RetryNTimes;
 import org.apache.curator.retry.RetryOneTime;
 import org.apache.curator.test.BaseClassForTests;
 import org.apache.curator.test.KillSession;
-import org.apache.curator.test.TestingServer;
 import org.apache.curator.test.Timing;
 import org.apache.curator.utils.CloseableUtils;
 import org.apache.curator.utils.ZKPaths;
@@ -43,6 +43,7 @@ import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.data.Stat;
 import org.testng.Assert;
 import org.testng.annotations.Test;
+import java.util.List;
 import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.CountDownLatch;
@@ -56,6 +57,45 @@ public class TestFrameworkEdges extends BaseClassForTests
     private final Timing timing = new Timing();
 
     @Test
+    public void testProtectedCreateNodeDeletion() throws Exception
+    {
+        CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(),
timing.session(), 1, new RetryNTimes(0, 0));
+        try
+        {
+            client.start();
+
+            for ( int i = 0; i < 2; ++i )
+            {
+                CuratorFramework localClient = (i == 0) ? client : client.usingNamespace("nm");
+                localClient.create().forPath("/parent");
+                Assert.assertEquals(localClient.getChildren().forPath("/parent").size(),
0);
+
+                CreateBuilderImpl createBuilder = (CreateBuilderImpl)localClient.create();
+                createBuilder.failNextCreateForTesting = true;
+                try
+                {
+                    createBuilder.withProtection().forPath("/parent/test");
+                    Assert.fail("failNextCreateForTesting should have caused a ConnectionLossException");
+                }
+                catch ( KeeperException.ConnectionLossException e )
+                {
+                    // ignore, correct
+                }
+
+                timing.sleepABit();
+                List<String> children = localClient.getChildren().forPath("/parent");
+                Assert.assertEquals(children.size(), 0, children.toString()); // protected
mode should have deleted the node
+
+                localClient.delete().forPath("/parent");
+            }
+        }
+        finally
+        {
+            CloseableUtils.closeQuietly(client);
+        }
+    }
+
+    @Test
     public void testPathsFromProtectingInBackground() throws Exception
     {
         for ( CreateMode mode : CreateMode.values() )


Mime
View raw message