This is a rather long but please bear with me: So, I've been looking at the number of threads created in our testsuite because quite often I'm getting OOM "unable to create thread" and note that tests have evolved to be quite complex with several CacheManagers, Caches...etc. In summary, I see plenty of maxThread=1 ThreadPoolExecutor (TPE) instances created that then are either useless (i.e. they get overriden after rewiring) or are there even though they're not gonna be used in the test (i.e. async listener notifier). Side note: Note that maxThread is used for the core pool size which means that this means that there's 1 thread available. For the useless TPEs, we need to avoid creating them in the 1st place. For the useful but not used in test, we should have an option that allows for such executors to be created lazily in a testsuite env. I see the point of creating these in advance in a prod or user env but when running the testsuite, it'd be better and would safe resources, specially wrt to threads if we created them lazily. Examples: For each CacheManager created, 2 x TPEs for ASYNC_NOTIFICATION_EXECUTOR are created. The second one during the re-wiring process. The result is that the 1st one is never used any more and it's left sitting there. Why is a 2nd one created? Something to do with non-volatility? Indeed it is. The j.u.e.ExecutorService instance is a j.u.c.TPE which has no NonVolatile annotation and hence it gets recreated. Note that this happens for all TPE usages (ASYNC_NOTIFICATION_EXECUTOR, ASYNC_TRANSPORT_EXECUTOR, EVICTION_SCHEDULED_EXECUTOR, ASYNC_REPLICATION_QUEUE_EXECUTOR)! For each one created, there's another one left there taking up resources!! So, what are the options? Two: - A question that seems to come up more and more, get rid of the Volatility/NonVolatility difference and treat everything as NonVolatile? Apart from simplyfying the code, it'd make easier to debug cos figuring out these type of issues are a royal PITA. I remember hearing Manik that since create and start phases were gone, this should be no problem. - Or provide our own ExecutorService and TPE facades that allows use to annotate TPE as @NonVolatile. For each Cache created, the same thing, 2 x TPE for ASYNC_NOTIFICATION_EXECUTOR are created. The lazyness I was refering about earlier comes into play here in the sense that, why not start the TPE the 1st time we register an async Listener? If we did so, we'd be saving resources if no async listeners are enabled (This looks to me the most common case right now). I can see the point of having this ready for normal use but certainly for testsuite use, this is waste when we're creating so many CacheManagers, Caches instances and have several threads running different tests at the same time. Would it make sense to apply such lazyness to ASYNC_TRANSPORT_EXECUTOR? Hmmmm, even in a configured SYNC configuration, there might be stuff we wanna send asynchronously, i.e. PFERs, so don't think so. With regards to EVICTION_SCHEDULED_EXECUTOR, this is a cache component. Currently, a TPE is created regardless of whether eviction is enabled or not and eviction is not a dynamic property. Now, if eviction is set to NONE, neither eviction nor expiration happens, correct? In that case, I don't see why TPE should be maintained if eviction is disabled. To finish with the lazy TPE startup topic, with ASYNC_REPLICATION_QUEUE_EXECUTOR, we're certainly doing a good job. In ReplicationQueueFactory, we only construct a ReplicationQueue and corresponding TPE if replication queue is enabled More comments now. Since there are 4 different usages of ThreadPoolExecutor each should have, *by default*, a different thread name prefix. Currently, there's no such thing which means that figuring out what a particular thread belongs to is difficult unless threadNamePrefix are set in each set of properties. Another thing that is very confusing is the DefaultExecutorFactory.counter. What is it trying to represent? For example, in the testsuite thread dumps, I was seeing thread names like: asyncThread-150 and this is very confusing when taking in account that maxThreads in each ThreadPoolExecutor in the testsuite is 1. Finally, KnownComponentNames should probably be an enum, shouldn't it? If you made it this far, thx!!

On 6 Oct 2009, at 15:59, Galder Zamarreno wrote: > </snip> > > More comments now. Since there are 4 different usages of > ThreadPoolExecutor each should have, *by default*, a different thread > name prefix. Currently, there's no such thing which means that > figuring > out what a particular thread belongs to is difficult unless > threadNamePrefix are set in each set of properties. Yes, there should be a way of handling this - perhaps related to the component name if it isn't overridden with a property. I'll have a look at this too.

This is a rather long but please bear with me: So, I've been looking at the number of threads created in our testsuite because quite often I'm getting OOM "unable to create thread" and note that tests have evolved to be quite complex with several CacheManagers, Caches...etc. In summary, I see plenty of maxThread=1 ThreadPoolExecutor (TPE) instances created that then are either useless (i.e. they get overriden after rewiring) or are there even though they're not gonna be used in the test (i.e. async listener notifier). Side note: Note that maxThread is used for the core pool size which means that this means that there's 1 thread available. For the useless TPEs, we need to avoid creating them in the 1st place. For the useful but not used in test, we should have an option that allows for such executors to be created lazily in a testsuite env. I see the point of creating these in advance in a prod or user env but when running the testsuite, it'd be better and would safe resources, specially wrt to threads if we created them lazily. Examples: For each CacheManager created, 2 x TPEs for ASYNC_NOTIFICATION_EXECUTOR are created. The second one during the re-wiring process. The result is that the 1st one is never used any more and it's left sitting there. Why is a 2nd one created? Something to do with non-volatility? Indeed it is. The j.u.e.ExecutorService instance is a j.u.c.TPE which has no NonVolatile annotation and hence it gets recreated. Note that this happens for all TPE usages (ASYNC_NOTIFICATION_EXECUTOR, ASYNC_TRANSPORT_EXECUTOR, EVICTION_SCHEDULED_EXECUTOR, ASYNC_REPLICATION_QUEUE_EXECUTOR)! For each one created, there's another one left there taking up resources!! So, what are the options? Two: - A question that seems to come up more and more, get rid of the Volatility/NonVolatility difference and treat everything as NonVolatile? Apart from simplyfying the code, it'd make easier to debug cos figuring out these type of issues are a royal PITA. I remember hearing Manik that since create and start phases were gone, this should be no problem. - Or provide our own ExecutorService and TPE facades that allows use to annotate TPE as @NonVolatile. For each Cache created, the same thing, 2 x TPE for ASYNC_NOTIFICATION_EXECUTOR are created. The lazyness I was refering about earlier comes into play here in the sense that, why not start the TPE the 1st time we register an async Listener? If we did so, we'd be saving resources if no async listeners are enabled (This looks to me the most common case right now). I can see the point of having this ready for normal use but certainly for testsuite use, this is waste when we're creating so many CacheManagers, Caches instances and have several threads running different tests at the same time. Would it make sense to apply such lazyness to ASYNC_TRANSPORT_EXECUTOR? Hmmmm, even in a configured SYNC configuration, there might be stuff we wanna send asynchronously, i.e. PFERs, so don't think so. With regards to EVICTION_SCHEDULED_EXECUTOR, this is a cache component. Currently, a TPE is created regardless of whether eviction is enabled or not and eviction is not a dynamic property. Now, if eviction is set to NONE, neither eviction nor expiration happens, correct? In that case, I don't see why TPE should be maintained if eviction is disabled. To finish with the lazy TPE startup topic, with ASYNC_REPLICATION_QUEUE_EXECUTOR, we're certainly doing a good job. In ReplicationQueueFactory, we only construct a ReplicationQueue and corresponding TPE if replication queue is enabled More comments now. Since there are 4 different usages of ThreadPoolExecutor each should have, *by default*, a different thread name prefix. Currently, there's no such thing which means that figuring out what a particular thread belongs to is difficult unless threadNamePrefix are set in each set of properties. Another thing that is very confusing is the DefaultExecutorFactory.counter. What is it trying to represent? For example, in the testsuite thread dumps, I was seeing thread names like: asyncThread-150 and this is very confusing when taking in account that maxThreads in each ThreadPoolExecutor in the testsuite is 1. Finally, KnownComponentNames should probably be an enum, shouldn't it? If you made it this far, thx!!

On 10/09/2009 04:19 AM, Vladimir Blagojevic wrote: > Galder, > > Under profiler none of these seem to be a big problem except > asyncTransport TPE. All of these threads that you saw in thread dumps > are related to rehash requests spawned by LeaveTask. LeaveTask tries to > submit request to TPE and is block waiting. All of these threads are. I > am still trying to figure out what is going. Block waiting where? > > Do you understand the lifecycle of asyncTransport TPE? Is there one > asyncTransport TPE per cache manager? We also seem to create > asyncTransport TPE with only one thread? Is this intended? I don't in detail but it appears to be used by several components (CommandAwareRpcDispatcher and RpcManagerImpl) for things like asynchronous marshalling, LeaveTask...etc. There is one asyncTransport TPE per manager yeah. I've seen that we keep creating asyncTransport TPE with one thread indeed but haven't seen any particular blocking scenario arising from that small pool. I'll double check again next time I run the testsuite.

> > Regards > > On 09-10-06 10:59 AM, Galder Zamarreno wrote: >> This is a rather long but please bear with me: >> >> So, I've been looking at the number of threads created in our testsuite >> because quite often I'm getting OOM "unable to create thread" and note >> that tests have evolved to be quite complex with several CacheManagers, >> Caches...etc. >> >> In summary, I see plenty of maxThread=1 ThreadPoolExecutor (TPE) >> instances created that then are either useless (i.e. they get overriden >> after rewiring) or are there even though they're not gonna be used in >> the test (i.e. async listener notifier). Side note: Note that maxThread >> is used for the core pool size which means that this means that there's >> 1 thread available. >> >> For the useless TPEs, we need to avoid creating them in the 1st place. >> For the useful but not used in test, we should have an option that >> allows for such executors to be created lazily in a testsuite env. I see >> the point of creating these in advance in a prod or user env but when >> running the testsuite, it'd be better and would safe resources, >> specially wrt to threads if we created them lazily. >> >> Examples: >> >> For each CacheManager created, 2 x TPEs for ASYNC_NOTIFICATION_EXECUTOR >> are created. The second one during the re-wiring process. The result is >> that the 1st one is never used any more and it's left sitting there. Why >> is a 2nd one created? Something to do with non-volatility? Indeed it is. >> The j.u.e.ExecutorService instance is a j.u.c.TPE which has no >> NonVolatile annotation and hence it gets recreated. Note that this >> happens for all TPE usages (ASYNC_NOTIFICATION_EXECUTOR, >> ASYNC_TRANSPORT_EXECUTOR, EVICTION_SCHEDULED_EXECUTOR, >> ASYNC_REPLICATION_QUEUE_EXECUTOR)! For each one created, there's another >> one left there taking up resources!! So, what are the options? Two: >> >> - A question that seems to come up more and more, get rid of the >> Volatility/NonVolatility difference and treat everything as NonVolatile? >> Apart from simplyfying the code, it'd make easier to debug cos figuring >> out these type of issues are a royal PITA. I remember hearing Manik that >> since create and start phases were gone, this should be no problem. >> >> - Or provide our own ExecutorService and TPE facades that allows use to >> annotate TPE as @NonVolatile. >> >> For each Cache created, the same thing, 2 x TPE for >> ASYNC_NOTIFICATION_EXECUTOR are created. >> >> The lazyness I was refering about earlier comes into play here in the >> sense that, why not start the TPE the 1st time we register an async >> Listener? If we did so, we'd be saving resources if no async listeners >> are enabled (This looks to me the most common case right now). I can see >> the point of having this ready for normal use but certainly for >> testsuite use, this is waste when we're creating so many CacheManagers, >> Caches instances and have several threads running different tests at the >> same time. >> >> Would it make sense to apply such lazyness to ASYNC_TRANSPORT_EXECUTOR? >> Hmmmm, even in a configured SYNC configuration, there might be stuff we >> wanna send asynchronously, i.e. PFERs, so don't think so. >> >> With regards to EVICTION_SCHEDULED_EXECUTOR, this is a cache component. >> Currently, a TPE is created regardless of whether eviction is enabled or >> not and eviction is not a dynamic property. Now, if eviction is set to >> NONE, neither eviction nor expiration happens, correct? In that case, I >> don't see why TPE should be maintained if eviction is disabled. >> >> To finish with the lazy TPE startup topic, with >> ASYNC_REPLICATION_QUEUE_EXECUTOR, we're certainly doing a good job. In >> ReplicationQueueFactory, we only construct a ReplicationQueue and >> corresponding TPE if replication queue is enabled >> >> More comments now. Since there are 4 different usages of >> ThreadPoolExecutor each should have, *by default*, a different thread >> name prefix. Currently, there's no such thing which means that figuring >> out what a particular thread belongs to is difficult unless >> threadNamePrefix are set in each set of properties. >> >> Another thing that is very confusing is the >> DefaultExecutorFactory.counter. What is it trying to represent? For >> example, in the testsuite thread dumps, I was seeing thread names like: >> asyncThread-150 and this is very confusing when taking in account that >> maxThreads in each ThreadPoolExecutor in the testsuite is 1. >> >> Finally, KnownComponentNames should probably be an enum, shouldn't it? >> >> If you made it this far, thx!! >> > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev

On 09-10-09 11:32 AM, Galder Zamarreno wrote: > > On 10/09/2009 04:19 AM, Vladimir Blagojevic wrote: > >> Galder, >> >> Under profiler none of these seem to be a big problem except >> asyncTransport TPE. All of these threads that you saw in thread dumps >> are related to rehash requests spawned by LeaveTask. LeaveTask tries to >> submit request to TPE and is block waiting. All of these threads are. I >> am still trying to figure out what is going. >> > Block waiting where? > ExecutorService#submit(Callable)<---- profiler shows thread is blocked here RPCManagerImpl#invokeRemotelyInFuture

Sorry for some reason there is no copy-paste feature for a thread stack in the profiler! I took a snapshot 3/4 into test suite and there more than 20 threads blocked in the same line code. _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

On 9 Oct 2009, at 16:57, Galder Zamarreno wrote: > > > On 10/09/2009 05:49 PM, Vladimir Blagojevic wrote: >> On 09-10-09 11:32 AM, Galder Zamarreno wrote: >>> >>> On 10/09/2009 04:19 AM, Vladimir Blagojevic wrote: >>> >>>> Galder, >>>> >>>> Under profiler none of these seem to be a big problem except >>>> asyncTransport TPE. All of these threads that you saw in thread >>>> dumps >>>> are related to rehash requests spawned by LeaveTask. LeaveTask >>>> tries to >>>> submit request to TPE and is block waiting. All of these threads >>>> are. I >>>> am still trying to figure out what is going. >>>> >>> Block waiting where? >>> >> ExecutorService#submit(Callable)<---- profiler shows thread is >> blocked here >> RPCManagerImpl#invokeRemotelyInFuture This can't be about the pool size being too small. I'll have a look, I'm going to be revamping the dist tests this week anyway.

On 6 Oct 2009, at 15:59, Galder Zamarreno wrote: > > The lazyness I was refering about earlier comes into play here in the > sense that, why not start the TPE the 1st time we register an async > Listener? If we did so, we'd be saving resources if no async listeners > are enabled (This looks to me the most common case right now). I can > see > the point of having this ready for normal use but certainly for > testsuite use, this is waste when we're creating so many > CacheManagers, > Caches instances and have several threads running different tests at > the > same time. Regarding laziness, I don't think this is an issue. Let's take the case of the DefaultExecutorFactory (and DefaultScheduledExecutorFactory). We create the executors using the JDK ExecutorService, passing in a ThreadFactory impl. So while we may create the executor upfront, this is relatively lightweight (except perhaps for the task queue, but again we use LinkedBlockingQueues which are cheap to construct). The threads themselves to populate the threadpool used by the executor is populated lazily (see j.u.c.ThreadPoolExecutor#execute(), Sun JDK 6).

Cheers -- Manik Surtani manik(a)jboss.org Lead, Infinispan Lead, JBoss Cache http://www.infinispan.org http://www.jbosscache.org _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev