Re: [jgroups-dev] [infinispan-dev] Race condition with RspFilter

How does this impact existing usage of a RspFilter?  Handling this in 1 method is sensible to me and would cover all of our use cases... 

On 20 Apr 2010, at 08:24, [email protected] wrote:

> What values does RspFilerResult contain exactly?
> 
> ----- "Bela Ban" <[email protected]> wrote:
> 
>> 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
> _______________________________________________
> infinispan-dev mailing list
> [email protected]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Manik Surtani
[email protected]
Lead, Infinispan
Lead, JBoss Cache
http://www.infinispan.org
http://www.jbosscache.org





------------------------------------------------------------------------------
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