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" <belaban(a)yahoo.com&gt; 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