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

2013-02-28 Thread Bela Ban

On 2/27/13 8:10 PM, Pedro Ruivo wrote:


  Original Message 
 Subject: ISPN-2808 - thread pool for incoming message [feedback]
 Date: Wed, 27 Feb 2013 19:06:49 +
 From: Pedro Ruivo pe...@infinispan.org
 To: infinispan-core-...@infinispan.org infinispan-core-...@infinispan.org

 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.

It is; I'll upload the 3.3.0.Beta1 version to Nexus in ca. 20 minutes...



 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


Can't a COMMIT or ROLLBACK be invoked directly ? Aren't they non-blocking ?

 (Note: commit command is only moved when in DIST mode and L1 is enabled).

 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?


I thought we were going to implement this as *first* interceptor in the 
chain, e.g. to allow thread locals to still work ? By the time a command 
reaches the CommandAwareRpcDispatcher, we're already half-way through 
the chain.


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


[infinispan-dev] JGroups 3.3.0.Beta1

2013-02-28 Thread Bela Ban
FYI,

added to Nexus.

This completes the implementation of message batching in all protocols, 
and contains the complete Async Invocation API as well. Once we have an 
implementation of async request handling in Infinispan, which runs 
independent transactions in separate threads from an internal thread 
pool, I expect a significant performance increase !

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


Re: [infinispan-dev] JGroups 3.3.0.Beta1

2013-02-28 Thread Sanne Grinovero
Very nice, thanks Bela!

I'm wondering if I should upgrade the next crop of Search technology
to use it. In other words, do you expect the 3.3 line to be included
in EAP 6.1 ?
Otherwise I'm stuck on older version.

Sanne

On 28 February 2013 11:58, Bela Ban b...@redhat.com wrote:
 FYI,

 added to Nexus.

 This completes the implementation of message batching in all protocols,
 and contains the complete Async Invocation API as well. Once we have an
 implementation of async request handling in Infinispan, which runs
 independent transactions in separate threads from an internal thread
 pool, I expect a significant performance increase !

 --
 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
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev


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

2013-02-28 Thread Mircea Markus

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. 

 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)




___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

Re: [infinispan-dev] JGroups 3.3.0.Beta1

2013-02-28 Thread Bela Ban
Thx,
I'm afraid I don't know which components are going to be included in EAP 
6.1; ask the EAP folks.

On 2/28/13 1:14 PM, Sanne Grinovero wrote:
 Very nice, thanks Bela!

 I'm wondering if I should upgrade the next crop of Search technology
 to use it. In other words, do you expect the 3.3 line to be included
 in EAP 6.1 ?
 Otherwise I'm stuck on older version.

 Sanne

 On 28 February 2013 11:58, Bela Ban b...@redhat.com wrote:
 FYI,

 added to Nexus.

 This completes the implementation of message batching in all protocols,
 and contains the complete Async Invocation API as well. Once we have an
 implementation of async request handling in Infinispan, which runs
 independent transactions in separate threads from an internal thread
 pool, I expect a significant performance increase !


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


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

2013-02-28 Thread 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)




___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev


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

2013-02-28 Thread Mircea Markus

On 28 Feb 2013, at 15:31, Pedro Ruivo 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?
we shouldn't create an interceptor yet, perhaps we'll do that with ISPN-2849.
 
 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.
+1 

Cheers,
-- 
Mircea Markus
Infinispan lead (www.infinispan.org)




___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev


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

2013-02-28 Thread Dan Berindei
On Thu, Feb 28, 2013 at 2:18 PM, Mircea Markus mmar...@redhat.com 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.


RollbackCommand can block as well, if it needs to be forwarded to other
nodes.



 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.

 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?


Yes, CacheTopologyControlCommand can definitely block.
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

2013-02-28 Thread Dan Berindei
Actually some of the commands you mentioned don't go through the
interceptor chain (CacheTopologyControlCommand, StateRequestCommand,
StateRequestCommand etc.) so you can't use an interceptor to move them to a
separate thread pool.


On Thu, Feb 28, 2013 at 5:47 PM, Mircea Markus mmar...@redhat.com wrote:


 On 28 Feb 2013, at 15:31, Pedro Ruivo 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?
 we shouldn't create an interceptor yet, perhaps we'll do that with
 ISPN-2849.
 
  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.
 +1

 Cheers,
 --
 Mircea Markus
 Infinispan lead (www.infinispan.org)




 ___
 infinispan-dev mailing list
 infinispan-dev@lists.jboss.org
 https://lists.jboss.org/mailman/listinfo/infinispan-dev

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

2013-02-28 Thread Pedro Ruivo


On 02/28/2013 03:47 PM, Mircea Markus wrote:

 On 28 Feb 2013, at 15:31, Pedro Ruivo 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?
 we shouldn't create an interceptor yet, perhaps we'll do that with ISPN-2849.

 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.
 +1
However I prefer to do this dispatch in CARD to avoid make to code more 
complex and duplicate it. I mean, in CARD we have:

handle() {
if is from another site
   process from another site
else
   if is cache rpc command
 inboundInvocationHandler.handle
   else
 command.perform
}

and if I move the dispatch inside the InbounInvocationHandler, as 
suggested, I will have to duplicate the code for each branch of the IF, 
namelly:

try
   command.perform
   transport.sendResponse
catch Thowable t
   transport.sendResponse(ExceptionResponse)

any thoughts?


 Cheers,

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev


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

2013-02-28 Thread Mircea Markus

On 28 Feb 2013, at 17:30, Pedro Ruivo wrote:

 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?
 we shouldn't create an interceptor yet, perhaps we'll do that with ISPN-2849.
 
 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.
 +1
 However I prefer to do this dispatch in CARD to avoid make to code more 
 complex and duplicate it. I mean, in CARD we have:
 
 handle() {
 if is from another site
   process from another site
 else
   if is cache rpc command
 inboundInvocationHandler.handle
   else
 command.perform
 }
 
 and if I move the dispatch inside the InbounInvocationHandler, as 
 suggested, I will have to duplicate the code for each branch of the IF, 
 namelly:
 
 try
   command.perform
   transport.sendResponse
 catch Thowable t
   transport.sendResponse(ExceptionResponse)
 
If it's only for this try-catch duplication then it should be fine.  

Cheers,
-- 
Mircea Markus
Infinispan lead (www.infinispan.org)





___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev