You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by "Jaakko Laine (JIRA)" <ji...@apache.org> on 2010/01/15 06:20:54 UTC
[jira] Created: (ZOOKEEPER-645) Bug in WriteLock recipe
implementation?
Bug in WriteLock recipe implementation?
---------------------------------------
Key: ZOOKEEPER-645
URL: https://issues.apache.org/jira/browse/ZOOKEEPER-645
Project: Zookeeper
Issue Type: Bug
Affects Versions: 3.2.2
Environment: 3.2.2 java 1.6.0_12
Reporter: Jaakko Laine
Priority: Minor
Not sure, but there seem to be two issues in the example WriteLock:
(1) ZNodeName is sorted according to session ID first, and then according to znode sequence number. This might cause starvation as lower session IDs always get priority. WriteLock is not thread-safe in the first place, so having session ID involved in compare operation does not seem to make sense.
(2) if findPrefixInChildren finds previous ID, it should add dir in front of the ID
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-645) Bug in WriteLock recipe
implementation?
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-645?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12800888#action_12800888 ]
Hadoop QA commented on ZOOKEEPER-645:
-------------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12430357/645-fix-findPrefixInChildren.patch
against trunk revision 899383.
+1 @author. The patch does not contain any @author tags.
-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no tests are needed for this patch.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/103/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/103/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/103/console
This message is automatically generated.
> Bug in WriteLock recipe implementation?
> ---------------------------------------
>
> Key: ZOOKEEPER-645
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-645
> Project: Zookeeper
> Issue Type: Bug
> Components: recipes
> Affects Versions: 3.2.2
> Environment: 3.2.2 java 1.6.0_12
> Reporter: Jaakko Laine
> Assignee: Jaakko Laine
> Priority: Minor
> Fix For: 3.3.0
>
> Attachments: 645-fix-findPrefixInChildren.patch
>
>
> Not sure, but there seem to be two issues in the example WriteLock:
> (1) ZNodeName is sorted according to session ID first, and then according to znode sequence number. This might cause starvation as lower session IDs always get priority. WriteLock is not thread-safe in the first place, so having session ID involved in compare operation does not seem to make sense.
> (2) if findPrefixInChildren finds previous ID, it should add dir in front of the ID
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-645) Bug in WriteLock recipe
implementation?
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-645?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mahadev konar updated ZOOKEEPER-645:
------------------------------------
Fix Version/s: (was: 3.3.0)
3.4.0
not a blocker. moving it to 3.4.
> Bug in WriteLock recipe implementation?
> ---------------------------------------
>
> Key: ZOOKEEPER-645
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-645
> Project: Zookeeper
> Issue Type: Bug
> Components: recipes
> Affects Versions: 3.2.2
> Environment: 3.2.2 java 1.6.0_12
> Reporter: Jaakko Laine
> Assignee: Mahadev konar
> Priority: Minor
> Fix For: 3.4.0
>
> Attachments: 645-fix-findPrefixInChildren.patch
>
>
> Not sure, but there seem to be two issues in the example WriteLock:
> (1) ZNodeName is sorted according to session ID first, and then according to znode sequence number. This might cause starvation as lower session IDs always get priority. WriteLock is not thread-safe in the first place, so having session ID involved in compare operation does not seem to make sense.
> (2) if findPrefixInChildren finds previous ID, it should add dir in front of the ID
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-645) Bug in WriteLock recipe
implementation?
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-645?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Hunt updated ZOOKEEPER-645:
-----------------------------------
Component/s: recipes
Fix Version/s: 3.3.0
Assignee: Jaakko Laine
Mahadev can you review this one?
> Bug in WriteLock recipe implementation?
> ---------------------------------------
>
> Key: ZOOKEEPER-645
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-645
> Project: Zookeeper
> Issue Type: Bug
> Components: recipes
> Affects Versions: 3.2.2
> Environment: 3.2.2 java 1.6.0_12
> Reporter: Jaakko Laine
> Assignee: Jaakko Laine
> Priority: Minor
> Fix For: 3.3.0
>
> Attachments: 645-fix-findPrefixInChildren.patch
>
>
> Not sure, but there seem to be two issues in the example WriteLock:
> (1) ZNodeName is sorted according to session ID first, and then according to znode sequence number. This might cause starvation as lower session IDs always get priority. WriteLock is not thread-safe in the first place, so having session ID involved in compare operation does not seem to make sense.
> (2) if findPrefixInChildren finds previous ID, it should add dir in front of the ID
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-645) Bug in WriteLock recipe
implementation?
Posted by "Jaakko Laine (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-645?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jaakko Laine updated ZOOKEEPER-645:
-----------------------------------
Attachment: 645-fix-findPrefixInChildren.patch
Attached patch addresses issue 2 above.
> Bug in WriteLock recipe implementation?
> ---------------------------------------
>
> Key: ZOOKEEPER-645
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-645
> Project: Zookeeper
> Issue Type: Bug
> Affects Versions: 3.2.2
> Environment: 3.2.2 java 1.6.0_12
> Reporter: Jaakko Laine
> Priority: Minor
> Attachments: 645-fix-findPrefixInChildren.patch
>
>
> Not sure, but there seem to be two issues in the example WriteLock:
> (1) ZNodeName is sorted according to session ID first, and then according to znode sequence number. This might cause starvation as lower session IDs always get priority. WriteLock is not thread-safe in the first place, so having session ID involved in compare operation does not seem to make sense.
> (2) if findPrefixInChildren finds previous ID, it should add dir in front of the ID
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (ZOOKEEPER-645) Bug in WriteLock recipe
implementation?
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-645?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mahadev konar reassigned ZOOKEEPER-645:
---------------------------------------
Assignee: Mahadev konar (was: Jaakko Laine)
> Bug in WriteLock recipe implementation?
> ---------------------------------------
>
> Key: ZOOKEEPER-645
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-645
> Project: Zookeeper
> Issue Type: Bug
> Components: recipes
> Affects Versions: 3.2.2
> Environment: 3.2.2 java 1.6.0_12
> Reporter: Jaakko Laine
> Assignee: Mahadev konar
> Priority: Minor
> Fix For: 3.3.0
>
> Attachments: 645-fix-findPrefixInChildren.patch
>
>
> Not sure, but there seem to be two issues in the example WriteLock:
> (1) ZNodeName is sorted according to session ID first, and then according to znode sequence number. This might cause starvation as lower session IDs always get priority. WriteLock is not thread-safe in the first place, so having session ID involved in compare operation does not seem to make sense.
> (2) if findPrefixInChildren finds previous ID, it should add dir in front of the ID
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-645) Bug in WriteLock recipe
implementation?
Posted by "Robert Crocombe (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-645?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12805736#action_12805736 ]
Robert Crocombe commented on ZOOKEEPER-645:
-------------------------------------------
I can address point (1) in the original comment: having the session ID breaks the lock recipe. It looks like the session ID was added per:
https://issues.apache.org/jira/browse/ZOOKEEPER-78?focusedCommentId=12615305&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_12615305
but the sorting via ZNodeName is worse than starvation: it causes clients with low session IDs not to see that clients with higher session IDs have acquired the lock which causes the function to return true: so you could theoretically have as many lock owners as clients as long as they acquire the lock in descending session ID order.
I fixed this in a local branch by replacing ZNodeName with a different class that ignores the session ID when sorting. My test for WriteLockTest became:
{noformat}
/*
* Test that the bug which makes it possible to acquire a lock in two
* different sessions simultaneously is fixed. Bug occurs because including
* session ID in node name results in sorting in
* LockZooKeeperOperation.execute() such that low session ID clients do not
* see that higher session ID clients already have the lock.
*/
@Test
public void testSessionOrderingBugFix() throws Exception {
// session IDs are presumably assigned in order, but perform some checks
final ZooKeeper zooA = createClient();
final ZooKeeper zooB = createClient();
final ZooKeeper lowZoo = zooA.getSessionId() < zooB.getSessionId() ? zooA
: zooB;
final ZooKeeper highZoo = zooA.getSessionId() < zooB.getSessionId() ? zooB
: zooA;
final WriteLock highLock = new WriteLock(highZoo, dir, null);
final WriteLock lowLock = new WriteLock(lowZoo, dir, null);
boolean highLocked = highLock.lock();
assertTrue(highLocked);
// If the bug is present, this attempt to lock will succeed.
boolean lowLocked = lowLock.lock();
assertFalse(lowLocked);
assertTrue(highLock.isOwner());
assertFalse(lowLock.isOwner());
}
{noformat}
> Bug in WriteLock recipe implementation?
> ---------------------------------------
>
> Key: ZOOKEEPER-645
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-645
> Project: Zookeeper
> Issue Type: Bug
> Components: recipes
> Affects Versions: 3.2.2
> Environment: 3.2.2 java 1.6.0_12
> Reporter: Jaakko Laine
> Assignee: Jaakko Laine
> Priority: Minor
> Fix For: 3.3.0
>
> Attachments: 645-fix-findPrefixInChildren.patch
>
>
> Not sure, but there seem to be two issues in the example WriteLock:
> (1) ZNodeName is sorted according to session ID first, and then according to znode sequence number. This might cause starvation as lower session IDs always get priority. WriteLock is not thread-safe in the first place, so having session ID involved in compare operation does not seem to make sense.
> (2) if findPrefixInChildren finds previous ID, it should add dir in front of the ID
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-645) Bug in WriteLock recipe
implementation?
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-645?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12800862#action_12800862 ]
Mahadev konar commented on ZOOKEEPER-645:
-----------------------------------------
sure am doing it right now.
> Bug in WriteLock recipe implementation?
> ---------------------------------------
>
> Key: ZOOKEEPER-645
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-645
> Project: Zookeeper
> Issue Type: Bug
> Components: recipes
> Affects Versions: 3.2.2
> Environment: 3.2.2 java 1.6.0_12
> Reporter: Jaakko Laine
> Assignee: Jaakko Laine
> Priority: Minor
> Fix For: 3.3.0
>
> Attachments: 645-fix-findPrefixInChildren.patch
>
>
> Not sure, but there seem to be two issues in the example WriteLock:
> (1) ZNodeName is sorted according to session ID first, and then according to znode sequence number. This might cause starvation as lower session IDs always get priority. WriteLock is not thread-safe in the first place, so having session ID involved in compare operation does not seem to make sense.
> (2) if findPrefixInChildren finds previous ID, it should add dir in front of the ID
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-645) Bug in WriteLock recipe
implementation?
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-645?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Hunt updated ZOOKEEPER-645:
-----------------------------------
Status: Open (was: Patch Available)
We should add tests to this to verify the change.
> Bug in WriteLock recipe implementation?
> ---------------------------------------
>
> Key: ZOOKEEPER-645
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-645
> Project: Zookeeper
> Issue Type: Bug
> Components: recipes
> Affects Versions: 3.2.2
> Environment: 3.2.2 java 1.6.0_12
> Reporter: Jaakko Laine
> Assignee: Jaakko Laine
> Priority: Minor
> Fix For: 3.3.0
>
> Attachments: 645-fix-findPrefixInChildren.patch
>
>
> Not sure, but there seem to be two issues in the example WriteLock:
> (1) ZNodeName is sorted according to session ID first, and then according to znode sequence number. This might cause starvation as lower session IDs always get priority. WriteLock is not thread-safe in the first place, so having session ID involved in compare operation does not seem to make sense.
> (2) if findPrefixInChildren finds previous ID, it should add dir in front of the ID
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-645) Bug in WriteLock recipe
implementation?
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-645?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mahadev konar updated ZOOKEEPER-645:
------------------------------------
Status: Patch Available (was: Open)
> Bug in WriteLock recipe implementation?
> ---------------------------------------
>
> Key: ZOOKEEPER-645
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-645
> Project: Zookeeper
> Issue Type: Bug
> Components: recipes
> Affects Versions: 3.2.2
> Environment: 3.2.2 java 1.6.0_12
> Reporter: Jaakko Laine
> Assignee: Jaakko Laine
> Priority: Minor
> Fix For: 3.3.0
>
> Attachments: 645-fix-findPrefixInChildren.patch
>
>
> Not sure, but there seem to be two issues in the example WriteLock:
> (1) ZNodeName is sorted according to session ID first, and then according to znode sequence number. This might cause starvation as lower session IDs always get priority. WriteLock is not thread-safe in the first place, so having session ID involved in compare operation does not seem to make sense.
> (2) if findPrefixInChildren finds previous ID, it should add dir in front of the ID
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-645) Bug in WriteLock recipe
implementation?
Posted by "Tim Robertson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-645?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12907331#action_12907331 ]
Tim Robertson commented on ZOOKEEPER-645:
-----------------------------------------
I have applied the patch but still see that locks generated by subsequent clients are not respected by the first client and it is always able to get the lock
> Bug in WriteLock recipe implementation?
> ---------------------------------------
>
> Key: ZOOKEEPER-645
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-645
> Project: Zookeeper
> Issue Type: Bug
> Components: recipes
> Affects Versions: 3.2.2
> Environment: 3.2.2 java 1.6.0_12
> Reporter: Jaakko Laine
> Assignee: Mahadev konar
> Priority: Minor
> Fix For: 3.4.0
>
> Attachments: 645-fix-findPrefixInChildren.patch
>
>
> Not sure, but there seem to be two issues in the example WriteLock:
> (1) ZNodeName is sorted according to session ID first, and then according to znode sequence number. This might cause starvation as lower session IDs always get priority. WriteLock is not thread-safe in the first place, so having session ID involved in compare operation does not seem to make sense.
> (2) if findPrefixInChildren finds previous ID, it should add dir in front of the ID
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.