On 15 Sep 2010, at 12:45, Galder Zamarreño wrote: > Hi, > > It seems like there's some confusion about what https://jira.jboss.org/browse/ISPN-649 is trying to fix. Let me explain this in further detail: > > Paul found some issues with redeployment of JPA apps and 2nd level cache (see http://opensource.atlassian.com/projects/hibernate/browse/HHH-5563 - currently offline btw). Basically, when SessionFactories are shutdown, all caches are stopped but in an AS environment, the CacheManager keeps running since it's deployed as a shared resource. Upon redeployment and retrieving the caches from the CacheManager, this were in TERMINATED state which meant that no replication happened between them. However, you could, as a client make modifications on those caches if these were local, and you could retrieve data without probs. Obviously, this shouldn't be the case. > > Paul's suggestion was that in the 2LC, the code would check the status of the cache and if it was in TERMINATED state, restart it (he actually implemented the condition checking if invocations were allowed, but don't think it's a good idea since other temporary statuses would lead to restarts, i.e. INITIALIZING and that could cause issue). Actually I think Paul is correct in that this should live in the Hibernate 2LC code. > I disagree with Paul in that such check and restart should be done in DefaultCacheManager. So, DCM, whenever someone calls any of getCache(), it would check whether the cache is TERMINATED, and if it is, it would restart it. After thinking this, I started to consider other possibilities: What happens if the client holds a ref to Cache and someone else stops it? IMO, CacheDelegate should protect against it and if it's TERMINATED, it should restart itself. -1 to caches restarting themselves. :) Imagine, I really want to stop the cache. I call cache.stop() but because some errant thread is calling operations against it, it keeps getting restarted. Restarting should be an explicit thing - in Hibernate and in JBoss AS, you know when something has been redeployed so you know that if your caches have been stopped you will explicitly restart them. This makes sense.

> Finally, I also considered what would happen if not only Cache was stopped, but DCM was stopped as well. What happens when you call getCache()? When testing this, I discovered that it was returning TERMINATED caches as well. So then I considered whether DCM should attempt to restart itself, but I decided not to do this cos most of the initialisation happens in the ctror, so it's not designed to be restarted. As a result, DCM now throws an IllegalStateException if you call getCache() and it's TERMINATED. Same thing happens with Cache if it discovers that not only it's TERMINATED, but the CacheManager to which it belongs is terminated and this way we cover for cached Cache instances. Yup this makes sense. > So, to sum up: > - Caches can be restarted by either DCM or the Cache instances themselves if they see them as TERMINATED. This should not happen "automagically"! > - CacheManager's cannot be restarted. Instead a new instance needs to be created. > > Hope this clarifies the issue. Please find a patch containing the fix for this. It includes a test case with the possibilities mentioned above: > > <If_stopped,_Caches_can_now_be_restarted__CacheContainers_still_need_recreating_after_being.patch> > > Cheers, > -- > Galder Zamarreño > Sr. Software Engineer > Infinispan, JBoss Cache > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev -- 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

On 15 Sep 2010, at 15:33, Galder Zamarreño wrote: > See below: > > On Sep 15, 2010, at 4:22 PM, Manik Surtani wrote: > >> >> On 15 Sep 2010, at 12:45, Galder Zamarreño wrote: >> >>> Hi, >>> >>> It seems like there's some confusion about what https://jira.jboss.org/browse/ISPN-649 is trying to fix. Let me explain this in further detail: >>> >>> Paul found some issues with redeployment of JPA apps and 2nd level cache (see http://opensource.atlassian.com/projects/hibernate/browse/HHH-5563 - currently offline btw). Basically, when SessionFactories are shutdown, all caches are stopped but in an AS environment, the CacheManager keeps running since it's deployed as a shared resource. Upon redeployment and retrieving the caches from the CacheManager, this were in TERMINATED state which meant that no replication happened between them. However, you could, as a client make modifications on those caches if these were local, and you could retrieve data without probs. Obviously, this shouldn't be the case. >>> >>> Paul's suggestion was that in the 2LC, the code would check the status of the cache and if it was in TERMINATED state, restart it (he actually implemented the condition checking if invocations were allowed, but don't think it's a good idea since other temporary statuses would lead to restarts, i.e. INITIALIZING and that could cause issue). >> >> Actually I think Paul is correct in that this should live in the Hibernate 2LC code. >> >>> I disagree with Paul in that such check and restart should be done in DefaultCacheManager. So, DCM, whenever someone calls any of getCache(), it would check whether the cache is TERMINATED, and if it is, it would restart it. After thinking this, I started to consider other possibilities: What happens if the client holds a ref to Cache and someone else stops it? IMO, CacheDelegate should protect against it and if it's TERMINATED, it should restart itself. >> >> -1 to caches restarting themselves. :) Imagine, I really want to stop the cache. I call cache.stop() but because some errant thread is calling operations against it, it keeps getting restarted. >> >> Restarting should be an explicit thing - in Hibernate and in JBoss AS, you know when something has been redeployed so you know that if your caches have been stopped you will explicitly restart them. This makes sense. > > Hmmm, I can see the point of not forcing a specific behaviour but I think the latter is a more common case. I can live with this code being in 2LC but if I see a lot of people requesting this, we should consider this as an option. Let's see if it is generically useful first - I suspect not. > What is true is that if we agree not to restart, then we cannot allow invocations on a TERMINATED cache. Particularly, we cannot allow anything that modifies to succeed. Anything that reads can simply return null. Anything that modifies should prob through an IllegalStateException. Remember that a TERMINATED cache cannot receive any commands from other nodes, so surely should not allow anything local either. Of course. Operations on terminated components should throw ISEs.

As per ComponentStatus.allowInvocations(), invocations should not be allowed on TERMINATED components. http://fisheye.jboss.org/browse/Infinispan/branches/4.2.x/core/src/main/j... > >> >>> Finally, I also considered what would happen if not only Cache was stopped, but DCM was stopped as well. What happens when you call getCache()? When testing this, I discovered that it was returning TERMINATED caches as well. So then I considered whether DCM should attempt to restart itself, but I decided not to do this cos most of the initialisation happens in the ctror, so it's not designed to be restarted. As a result, DCM now throws an IllegalStateException if you call getCache() and it's TERMINATED. Same thing happens with Cache if it discovers that not only it's TERMINATED, but the CacheManager to which it belongs is terminated and this way we cover for cached Cache instances. >> >> Yup this makes sense. >> >>> So, to sum up: >>> - Caches can be restarted by either DCM or the Cache instances themselves if they see them as TERMINATED. >> >> This should not happen "automagically"! >> >>> - CacheManager's cannot be restarted. Instead a new instance needs to be created. >>> >>> Hope this clarifies the issue. Please find a patch containing the fix for this. It includes a test case with the possibilities mentioned above: >>> >>> <If_stopped,_Caches_can_now_be_restarted__CacheContainers_still_need_recreating_after_being.patch> >>> >>> Cheers, >>> -- >>> Galder Zamarreño >>> Sr. Software Engineer >>> Infinispan, JBoss Cache >>> >>> _______________________________________________ >>> infinispan-dev mailing list >>> infinispan-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >> >> -- >> 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 > > -- > Galder Zamarreño > Sr. Software Engineer > Infinispan, JBoss Cache > > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev -- 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