On 2/7/13 2:55 PM, Pedro Ruivo wrote: > >> The default would be to invoke the old handle(Message) method. The >> dispatching mechanism could be changed to use the async method by >> setting an attribute in MessageDispatcher (which in turn sets it in >> RequestCorrelator). >> >> How would you do this ? Remember, we cannot change or remove >> handle(Message) as subclasses of RpcDispatcher or MessageDispatcher, or >> impls of RequestHandler are out there and any change to handle(Message) >> would break them. >> >> Would you simply provide a separate AsyncRequestHandler interface, not >> extending RequestHandler ? This would require RequestCorrelator and >> MessageDispatcher to have 2 refs instead of 1. With the current approach >> I can do an instanceof on the RequestHandler. >> >> I eventually like to merge RequestHandler and AsyncRequestHandler into 1 >> class, but this can be done in 4.0 at the earliest time. >> > My opinion, I would use a separate a interface and 2 references (while > RequestHandler is still active).
That's not good because that would be an API change and would break code in MuxRpcDispatcher/MuxRequestCorrelator. I've pushed my branch JGRP-1587 to the repo, so you can look at the code. This isn't final yet of course. > However, I will change the > AsyncRequestHandler interface to: > > Object handle(Message message, Response response) throws Exception OK, that's what it is currently > > and the Response interface would be as follow: > > void sendResponse(Object reply, boolean isException) //or sendReply(...); Yes > > void setAsyncResponse(boolean value); //or setAsyncReply(...); > > boolean isAsyncResponse(); //or isAsyncReply(); I don't like this. Intuitively, we create a response or not, depending on information in the request. Also, 2 more method to implement. > > And with this, it can supports both behaviors, with a minor addition: > > 1) To work in async mode: > the application invokes setAsyncResponse(true) and eventually it > will invoke the sendResponse(...) > > 2) To work in sync mode: > the application invokes setAsyncResponse(false). I think this > should be the default value > > in the RequestCorrelator, it checks if isAsyncReponse() value. If true, > it does not send the response (it only sends it if it is an exception > caught). If the value is false, it sends the return value as a response > (the current behavior). In pseudo code: > > try { > ret = asyncRequestHandler.handle(message, response); > if (response expected and !response.isAsynResponse()) > reponse.sendResponse(ret, false); > catch Throwable t > reponse.sendResponse(t, true) > > With this suggestion, when you remove the RequestHandler interface (in > 4.0 or 5.0), the user only needs to add the Response parameter and > everything will work as usual. > > For Infinispan 5.3, it can be adopted as soon as it is implemented in > JGroups without using the async method. > > To avoid the problem of sending twice a response, I would put an > AtomicBoolean or a similar synchronization mechanism in the Response > implementation. > > opinions? > > //more below >>>> - AsyncRequestHandler will look similar to the following: >>>> void handle(Message request, Handback hb, boolean requires_response) >>>> throws Throwable; >>>> - Handback is an interface, and its impl contains header information >>>> (e.g. request ID) >>>> - Handback has a sendReply(Object reply, boolean is_exception) method >>>> which sends a response (or exception) back to the caller >>>> - When requires_response is false, the AsyncRequestHandler doesn't need >>>> to invoke sendReply() >>> My 2 cents, I think that the boolean requires_response should be in >>> Handback implementation to avoid passing an extra argument >> I actually removed the requires_response parameter. If no response is >> required, rsp in handle(Message req, Response rsp) will simply be null. > I would avoid put the rsp in null and I would do an empty implementation > of the Response interface, in which the methods do nothing. Can't do that as I want to do stuff like: void handle(Message req, response rsp) { try { // invoke the request } catch(Throwable t) { if(rsp != null) rsp.send(t, true); // send the exception back to the caller else logError(); } } -- Bela Ban, JGroups lead (http://www.jgroups.org) _______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev