zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ph...@apache.org
Subject zookeeper git commit: ZOOKEEPER-2931: WriteLock recipe: Fix bug in znode ordering when the sessionId is part of the znode name
Date Wed, 15 Nov 2017 23:35:40 GMT
Repository: zookeeper
Updated Branches:
  refs/heads/branch-3.4 8be14c4d4 -> bb4d9cc40


ZOOKEEPER-2931: WriteLock recipe: Fix bug in znode ordering when the sessionId is part of
the znode name

This PR solves a bug in the WriteLock recipe related to the ordering the znodes.

In the original version when the nodes are sorted in WriteLock.java using a TreeSet the whole
znode path is taken into account and not just the sequence number.

This causes an issue when the sessionId is included in the znode path because a znode with
a lower sessionId will appear as lower than other znode with a higher sessionId even if its
sequence number is bigger. In specific situations this ended with two clients holding the
lock at the same time.

Author: Javier Cacheiro <alcachi@gmail.com>

Reviewers: phunt@apache.org

Closes #413 from javicacheiro/fix_writelock and squashes the following commits:

ed0e6648 [Javier Cacheiro] Secondary sort znodes with the same sequence number by prefix
b382648d [Javier Cacheiro] Add unit tests
f9c581d4 [Javier Cacheiro] Fix bug in znode ordering when the sessionId is part of the znode
name

Change-Id: Id384ce6079bbdfef10054fabb8bb93378d86024f
(cherry picked from commit f299303add79250ec2181f6c03b15e3754825284)
Signed-off-by: Patrick Hunt <phunt@apache.org>


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

Branch: refs/heads/branch-3.4
Commit: bb4d9cc405ff06191cdbcaf0265175f9da21ba80
Parents: 8be14c4
Author: Javier Cacheiro <alcachi@gmail.com>
Authored: Wed Nov 15 15:21:33 2017 -0800
Committer: Patrick Hunt <phunt@apache.org>
Committed: Wed Nov 15 15:34:58 2017 -0800

----------------------------------------------------------------------
 .../apache/zookeeper/recipes/lock/ZNodeName.java    | 16 +++++++++-------
 .../zookeeper/recipes/lock/ZNodeNameTest.java       | 16 ++++++++++++++--
 2 files changed, 23 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zookeeper/blob/bb4d9cc4/src/recipes/lock/src/java/org/apache/zookeeper/recipes/lock/ZNodeName.java
----------------------------------------------------------------------
diff --git a/src/recipes/lock/src/java/org/apache/zookeeper/recipes/lock/ZNodeName.java b/src/recipes/lock/src/java/org/apache/zookeeper/recipes/lock/ZNodeName.java
index 99b6616..2e32e59 100644
--- a/src/recipes/lock/src/java/org/apache/zookeeper/recipes/lock/ZNodeName.java
+++ b/src/recipes/lock/src/java/org/apache/zookeeper/recipes/lock/ZNodeName.java
@@ -74,15 +74,17 @@ class ZNodeName implements Comparable<ZNodeName> {
         return name.hashCode() + 37;
     }
 
+    /**
+     * Compare znodes based on their sequence number
+     * @param that other znode to compare to
+     * @return the difference between their sequence numbers: a positive value if this
+     *         znode has a larger sequence number, 0 if they have the same sequence number
+     *         or a negative number if this znode has a lower sequence number
+     */
     public int compareTo(ZNodeName that) {
-        int answer = this.prefix.compareTo(that.prefix);
+        int answer = this.sequence - that.sequence;
         if (answer == 0) {
-            int s1 = this.sequence;
-            int s2 = that.sequence;
-            if (s1 == -1 && s2 == -1) {
-                return this.name.compareTo(that.name);
-            }
-            answer = s1 == -1 ? 1 : s2 == -1 ? -1 : s1 - s2;
+            return this.prefix.compareTo(that.prefix);
         }
         return answer;
     }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/bb4d9cc4/src/recipes/lock/test/org/apache/zookeeper/recipes/lock/ZNodeNameTest.java
----------------------------------------------------------------------
diff --git a/src/recipes/lock/test/org/apache/zookeeper/recipes/lock/ZNodeNameTest.java b/src/recipes/lock/test/org/apache/zookeeper/recipes/lock/ZNodeNameTest.java
index 6c3bdac..773c2ec 100644
--- a/src/recipes/lock/test/org/apache/zookeeper/recipes/lock/ZNodeNameTest.java
+++ b/src/recipes/lock/test/org/apache/zookeeper/recipes/lock/ZNodeNameTest.java
@@ -37,17 +37,29 @@ public class ZNodeNameTest extends TestCase {
     @Test
     public void testOrderWithDifferentPrefixes() throws Exception {
         String[] names = { "r-3", "r-2", "r-1", "w-2", "w-1" };
-        String[] expected = { "r-1", "r-2", "r-3", "w-1", "w-2" };
+        String[] expected = { "r-1", "w-1", "r-2", "w-2", "r-3" };
+        assertOrderedNodeNames(names, expected);
+    }
+    @Test
+    public void testOrderWithDifferentPrefixIncludingSessionId() throws Exception {
+        String[] names = { "x-242681582799028564-0000000002", "x-170623981976748329-0000000003",
"x-98566387950223723-0000000001" };
+        String[] expected = { "x-98566387950223723-0000000001", "x-242681582799028564-0000000002",
"x-170623981976748329-0000000003" };
+        assertOrderedNodeNames(names, expected);
+    }
+    @Test
+    public void testOrderWithExtraPrefixes() throws Exception {
+        String[] names = { "r-1-3-2", "r-2-2-1", "r-3-1-3" };
+        String[] expected = { "r-2-2-1", "r-1-3-2", "r-3-1-3" };
         assertOrderedNodeNames(names, expected);
     }
 
     protected void assertOrderedNodeNames(String[] names, String[] expected) {
         int size = names.length;
-        assertEquals("The two arrays should be the same size!", names.length, expected.length);
         SortedSet<ZNodeName> nodeNames = new TreeSet<ZNodeName>();
         for (String name : names) {
             nodeNames.add(new ZNodeName(name));
         }
+        assertEquals("The SortedSet does not have the expected size!", nodeNames.size(),
expected.length);
 
         int index = 0;
         for (ZNodeName nodeName : nodeNames) {


Mime
View raw message