Github user javicacheiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/413#discussion_r149135888
--- Diff:
src/recipes/lock/test/org/apache/zookeeper/recipes/lock/ZNodeNameTest.java ---
@@ -37,17 +37,30 @@ public void testOrderWithSamePrefix() throws Exception {
@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" };
+ // names with duplicated sequence numbers are not included in the
result
+ String[] expected = { "r-1", "r-2", "r-3" };
--- End diff --
I agree it is a confusing behaviour of the current implementation. The
reason is that we are using a TreeSet to do the sorting so when a new ZNodeName
is added and the compareTo method returns 0 with any of the existing Set
members, then it is considered a duplicate and it is not added. So in this case
w-2 will be considered a duplicate of r-2 because they share the same sequence
number. In the WriteLock context trying to sort znodes with duplicated sequence
numbers will probably indicate some problem.
To adhere to the principle of least surprise I have changed the
implementation of the compareTo so it produces the expected result.
---
