curator-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cammcken...@apache.org
Subject git commit: CURATOR-42 - Modified guaranteed delete handling so that it will only add the node to the guaranteed delete set if a recoverable exception is encountered. If a non recoverable exception (such as a NoNodeException) is encountered, then this wi
Date Tue, 29 Jul 2014 22:13:47 GMT
Repository: curator
Updated Branches:
  refs/heads/master 9c0a61700 -> b174dfb14


CURATOR-42 - Modified guaranteed delete handling so that it will only
add the node to the guaranteed delete set if a recoverable exception is
encountered. If a non recoverable exception (such as a NoNodeException)
is encountered, then this will not be retried. Added a debug listener to
the FailedDeleteManager to facilitate unit testing this case. Added unit
tests, for both guaranteed deletes in the background and foreground for
the NoNodeException case. Note that this error was only present for
guaranteed deletes in the foreground.

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

Branch: refs/heads/master
Commit: b174dfb145ebaea51bcd348cec950997725e7b3f
Parents: 9c0a617
Author: Cameron McKenzie <cameron@unico.com.au>
Authored: Wed Jul 30 08:06:25 2014 +1000
Committer: Cameron McKenzie <cameron@unico.com.au>
Committed: Wed Jul 30 08:06:25 2014 +1000

----------------------------------------------------------------------
 .../framework/imps/DeleteBuilderImpl.java       | 10 +--
 .../framework/imps/FailedDeleteManager.java     | 13 +++
 .../framework/imps/TestFailedDeleteManager.java | 86 ++++++++++++++++++++
 3 files changed, 102 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/b174dfb1/curator-framework/src/main/java/org/apache/curator/framework/imps/DeleteBuilderImpl.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/DeleteBuilderImpl.java
b/curator-framework/src/main/java/org/apache/curator/framework/imps/DeleteBuilderImpl.java
index 319fce2..5d8b846 100644
--- a/curator-framework/src/main/java/org/apache/curator/framework/imps/DeleteBuilderImpl.java
+++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/DeleteBuilderImpl.java
@@ -27,7 +27,6 @@ import org.apache.curator.framework.api.ChildrenDeletable;
 import org.apache.curator.framework.api.CuratorEvent;
 import org.apache.curator.framework.api.CuratorEventType;
 import org.apache.curator.framework.api.DeleteBuilder;
-import org.apache.curator.framework.api.Guaranteeable;
 import org.apache.curator.framework.api.Pathable;
 import org.apache.curator.framework.api.transaction.CuratorTransactionBridge;
 import org.apache.curator.framework.api.transaction.OperationType;
@@ -248,14 +247,11 @@ class DeleteBuilderImpl implements DeleteBuilder, BackgroundOperation<String>
                     }
                 }
             );
-        }
-        catch ( KeeperException.NodeExistsException e )
-        {
-            throw e;
-        }
+        }      
         catch ( Exception e )
         {
-            if ( guaranteed )
+            //Only retry a guaranteed delete if it's a retryable error
+            if( RetryLoop.isRetryException(e) && guaranteed )
             {
                 client.getFailedDeleteManager().addFailedDelete(unfixedPath);
             }

http://git-wip-us.apache.org/repos/asf/curator/blob/b174dfb1/curator-framework/src/main/java/org/apache/curator/framework/imps/FailedDeleteManager.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/FailedDeleteManager.java
b/curator-framework/src/main/java/org/apache/curator/framework/imps/FailedDeleteManager.java
index d9f1771..f02e852 100644
--- a/curator-framework/src/main/java/org/apache/curator/framework/imps/FailedDeleteManager.java
+++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/FailedDeleteManager.java
@@ -26,6 +26,13 @@ class FailedDeleteManager
 {
     private final Logger log = LoggerFactory.getLogger(getClass());
     private final CuratorFramework client;
+    
+    volatile FailedDeleteManagerListener debugListener = null;
+    
+    interface FailedDeleteManagerListener
+    {
+       public void pathAddedForDelete(String path);
+    }
 
     FailedDeleteManager(CuratorFramework client)
     {
@@ -34,6 +41,12 @@ class FailedDeleteManager
 
     void addFailedDelete(String path)
     {
+        if ( debugListener != null )
+        {
+            debugListener.pathAddedForDelete(path);
+        }
+        
+        
         if ( client.isStarted() )
         {
             log.debug("Path being added to guaranteed delete set: " + path);

http://git-wip-us.apache.org/repos/asf/curator/blob/b174dfb1/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFailedDeleteManager.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFailedDeleteManager.java
b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFailedDeleteManager.java
index b684276..6599745 100644
--- a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFailedDeleteManager.java
+++ b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFailedDeleteManager.java
@@ -20,6 +20,9 @@ package org.apache.curator.framework.imps;
 
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.CuratorFrameworkFactory;
+import org.apache.curator.framework.api.BackgroundCallback;
+import org.apache.curator.framework.api.CuratorEvent;
+import org.apache.curator.framework.imps.FailedDeleteManager.FailedDeleteManagerListener;
 import org.apache.curator.framework.state.ConnectionState;
 import org.apache.curator.framework.state.ConnectionStateListener;
 import org.apache.curator.retry.ExponentialBackoffRetry;
@@ -28,10 +31,13 @@ import org.apache.curator.test.BaseClassForTests;
 import org.apache.curator.test.Timing;
 import org.apache.curator.utils.CloseableUtils;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.NoNodeException;
 import org.testng.Assert;
 import org.testng.annotations.Test;
+
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Semaphore;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 public class TestFailedDeleteManager extends BaseClassForTests
 {
@@ -276,4 +282,84 @@ public class TestFailedDeleteManager extends BaseClassForTests
             CloseableUtils.closeQuietly(client);
         }
     }
+    
+    @Test
+    public void testGuaranteedDeleteOnNonExistentNodeInForeground() throws Exception
+    {
+        CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(),
new RetryOneTime(1));
+        client.start();
+        
+        final AtomicBoolean pathAdded = new AtomicBoolean(false);
+        
+        ((CuratorFrameworkImpl)client).getFailedDeleteManager().debugListener = new FailedDeleteManagerListener()
+        {
+            
+            @Override
+            public void pathAddedForDelete(String path)
+            {
+                pathAdded.set(true);
+            }
+        };
+        
+        try
+        {
+            client.delete().guaranteed().forPath("/nonexistent");
+            Assert.fail();
+        }
+        catch(NoNodeException e)
+        {
+            //Exception is expected, the delete should not be retried
+            Assert.assertFalse(pathAdded.get());
+        }
+        finally
+        {
+            client.close();
+        }        
+    }
+    
+    @Test
+    public void testGuaranteedDeleteOnNonExistentNodeInBackground() throws Exception
+    {
+        CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(),
new RetryOneTime(1));
+        client.start();
+        
+        final AtomicBoolean pathAdded = new AtomicBoolean(false);
+        
+        ((CuratorFrameworkImpl)client).getFailedDeleteManager().debugListener = new FailedDeleteManagerListener()
+        {
+            
+            @Override
+            public void pathAddedForDelete(String path)
+            {
+                pathAdded.set(true);
+            }
+        };
+        
+        final CountDownLatch backgroundLatch = new CountDownLatch(1);
+        
+        BackgroundCallback background = new BackgroundCallback()
+        {
+            
+            @Override
+            public void processResult(CuratorFramework client, CuratorEvent event)
+                    throws Exception
+            {
+                backgroundLatch.countDown();
+            }
+        };
+        
+        try
+        {
+            client.delete().guaranteed().inBackground(background).forPath("/nonexistent");
+            
+            backgroundLatch.await();
+            
+            //Exception is expected, the delete should not be retried
+            Assert.assertFalse(pathAdded.get());
+        }
+        finally
+        {
+            client.close();
+        }        
+    }    
 }


Mime
View raw message