Re: [infinispan-dev] Why no use async listener executor?
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?
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?
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?
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?
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?
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