Re: [jgroups-dev] Race condition with RspFilter

Yes, that's something workable.

Is everybody (especially the Infinispan team) fine with this new 
interface ? I'd like to avoid having to introduce RspFilter3 soon ... :-)




Galder Zamarreno wrote:
> Hmmm, maybe you could have a RspFilter2 interface with this and deprecate RspFilter?
>
> ----- "Bela Ban" <[email protected]> wrote:
>
>   
>> Problem is this is an API change...
>>
>> Vladimir Blagojevic wrote:
>>     
>>> Brian and I had a brief chat on IIRC regarding this issue. How about
>>>       
>> having only one method in RspFilter:
>>     
>>> RspFilerResult responseReceived(Object response, Address sender);
>>>
>>> RspFilterResult is an enum with four states where each state is
>>>       
>> essentially a combination of two boolean variables: validResponse and
>> needMoreResponses.
>>     
>>> Cheers
>>> On 2010-04-15, at 6:29 PM, Brian Stansberry wrote:
>>>
>>>   
>>>       
>>>> I think the RspFilter handling has a small race. (Writing this has
>>>>         
>> made 
>>     
>>>> me think it's very small, but since it's 1/2 written...)
>>>>
>>>> Consider this simple RspFilter implementation that signals that no
>>>>         
>>>> further responses are needed once a non-null response is received:
>>>>
>>>> public class NonNullFilter implements RspFilter
>>>> {
>>>>    private volatile boolean validResponse;
>>>>
>>>>    public boolean isAcceptable(Object response, Address sender)
>>>>    {
>>>>       if (response != null)
>>>>       {
>>>>          validResponse = true;
>>>>       }
>>>>
>>>>       return true;
>>>>    }
>>>>
>>>>    public boolean needMoreResponses()
>>>>    {
>>>>       return !(validResponse);
>>>>    }
>>>> }
>>>>
>>>> I think that captures the basic use case of RspFilter.
>>>>
>>>> The handling of response is like this in
>>>>         
>> GroupRequest.responseReceived():
>>     
>>>>         boolean responseReceived=false;
>>>>         if(!rsp.wasReceived()) {
>>>>             if((responseReceived=(rsp_filter == null) || 
>>>> rsp_filter.isAcceptable(response_value, sender)))
>>>>                 rsp.setValue(response_value);
>>>>             rsp.setReceived(responseReceived);
>>>>         }
>>>>
>>>>         lock.lock();
>>>>         try {
>>>>             if(responseReceived)
>>>>                 num_received++;
>>>>             done=rsp_filter == null? responsesComplete() : 
>>>> !rsp_filter.needMoreResponses();
>>>>             if(responseReceived || done)
>>>>                 completed.signalAll(); // wakes up execute()
>>>>
>>>> Now imagine 2 responses arriving nearly concurrently, with thread
>>>>         
>> T1 
>>     
>>>> carrying a response value of null, and T2 carrying a non-null
>>>>         
>> response.
>>     
>>>> 1) T1 calls isAcceptable(), validResponse == false
>>>> 2) T1 calls rsp.setValue()
>>>> 3) T2 calls isAcceptable(), validResponse == true
>>>> 4) T1 calls needMoreResponses, gets "false" since validResponse ==
>>>>         
>> true
>>     
>>>> 5) T1 calls completed.signalAll(), wakes up caller
>>>> 6) Caller thread wakes up
>>>> 7) Caller thread processes response list, doesn't see the T2 value
>>>>         
>> as 
>>     
>>>> it's not set yet
>>>> 8) T2 calls rsp.setValue().
>>>>
>>>> Now that's pretty improbable, i.e. that steps 4,5,6,7 all execute 
>>>> between 3 and 8. But it's a race.
>>>>
>>>> A semi-related thing is that NonNullFilter.isAcceptable() always
>>>>         
>> returns 
>>     
>>>> true. Seems counter-intuitive, why not return "false" if response
>>>>         
>> == 
>>     
>>>> null?  Reason is that num_received is only incremented if
>>>>         
>> isAcceptable() 
>>     
>>>> returns true. Effect is that if isAcceptable() doesn't always
>>>>         
>> return 
>>     
>>>> true, if only "null" responses are received 
>>>> NonNullFilter.needMoreResponses() will never return false, *and* 
>>>> GroupRequest.responsesComplete() will never return true!  The
>>>>         
>> caller 
>>     
>>>> thread will just wait until timeout
>>>>
>>>> I wonder if in 3.0 a simpler RspFilter API would just be a single
>>>>         
>> method:
>>     
>>>> boolean needMoreResponses((Object response, Address sender)
>>>>
>>>> That would make it easier to avoid the race. Is there a use case
>>>>         
>> for not 
>>     
>>>> marking a Rsp as received? That's what the separate 
>>>> isResponseAcceptable() method allows.
>>>>
>>>> Apologies if this has been discussed before; I have a vague feeling
>>>>         
>> it 
>>     
>>>> has come up.
>>>>
>>>> -- 
>>>> Brian Stansberry
>>>> Lead, AS Clustering
>>>> JBoss by Red Hat
>>>>
>>>>
>>>>         
>> ------------------------------------------------------------------------------
>>     
>>>> Download Intel&#174; Parallel Studio Eval
>>>> Try the new software tools for yourself. Speed compiling, find
>>>>         
>> bugs
>>     
>>>> proactively, and fine-tune applications for parallel performance.
>>>> See why Intel Parallel Studio got high marks during beta.
>>>> http://p.sf.net/sfu/intel-sw-dev
>>>> _______________________________________________
>>>> Javagroups-development mailing list
>>>>
>>>>     
>>>>         
>>>
>>>       
>> ------------------------------------------------------------------------------
>>     
>>> Download Intel&#174; Parallel Studio Eval
>>> Try the new software tools for yourself. Speed compiling, find bugs
>>> proactively, and fine-tune applications for parallel performance.
>>> See why Intel Parallel Studio got high marks during beta.
>>> http://p.sf.net/sfu/intel-sw-dev
>>> _______________________________________________
>>> Javagroups-development mailing list
>>>
>>>
>>>   
>>>       
>> -- 
>> Bela Ban
>> Lead JGroups / JBoss Clustering team
>> JBoss - a division of Red Hat
>>
>>
>> ------------------------------------------------------------------------------
>> Download Intel&#174; Parallel Studio Eval
>> Try the new software tools for yourself. Speed compiling, find bugs
>> proactively, and fine-tune applications for parallel performance.
>> See why Intel Parallel Studio got high marks during beta.
>> http://p.sf.net/sfu/intel-sw-dev
>> _______________________________________________
>> Javagroups-development mailing list
>>     
>
>   

-- 
Bela Ban
Lead JGroups / Clustering Team
JBoss


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Javagroups-development mailing list