[infinispan-dev] ISPN-2808 - thread pool for incoming message [feedback]

[Pedro Ruivo]

On 02/28/2013 12:18 PM, Mircea Markus wrote:
>
> On 27 Feb 2013, at 19:06, Pedro Ruivo wrote:
>
>> Hi all,
>>
>> I'm working on ISPN-2808 and I want some feedback about it (code is
>> here [1])
>>
>> I'm starting to implement this feature but I know that Asynchronous
>> Invocation API is not totally finished in JGroups.
>>
>> My idea in to use an executor service in CommandAwareRpcDispatcher
>> (CARD) and when a request (command) is received, it checks if it is
>> useful to move the command execution to another thread (in this line [2])
>>
>> For now, I'm thinking to move all the write commands, lock control
>> command, prepare command and commit command to the executor service
>> (Note: commit command is only moved when in DIST mode and L1 is enabled).
>
> you might want to move Commit there when we have a tx cache and cache
> store - it's during the commit where the data is written to the cache
> store and that might take time.
>
>> first question: do you think it is fine to move the commands to the
>> executor service in CARD or should I move this functionally to the
>> InvoundHandler?
> +1 for the InboundInvocationHandler: with ISPN-2849 we'll build the tx
> dependency right before invoking the interceptor chain (potentially in a
> new interceptor), so i think the closer you move it to the interceptor
> chain the better.
So do you think that is better to create a new interceptor to dispatch 
the commands to the thread pool? (at least for the VisitableCommands). 
And put this new interceptor after the InvocationContextInterceptor?

My opinion, it to dispatch the command to a new thread before invoking 
command.perform() in order to avoid to move some ThreadLocal variable, 
set by the perform() method.
>
>> second question: do you have in mind other commands may block the
>> OOB/Regular thread and should be moved to a thread in the executor
>> service?
>
> Generally all the commands that are long-processing(lock acquisition or
> interaction with a cache store) would be better executed in this pool in
> order to avoid the OOB/regular thread pool to deadlock.
> Looking at the command hierarchy for long processing commands:
> - StateResponseCommand  seems to be a good candidate as it might acquire
> locks
> -IndexUpdateCommand/ClusterQueryCommand - I'll let Sanne comment on
> these two, which might require an update on query module as well
> - MapCombineCommand if a cache loader is present (it iterates over the
> entries in the loader)
> Dan/Adrian care to comment on the CacheTopologyControlCommand?
>
> Cheers,
> --
> Mircea Markus
> Infinispan lead (www.infinispan.org <http://www.infinispan.org>)
>
>
>
>