Re: [infinispan-dev] Why no use async listener executor?

2011-10-10 Thread Mircea Markus
 
 Hey guys,
 
 Re: https://issues.jboss.org/browse/ISPN-1396
 
 While looking into this, I've discovered that we have been creating 
 executors in cache level components, where we're calling submit from 
 @Listener implementations.
 
 For example, BaseStateTransferManagerImpl.ViewChangeListener submits a 
 rehash task and TransactionTable.StaleTransactionCleanup submits a tasks 
 to break locks.
 
 Did the people that wrote these listeners (Dan  Mircea respectively) 
 consider defining listeners to be called asynchronously? i.e. 
 @Listener(sync = false)
 this can work indeed for BaseStateTransferManagerImpl.ViewChangeListener? 
 This seems like a small change - can you modify that in the scope of 
 ISPN-1396 or do you want me to look into it?
 
 BaseStateTransferManagerImpl.ViewChangeListener? Dan seems to disagree. You 
 meant TransactionTable.StaleTransactionCleanup instead?
 yes indeed :-)
 
 Ok. I'll get this changed as part of ISPN-1396.
Thank you.
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev


Re: [infinispan-dev] Why no use async listener executor?

2011-10-07 Thread Galder Zamarreño

On Oct 6, 2011, at 5:48 PM, Mircea Markus wrote:

 
 On 6 Oct 2011, at 15:52, Galder Zamarreño wrote:
 
 
 On Oct 6, 2011, at 4:04 PM, Mircea Markus wrote:
 
 
 On 5 Oct 2011, at 16:53, Galder Zamarreño wrote:
 
 Hey guys,
 
 Re: https://issues.jboss.org/browse/ISPN-1396
 
 While looking into this, I've discovered that we have been creating 
 executors in cache level components, where we're calling submit from 
 @Listener implementations.
 
 For example, BaseStateTransferManagerImpl.ViewChangeListener submits a 
 rehash task and TransactionTable.StaleTransactionCleanup submits a tasks 
 to break locks.
 
 Did the people that wrote these listeners (Dan  Mircea respectively) 
 consider defining listeners to be called asynchronously? i.e. 
 @Listener(sync = false)
 this can work indeed for BaseStateTransferManagerImpl.ViewChangeListener? 
 This seems like a small change - can you modify that in the scope of 
 ISPN-1396 or do you want me to look into it?
 
 BaseStateTransferManagerImpl.ViewChangeListener? Dan seems to disagree. You 
 meant TransactionTable.StaleTransactionCleanup instead?
 yes indeed :-)

Ok. I'll get this changed as part of ISPN-1396.

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

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


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


Re: [infinispan-dev] Why no use async listener executor?

2011-10-07 Thread Galder Zamarreño
Btw, state transfer and lock break service's executors are based around the 
concept of having a single thread running for each of the executors and either 
discarding any other requests (i.e. view changes) while there's an executor in 
use.

So, how do these carry on functioning the same way when all caches share the 
same cache manager? You can't just have an executor of 1 because it could 
easily cause different caches to block each other. If you have a thread pool of 
N, you could easily find yourself in the situation where multiple threads 
execute rehashing for a single cache for example, which I don't think is 
desirable.

A custom thread factory could make sure only as many different cache threads 
are created, but the problem is still there. If you have 5 caches and you 
create 5 threads, two consecutive view changes for a cache could still result 
in two paralell threads executing it.

I can't think of an easy way to resolve this with the standard executors 
available and maintaining a central executor for all caches. 

This might be a situation where it does make sense having cache specific 
executor configuration. Besides, these executors are single thread ones, so 
even if you create thousands of caches, the cost should be relatively small.

Thoughts?

On Oct 7, 2011, at 9:05 AM, Galder Zamarreño wrote:

 
 On Oct 6, 2011, at 5:48 PM, Mircea Markus wrote:
 
 
 On 6 Oct 2011, at 15:52, Galder Zamarreño wrote:
 
 
 On Oct 6, 2011, at 4:04 PM, Mircea Markus wrote:
 
 
 On 5 Oct 2011, at 16:53, Galder Zamarreño wrote:
 
 Hey guys,
 
 Re: https://issues.jboss.org/browse/ISPN-1396
 
 While looking into this, I've discovered that we have been creating 
 executors in cache level components, where we're calling submit from 
 @Listener implementations.
 
 For example, BaseStateTransferManagerImpl.ViewChangeListener submits a 
 rehash task and TransactionTable.StaleTransactionCleanup submits a tasks 
 to break locks.
 
 Did the people that wrote these listeners (Dan  Mircea respectively) 
 consider defining listeners to be called asynchronously? i.e. 
 @Listener(sync = false)
 this can work indeed for BaseStateTransferManagerImpl.ViewChangeListener? 
 This seems like a small change - can you modify that in the scope of 
 ISPN-1396 or do you want me to look into it?
 
 BaseStateTransferManagerImpl.ViewChangeListener? Dan seems to disagree. You 
 meant TransactionTable.StaleTransactionCleanup instead?
 yes indeed :-)
 
 Ok. I'll get this changed as part of ISPN-1396.
 
 
 
 
 ___
 infinispan-dev mailing list
 infinispan-dev@lists.jboss.org
 https://lists.jboss.org/mailman/listinfo/infinispan-dev
 
 --
 Galder Zamarreño
 Sr. Software Engineer
 Infinispan, JBoss Cache
 
 
 ___
 infinispan-dev mailing list
 infinispan-dev@lists.jboss.org
 https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


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


Re: [infinispan-dev] Why no use async listener executor?

2011-10-06 Thread Dan Berindei
Galder, I didn't know about @Listener(sync = false), but a separate
executor makes sense for state transfer because I didn't want
concurrent state transfers and I could set it up to automatically
cancels any state transfer task that's waiting in the queue.

For the StaleTransactionCleanup I suppose we could use an async
listener but it would definitely need a bigger thread pool because the
user might have async listeners as well.


If only you'd have sent this email one day before, I've been been
trying to reuse the async transport executor in my cache views
manager, but I didn't know it's also configured to use 1 thread max
and I spent a good part of yesterday trying to understand what's going
on :)

I eventually moved on to create my own executor because I also wanted
to name my threads better and I couldn't find a simple way to include
the node name in the async transport executor's thread names, but I'm
pretty sure 1 is not a reasonable default for the async transport
either.

Cheers
Dan


On Wed, Oct 5, 2011 at 6:53 PM, Galder Zamarreño gal...@redhat.com wrote:
 Hey guys,

 Re: https://issues.jboss.org/browse/ISPN-1396

 While looking into this, I've discovered that we have been creating executors 
 in cache level components, where we're calling submit from @Listener 
 implementations.

 For example, BaseStateTransferManagerImpl.ViewChangeListener submits a rehash 
 task and TransactionTable.StaleTransactionCleanup submits a tasks to break 
 locks.

 Did the people that wrote these listeners (Dan  Mircea respectively) 
 consider defining listeners to be called asynchronously? i.e. @Listener(sync 
 = false)

 Unless you have very strict requirements, the async listener executor should 
 just work, and would lead to reuse of executors which results in lower 
 consumption.

 The same applies to the singleton store executor btw.

 Surely, if we were to expand on to other use cases, async notification 
 executor should have more than 1 max thread by default, otherwise we could 
 easily hold up tasks.

 Cheers,
 --
 Galder Zamarreño
 Sr. Software Engineer
 Infinispan, JBoss Cache


 ___
 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] Why no use async listener executor?

2011-10-06 Thread Galder Zamarreño

On Oct 6, 2011, at 12:26 PM, Dan Berindei wrote:

 Galder, I didn't know about @Listener(sync = false),

Now you know :)

 but a separate
 executor makes sense for state transfer because I didn't want
 concurrent state transfers and I could set it up to automatically
 cancels any state transfer task that's waiting in the queue.

Fair point.

 For the StaleTransactionCleanup I suppose we could use an async
 listener but it would definitely need a bigger thread pool because the
 user might have async listeners as well.

For sure, I was planning to expand it to 5/10 threads by default to extend its 
usage.

 If only you'd have sent this email one day before,

I'm working on my mind reading skills, but not there yet... ;)

 I've been been
 trying to reuse the async transport executor in my cache views
 manager, but I didn't know it's also configured to use 1 thread max
 and I spent a good part of yesterday trying to understand what's going
 on :)
 I eventually moved on to create my own executor because I also wanted
 to name my threads better and I couldn't find a simple way to include
 the node name in the async transport executor's thread names, but I'm
 pretty sure 1 is not a reasonable default for the async transport
 either.

I think we should have a chat online to see what exactly you're trying to do 
and see if we can accomodate it. I'm looking into this area right now.

Btw, if you end up creating your executor, you need to follow the global 
configuration rules. I'd probably suggest that I commit first 
https://issues.jboss.org/browse/ISPN-1396 so that you can get an idea of how I 
extended other parts to use shared executors.

Btw, async transport does not use 1 as default, but instead 25 threads, see 
KnownComponentNames.

 
 Cheers
 Dan
 
 
 On Wed, Oct 5, 2011 at 6:53 PM, Galder Zamarreño gal...@redhat.com wrote:
 Hey guys,
 
 Re: https://issues.jboss.org/browse/ISPN-1396
 
 While looking into this, I've discovered that we have been creating 
 executors in cache level components, where we're calling submit from 
 @Listener implementations.
 
 For example, BaseStateTransferManagerImpl.ViewChangeListener submits a 
 rehash task and TransactionTable.StaleTransactionCleanup submits a tasks to 
 break locks.
 
 Did the people that wrote these listeners (Dan  Mircea respectively) 
 consider defining listeners to be called asynchronously? i.e. @Listener(sync 
 = false)
 
 Unless you have very strict requirements, the async listener executor should 
 just work, and would lead to reuse of executors which results in lower 
 consumption.
 
 The same applies to the singleton store executor btw.
 
 Surely, if we were to expand on to other use cases, async notification 
 executor should have more than 1 max thread by default, otherwise we could 
 easily hold up tasks.
 
 Cheers,
 --
 Galder Zamarreño
 Sr. Software Engineer
 Infinispan, JBoss Cache
 
 
 ___
 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

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


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


[infinispan-dev] Why no use async listener executor?

2011-10-05 Thread Galder Zamarreño
Hey guys,

Re: https://issues.jboss.org/browse/ISPN-1396

While looking into this, I've discovered that we have been creating executors 
in cache level components, where we're calling submit from @Listener 
implementations.

For example, BaseStateTransferManagerImpl.ViewChangeListener submits a rehash 
task and TransactionTable.StaleTransactionCleanup submits a tasks to break 
locks.

Did the people that wrote these listeners (Dan  Mircea respectively) consider 
defining listeners to be called asynchronously? i.e. @Listener(sync = false)

Unless you have very strict requirements, the async listener executor should 
just work, and would lead to reuse of executors which results in lower 
consumption.

The same applies to the singleton store executor btw.

Surely, if we were to expand on to other use cases, async notification executor 
should have more than 1 max thread by default, otherwise we could easily hold 
up tasks.

Cheers,
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


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