[infinispan-dev] Threadpools in a large cluster

[Bela Ban]

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)