[jira] Commented: (ZOOKEEPER-645) Bug in WriteLock recipe implementation?
[
https://issues.apache.org/jira/browse/ZOOKEEPER-645?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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=12615305page=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?
[ https://issues.apache.org/jira/browse/ZOOKEEPER-645?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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] Commented: (ZOOKEEPER-645) Bug in WriteLock recipe implementation?
[ https://issues.apache.org/jira/browse/ZOOKEEPER-645?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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.
