[jira] [Commented] (CASSANDRA-5691) Remove SimpleCondition

2013-06-24 Thread Jonathan Ellis (JIRA)

[ 
https://issues.apache.org/jira/browse/CASSANDRA-5691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13692770#comment-13692770
 ] 

Jonathan Ellis commented on CASSANDRA-5691:
---

bq. If the patch is not applied then at least singnal() invocations should be 
replaced with signalAll() and signal() should throw 
UnsupportedOperationException.

Agreed.

> Remove SimpleCondition
> --
>
> Key: CASSANDRA-5691
> URL: https://issues.apache.org/jira/browse/CASSANDRA-5691
> Project: Cassandra
>  Issue Type: Bug
>  Components: Core
>Reporter: Mikhail Mazursky
>Priority: Minor
> Attachments: trunk-5691.patch
>
>
> Problematic scenario:
> 1. two threads get blocked in SimpleCondition.await();
> 2. some thread calls SimpleCondition.signal();
> 3. one of blocked threads wakes up and runs;
> 4. spurious wakeup happens in the second thread and it wakes up too and runs 
> even though nobody signaled it.
> Thus this is a broken implementation of Condition interface.
> Anyway, looking at how code uses it, SimpleCondition can just be replaced 
> with CountDownLatch.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira


[jira] [Commented] (CASSANDRA-5691) Remove SimpleCondition

2013-06-24 Thread Mikhail Mazursky (JIRA)

[ 
https://issues.apache.org/jira/browse/CASSANDRA-5691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13692707#comment-13692707
 ] 

Mikhail Mazursky commented on CASSANDRA-5691:
-

I see following reasons to make this change:
1. It removes code that have a bug;
2. It removes code that is not tested;
3. Why invent a wheel? CountDownLatch(1) is a common way to do this kind of 
signaling and is widely used and known. So it's easier to understand code.

If the patch is not applied then at least singnal() invocations should be 
replaced with signalAll() and signal() should throw 
UnsupportedOperationException.

p.s. I don't think CDL offers any better or worse performance.

> Remove SimpleCondition
> --
>
> Key: CASSANDRA-5691
> URL: https://issues.apache.org/jira/browse/CASSANDRA-5691
> Project: Cassandra
>  Issue Type: Bug
>  Components: Core
>Reporter: Mikhail Mazursky
>Priority: Minor
> Attachments: trunk-5691.patch
>
>
> Problematic scenario:
> 1. two threads get blocked in SimpleCondition.await();
> 2. some thread calls SimpleCondition.signal();
> 3. one of blocked threads wakes up and runs;
> 4. spurious wakeup happens in the second thread and it wakes up too and runs 
> even though nobody signaled it.
> Thus this is a broken implementation of Condition interface.
> Anyway, looking at how code uses it, SimpleCondition can just be replaced 
> with CountDownLatch.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira


[jira] [Commented] (CASSANDRA-5691) Remove SimpleCondition

2013-06-24 Thread Jonathan Ellis (JIRA)

[ 
https://issues.apache.org/jira/browse/CASSANDRA-5691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13691962#comment-13691962
 ] 

Jonathan Ellis commented on CASSANDRA-5691:
---

Yes, it's pretty obvious that signal==signalAll in SimpleCondition.  (Signaling 
just a single waiter is fairly useless and not something we care about.)

Unless CDL offers a performance benefit I don't see any point in churning this.

> Remove SimpleCondition
> --
>
> Key: CASSANDRA-5691
> URL: https://issues.apache.org/jira/browse/CASSANDRA-5691
> Project: Cassandra
>  Issue Type: Bug
>  Components: Core
>Reporter: Mikhail Mazursky
>Priority: Minor
> Attachments: trunk-5691.patch
>
>
> Problematic scenario:
> 1. two threads get blocked in SimpleCondition.await();
> 2. some thread calls SimpleCondition.signal();
> 3. one of blocked threads wakes up and runs;
> 4. spurious wakeup happens in the second thread and it wakes up too and runs 
> even though nobody signaled it.
> Thus this is a broken implementation of Condition interface.
> Anyway, looking at how code uses it, SimpleCondition can just be replaced 
> with CountDownLatch.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira