You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ra...@apache.org on 2014/05/23 14:52:55 UTC

[1/8] git commit: Lock is leaving dangling watchers for nodes that don't exist.

Repository: curator
Updated Branches:
  refs/heads/master 351c67c97 -> 0dcf83357


Lock is leaving dangling watchers for nodes that don't exist.


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

Branch: refs/heads/master
Commit: 8a94985c07b3e6f74ac1a2935af008cbdb649410
Parents: 351c67c
Author: Dominic Wong <do...@hailocab.com>
Authored: Wed May 21 13:20:55 2014 +0100
Committer: Dominic Wong <do...@hailocab.com>
Committed: Wed May 21 13:20:55 2014 +0100

----------------------------------------------------------------------
 .../apache/curator/framework/recipes/locks/LockInternals.java    | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/8a94985c/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
index 06a1f6b..4b0d085 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
@@ -295,8 +295,8 @@ public class LockInternals
 
                     synchronized(this)
                     {
-                        Stat stat = client.checkExists().usingWatcher(watcher).forPath(previousSequencePath);
-                        if ( stat != null )
+                        byte[] data = client.getData().usingWatcher(watcher).forPath(previousSequencePath);
+                        if ( data != null )
                         {
                             if ( millisToWait != null )
                             {


[8/8] git commit: use getData() instead of exists() to avoid leaving unneeded watchers which is a type of resource leak. However, this might have performance affects. Therefore, added system proprety "curator-path-children-cache-use-exists" to use the ol

Posted by ra...@apache.org.
use getData() instead of exists() to avoid leaving unneeded watchers which is a type of resource leak. However, this might have
performance affects. Therefore, added system proprety "curator-path-children-cache-use-exists" to use the old behavior. i.e.
add -Dcurator-path-children-cache-use-exists=true or call System.setProperty("curator-path-children-cache-use-exists", "true") to
use the old behavior of calling exists() instead.


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

Branch: refs/heads/master
Commit: 0dcf8335745003c578dbaf09b3be658b89b4a045
Parents: 91d6503
Author: randgalt <ra...@apache.org>
Authored: Fri May 23 07:48:11 2014 -0500
Committer: randgalt <ra...@apache.org>
Committed: Fri May 23 07:48:11 2014 -0500

----------------------------------------------------------------------
 .../recipes/cache/PathChildrenCache.java        | 30 ++++++++------------
 1 file changed, 12 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/0dcf8335/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
index 855d060..dc23e04 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/PathChildrenCache.java
@@ -89,6 +89,8 @@ public class PathChildrenCache implements Closeable
 
     private static final ChildData NULL_CHILD_DATA = new ChildData(null, null, null);
 
+    private static final boolean USE_EXISTS = Boolean.getBoolean("curator-path-children-cache-use-exists");
+
     private final Watcher childrenWatcher = new Watcher()
     {
         @Override
@@ -508,39 +510,31 @@ public class PathChildrenCache implements Closeable
 
     void getDataAndStat(final String fullPath) throws Exception
     {
-        BackgroundCallback existsCallback = new BackgroundCallback()
+        BackgroundCallback callback = new BackgroundCallback()
         {
             @Override
             public void processResult(CuratorFramework client, CuratorEvent event) throws Exception
             {
-                applyNewData(fullPath, event.getResultCode(), event.getStat(), null);
+                applyNewData(fullPath, event.getResultCode(), event.getStat(), cacheData ? event.getData() : null);
             }
         };
 
-        BackgroundCallback getDataCallback = new BackgroundCallback()
+        if ( USE_EXISTS && !cacheData )
         {
-            @Override
-            public void processResult(CuratorFramework client, CuratorEvent event) throws Exception
-            {
-                applyNewData(fullPath, event.getResultCode(), event.getStat(), event.getData());
-            }
-        };
-
-        if ( cacheData )
+            client.checkExists().usingWatcher(dataWatcher).inBackground(callback).forPath(fullPath);
+        }
+        else
         {
-            if ( dataIsCompressed )
+            // always use getData() instead of exists() to avoid leaving unneeded watchers which is a type of resource leak
+            if ( dataIsCompressed && cacheData )
             {
-                client.getData().decompressed().usingWatcher(dataWatcher).inBackground(getDataCallback).forPath(fullPath);
+                client.getData().decompressed().usingWatcher(dataWatcher).inBackground(callback).forPath(fullPath);
             }
             else
             {
-                client.getData().usingWatcher(dataWatcher).inBackground(getDataCallback).forPath(fullPath);
+                client.getData().usingWatcher(dataWatcher).inBackground(callback).forPath(fullPath);
             }
         }
-        else
-        {
-            client.checkExists().usingWatcher(dataWatcher).inBackground(existsCallback).forPath(fullPath);
-        }
     }
 
     /**


[2/8] git commit: Catch no node exception

Posted by ra...@apache.org.
Catch no node exception


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

Branch: refs/heads/master
Commit: c9716499642b2dd4a0d77e5df6f9b84e8dfc90d0
Parents: 8a94985
Author: Dominic Wong <do...@hailocab.com>
Authored: Wed May 21 13:40:39 2014 +0100
Committer: Dominic Wong <do...@hailocab.com>
Committed: Wed May 21 13:40:39 2014 +0100

----------------------------------------------------------------------
 .../framework/recipes/locks/LockInternals.java  | 42 +++++++++++++-------
 1 file changed, 27 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/c9716499/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
index 4b0d085..c66e4d1 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
@@ -295,29 +295,41 @@ public class LockInternals
 
                     synchronized(this)
                     {
-                        byte[] data = client.getData().usingWatcher(watcher).forPath(previousSequencePath);
-                        if ( data != null )
+                        try 
                         {
-                            if ( millisToWait != null )
+                            byte[] data = client.getData().usingWatcher(watcher).forPath(previousSequencePath);
+                            if ( data != null )
                             {
-                                millisToWait -= (System.currentTimeMillis() - startMillis);
-                                startMillis = System.currentTimeMillis();
-                                if ( millisToWait <= 0 )
+                                if ( millisToWait != null )
                                 {
-                                    doDelete = true;    // timed out - delete our node
-                                    break;
-                                }
+                                    millisToWait -= (System.currentTimeMillis() - startMillis);
+                                    startMillis = System.currentTimeMillis();
+                                    if ( millisToWait <= 0 )
+                                    {
+                                        doDelete = true;    // timed out - delete our node
+                                        break;
+                                    }
 
-                                wait(millisToWait);
-                            }
-                            else
-                            {
-                                wait();
+                                    wait(millisToWait);
+                                }
+                                else
+                                {
+                                    wait();
+                                }
                             }
                         }
+                        catch ( KeeperException.NoNodeException e ) 
+                        {
+                            // ignore - clearly already deleted
+                        }
+                        catch ( Exception e) 
+                        {
+                            // bubble it up
+                            throw e;
+                        }
                     }
-                    // else it may have been deleted (i.e. lock released). Try to acquire again
                 }
+                // else it may have been deleted (i.e. lock released). Try to acquire again
             }
         }
         catch ( Exception e )


[4/8] git commit: data isn't used

Posted by ra...@apache.org.
data isn't used


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

Branch: refs/heads/master
Commit: c6700f0bea33c3daf5fb0f5f5890fae627cf908e
Parents: ac57e11
Author: randgalt <ra...@apache.org>
Authored: Thu May 22 17:54:06 2014 -0500
Committer: randgalt <ra...@apache.org>
Committed: Thu May 22 17:54:06 2014 -0500

----------------------------------------------------------------------
 .../apache/curator/framework/recipes/locks/LockInternals.java   | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/c6700f0b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
index d6298ce..0f7460f 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
@@ -26,13 +26,12 @@ import org.apache.curator.RetryLoop;
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.api.CuratorWatcher;
 import org.apache.curator.framework.imps.CuratorFrameworkState;
+import org.apache.curator.utils.PathUtils;
 import org.apache.curator.utils.ZKPaths;
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.curator.utils.PathUtils;
-import org.apache.zookeeper.data.Stat;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -297,7 +296,7 @@ public class LockInternals
                     {
                         try 
                         {
-                            byte[] data = client.getData().usingWatcher(watcher).forPath(previousSequencePath);
+                            client.getData().usingWatcher(watcher).forPath(previousSequencePath);
                             if ( millisToWait != null )
                             {
                                 millisToWait -= (System.currentTimeMillis() - startMillis);


[5/8] git commit: updated comments

Posted by ra...@apache.org.
updated comments


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

Branch: refs/heads/master
Commit: 4e548a29e60c3de1809db4eedf320e38f925671e
Parents: c6700f0
Author: randgalt <ra...@apache.org>
Authored: Fri May 23 07:25:00 2014 -0500
Committer: randgalt <ra...@apache.org>
Committed: Fri May 23 07:25:00 2014 -0500

----------------------------------------------------------------------
 .../curator/framework/recipes/locks/LockInternals.java      | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/4e548a29/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
index 0f7460f..812cfd8 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
@@ -296,6 +296,7 @@ public class LockInternals
                     {
                         try 
                         {
+                            // use getData instead of exists to avoid leaving unneeded watchers which is a type of resource leak
                             client.getData().usingWatcher(watcher).forPath(previousSequencePath);
                             if ( millisToWait != null )
                             {
@@ -316,16 +317,10 @@ public class LockInternals
                         }
                         catch ( KeeperException.NoNodeException e ) 
                         {
-                            // ignore - clearly already deleted
-                        }
-                        catch ( Exception e) 
-                        {
-                            // bubble it up
-                            throw e;
+                            // it has been deleted (i.e. lock released). Try to acquire again
                         }
                     }
                 }
-                // else it may have been deleted (i.e. lock released). Try to acquire again
             }
         }
         catch ( Exception e )


[7/8] git commit: use getData() instead of exists() to avoid leaving unneeded watchers which is a type of resource leak

Posted by ra...@apache.org.
use getData() instead of exists() to avoid leaving unneeded watchers which is a type of resource leak


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

Branch: refs/heads/master
Commit: 91d65037bf28947b5b28b75ec83614bb70a3e12a
Parents: 290186a
Author: randgalt <ra...@apache.org>
Authored: Fri May 23 07:47:56 2014 -0500
Committer: randgalt <ra...@apache.org>
Committed: Fri May 23 07:47:56 2014 -0500

----------------------------------------------------------------------
 .../org/apache/curator/framework/recipes/leader/LeaderLatch.java  | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/91d65037/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java
index ecdd903..88456af 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java
@@ -519,7 +519,8 @@ public class LeaderLatch implements Closeable
                     }
                 }
             };
-            client.checkExists().usingWatcher(watcher).inBackground(callback).forPath(ZKPaths.makePath(latchPath, watchPath));
+            // use getData() instead of exists() to avoid leaving unneeded watchers which is a type of resource leak
+            client.getData().usingWatcher(watcher).inBackground(callback).forPath(ZKPaths.makePath(latchPath, watchPath));
         }
     }
 


[3/8] git commit: No need for null check

Posted by ra...@apache.org.
No need for null check


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

Branch: refs/heads/master
Commit: ac57e11b34bb2970775b385f5a1e94bccd287232
Parents: c971649
Author: Dominic Wong <do...@hailocab.com>
Authored: Wed May 21 13:45:54 2014 +0100
Committer: Dominic Wong <do...@hailocab.com>
Committed: Wed May 21 13:45:54 2014 +0100

----------------------------------------------------------------------
 .../framework/recipes/locks/LockInternals.java  | 27 +++++++++-----------
 1 file changed, 12 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/ac57e11b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
index c66e4d1..d6298ce 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
@@ -298,24 +298,21 @@ public class LockInternals
                         try 
                         {
                             byte[] data = client.getData().usingWatcher(watcher).forPath(previousSequencePath);
-                            if ( data != null )
+                            if ( millisToWait != null )
                             {
-                                if ( millisToWait != null )
+                                millisToWait -= (System.currentTimeMillis() - startMillis);
+                                startMillis = System.currentTimeMillis();
+                                if ( millisToWait <= 0 )
                                 {
-                                    millisToWait -= (System.currentTimeMillis() - startMillis);
-                                    startMillis = System.currentTimeMillis();
-                                    if ( millisToWait <= 0 )
-                                    {
-                                        doDelete = true;    // timed out - delete our node
-                                        break;
-                                    }
-
-                                    wait(millisToWait);
-                                }
-                                else
-                                {
-                                    wait();
+                                    doDelete = true;    // timed out - delete our node
+                                    break;
                                 }
+
+                                wait(millisToWait);
+                            }
+                            else
+                            {
+                                wait();
                             }
                         }
                         catch ( KeeperException.NoNodeException e ) 


[6/8] git commit: comment udpate

Posted by ra...@apache.org.
comment udpate


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

Branch: refs/heads/master
Commit: 290186af13dca06fd15868b422583dfdae5b594e
Parents: 4e548a2
Author: randgalt <ra...@apache.org>
Authored: Fri May 23 07:47:30 2014 -0500
Committer: randgalt <ra...@apache.org>
Committed: Fri May 23 07:47:30 2014 -0500

----------------------------------------------------------------------
 .../org/apache/curator/framework/recipes/locks/LockInternals.java  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/290186af/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
index 812cfd8..706b242 100644
--- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
+++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/locks/LockInternals.java
@@ -296,7 +296,7 @@ public class LockInternals
                     {
                         try 
                         {
-                            // use getData instead of exists to avoid leaving unneeded watchers which is a type of resource leak
+                            // use getData() instead of exists() to avoid leaving unneeded watchers which is a type of resource leak
                             client.getData().usingWatcher(watcher).forPath(previousSequencePath);
                             if ( millisToWait != null )
                             {