From: "Galder Zamarreño" <galder(a)redhat.com&gt; To: "Andrig Miller" <anmiller(a)redhat.com&gt; Cc: "Steve Ebersole" <steve(a)hibernate.org&gt;, "John O'Hara" <johara(a)redhat.com&gt;, "Jeremy Whiting" <jwhiting(a)redhat.com&gt;, "infinispan -Dev List" <infinispan-dev(a)lists.jboss.org&gt; Sent: Thursday, September 20, 2012 6:48:59 AM Subject: Re: [infinispan-dev] Issue with cache blocks for local read-only cache On Sep 19, 2012, at 4:20 PM, Andrig Miller <anmiller(a)redhat.com&gt; wrote: > Yes, I can see how that can happen, if the data is deleted from > outside the application. ^ The issue does not only happen if the data is deleted outside the application. As indicated in https://hibernate.onjira.com/browse/HHH-3817, this can happen with two competing transactions. > If you cache something as READ_ONLY, and it gets deleted, that > doesn't fit the definition of READ_ONLY though. You are using the > wrong cache concurrency strategy. > > Even that issue outlines the scenario where the collection is > updated, which means its not a READ_ONLY. I think the update is irrelevant here. The issue is related to putFromLoad + remove, which both AFAIK, are allowed in READ_ONLY (remember that we had the discussion on whether remove should be allowed in a READ_ONLY cache: https://hibernate.onjira.com/browse/HHH-7350).

> > Andy > > ----- Original Message ----- >> From: "Galder Zamarreño" <galder(a)redhat.com&gt; >> To: "infinispan -Dev List" <infinispan-dev(a)lists.jboss.org&gt; >> Cc: "Steve Ebersole" <steve(a)hibernate.org&gt;, "John O'Hara" >> <johara(a)redhat.com&gt;, "Andrig Miller" <anmiller(a)redhat.com&gt;, >> "Jeremy Whiting" <jwhiting(a)redhat.com&gt; >> Sent: Wednesday, September 19, 2012 2:48:37 AM >> Subject: Re: [infinispan-dev] Issue with cache blocks for local >> read-only cache >> >> This is code written in JBoss Cache days to deal with situations >> where putFromLoad might try to store stale data (if data is >> deleted >> in between a database read and putFromLoad being called). >> >> Indeed this can happen with read only data, and has nothing to do >> with clustering. >> >> The original issue is: >> https://hibernate.onjira.com/browse/HHH-3817 >> >> I'll check how this can be improved. >> >> Cheers, >> >> On Sep 18, 2012, at 4:55 PM, Sanne Grinovero >> <sanne(a)infinispan.org&gt; >> wrote: >> >>> There seems to be a single (global) reentrant lock which is >>> acquired >>> on any put; I don't know much about the module design, but there >>> is >>> a >>> comment close to the lock acquisition mentioning a need to flush >>> an >>> operations queue to avoid a deadlock. >>> Could it just make sure to reorder keys, so that deadlocks are >>> avoided? >>> >>> On 18 September 2012 16:26, Manik Surtani <manik(a)jboss.org&gt; >>> wrote: >>>> Agreed. >>>> >>>> This locking appears to be in the 2LC integration code to guard >>>> a >>>> local >>>> collection though - there must be better, non-blocking ways to >>>> guard this - >>>> if it is needed at all for RO entities. >>>> >>>> - M >>>> >>>> On 18 Sep 2012, at 15:18, Andrig Miller <anmiller(a)redhat.com&gt; >>>> wrote: >>>> >>>> What I find interesting, is that in this use case, READ_ONLY >>>> concurrency >>>> strategy and a local cache (no invalidation and no replication), >>>> there is no >>>> need to do any locking. >>>> >>>> If we can get to a solution without any locking that would be >>>> ideal. >>>> >>>> Andy >>>> >>>> ________________________________ >>>> >>>> From: "Manik Surtani" <manik(a)jboss.org&gt; >>>> To: "Ståle W. Pedersen" <spederse(a)redhat.com&gt; >>>> Cc: "Galder Zamarreño" <galder(a)redhat.com&gt;, "John O'Hara" >>>> <johara(a)redhat.com&gt;, "Jeremy Whiting" <jwhiting(a)redhat.com&gt;, >>>> "Andrig Miller" >>>> <anmiller(a)redhat.com&gt;, "Steve Ebersole" <steve(a)hibernate.org&gt;, >>>> "infinispan-dev" <infinispan-dev(a)lists.jboss.org&gt; >>>> Sent: Tuesday, September 18, 2012 8:13:43 AM >>>> Subject: Re: Issue with cache blocks for local read-only cache >>>> >>>> >>>> Looking at your profiler snapshot, these locks are in the >>>> Hibernate 2nd >>>> level cache implementation for Infinispan. Galder, any ideas? >>>> >>>> - M >>>> >>>> On 18 Sep 2012, at 13:43, Ståle W. Pedersen >>>> <spederse(a)redhat.com&gt; >>>> wrote: >>>> >>>> hi galder and manik, sorry for sending this mail to so many, but >>>> we've ran >>>> into a issue that prevents us from further scaling of the >>>> specjenterprise2010 benchmark. >>>> >>>> so when doing specjenterprise2010 benchmark testing we've seen a >>>> lot of >>>> blocks caused by the entity/query cache. we've been testing with >>>> only >>>> caching a simple entity bean that's read-only and queries >>>> related >>>> to this >>>> entity (selects). >>>> >>>> here is a screenshot of the hotspot >>>> found:https://dl.dropbox.com/u/30971563/specjent_block.png >>>> >>>> here is the standalone.xml: >>>> https://dl.dropbox.com/u/30971563/standalone-full.xml >>>> >>>> here is the orm.xml: >>>> https://dl.dropbox.com/u/30971563/order_orm.xml >>>> >>>> what we don't understand is why there are so many puts into the >>>> cache for an >>>> object that is marked as read-only. when we're testing without >>>> caching we do >>>> not see any blocks. >>>> >>>> any help/ideas would be great. if anyone want a jprofiler >>>> snapshot >>>> of the >>>> run, let me know. >>>> >>>> regards, ståle >>>> -- >>>> JBoss Performance Team Lead >>>> JBoss by Red Hat >>>> >>>> >>>> -- >>>> Manik Surtani >>>> manik(a)jboss.org >>>> twitter.com/maniksurtani >>>> >>>> Platform Architect, JBoss Data Grid >>>> http://red.ht/data-grid >>>> >>>> >>>> -- >>>> Manik Surtani >>>> manik(a)jboss.org >>>> twitter.com/maniksurtani >>>> >>>> Platform Architect, JBoss Data Grid >>>> http://red.ht/data-grid >>>> >>>> >>>> _______________________________________________ >>>> infinispan-dev mailing list >>>> infinispan-dev(a)lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>> >>> _______________________________________________ >>> infinispan-dev mailing list >>> infinispan-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >> >> >> -- >> Galder Zamarreño >> galder(a)redhat.com >> twitter.com/galderz >> >> Project Lead, Escalante >> http://escalante.io >> >> Engineer, Infinispan >> http://infinispan.org >> >> -- Galder Zamarreño galder(a)redhat.com twitter.com/galderz Project Lead, Escalante http://escalante.io Engineer, Infinispan http://infinispan.org

On 20 September 2012 17:38, Andrig Miller <anmiller(a)redhat.com&gt; wrote: > > > ----- Original Message ----- >> From: "Galder Zamarreño" <galder(a)redhat.com&gt; >> To: "Andrig Miller" <anmiller(a)redhat.com&gt; >> Cc: "Steve Ebersole" <steve(a)hibernate.org&gt;, "John O'Hara" <johara(a)redhat.com&gt;, "Jeremy Whiting" >> <jwhiting(a)redhat.com&gt;, "infinispan -Dev List" <infinispan-dev(a)lists.jboss.org&gt; >> Sent: Thursday, September 20, 2012 6:48:59 AM >> Subject: Re: [infinispan-dev] Issue with cache blocks for local read-only cache >> >> >> On Sep 19, 2012, at 4:20 PM, Andrig Miller <anmiller(a)redhat.com&gt; >> wrote: >> >>> Yes, I can see how that can happen, if the data is deleted from >>> outside the application. >> >> ^ The issue does not only happen if the data is deleted outside the >> application. As indicated in >> https://hibernate.onjira.com/browse/HHH-3817, this can happen with >> two competing transactions. >> >>> If you cache something as READ_ONLY, and it gets deleted, that >>> doesn't fit the definition of READ_ONLY though. You are using the >>> wrong cache concurrency strategy. >>> >>> Even that issue outlines the scenario where the collection is >>> updated, which means its not a READ_ONLY. >> >> I think the update is irrelevant here. The issue is related to >> putFromLoad + remove, which both AFAIK, are allowed in READ_ONLY >> (remember that we had the discussion on whether remove should be >> allowed in a READ_ONLY cache: >> https://hibernate.onjira.com/browse/HHH-7350). >> > > Yes, remove can be done, its just update that matters to READ_ONLY. One thing I thought about was I thought we were using MVCC for this stuff. Any transaction that reads from the cache, while something is being added/removed, should be reading the read consistent image, and should never wait on a lock, correct? We see all the threads in our thread pool sitting in a blocked state based on this locking.

> > That really shouldn't be happening with MVCC. This would be correct, if only Infinispan would support MVCC, but that's not the case today.

Sanne > > Andy > >>> >>> Andy >>> >>> ----- Original Message ----- >>>> From: "Galder Zamarreño" <galder(a)redhat.com&gt; >>>> To: "infinispan -Dev List" <infinispan-dev(a)lists.jboss.org&gt; >>>> Cc: "Steve Ebersole" <steve(a)hibernate.org&gt;, "John O'Hara" >>>> <johara(a)redhat.com&gt;, "Andrig Miller" <anmiller(a)redhat.com&gt;, >>>> "Jeremy Whiting" <jwhiting(a)redhat.com&gt; >>>> Sent: Wednesday, September 19, 2012 2:48:37 AM >>>> Subject: Re: [infinispan-dev] Issue with cache blocks for local >>>> read-only cache >>>> >>>> This is code written in JBoss Cache days to deal with situations >>>> where putFromLoad might try to store stale data (if data is >>>> deleted >>>> in between a database read and putFromLoad being called). >>>> >>>> Indeed this can happen with read only data, and has nothing to do >>>> with clustering. >>>> >>>> The original issue is: >>>> https://hibernate.onjira.com/browse/HHH-3817 >>>> >>>> I'll check how this can be improved. >>>> >>>> Cheers, >>>> >>>> On Sep 18, 2012, at 4:55 PM, Sanne Grinovero >>>> <sanne(a)infinispan.org&gt; >>>> wrote: >>>> >>>>> There seems to be a single (global) reentrant lock which is >>>>> acquired >>>>> on any put; I don't know much about the module design, but there >>>>> is >>>>> a >>>>> comment close to the lock acquisition mentioning a need to flush >>>>> an >>>>> operations queue to avoid a deadlock. >>>>> Could it just make sure to reorder keys, so that deadlocks are >>>>> avoided? >>>>> >>>>> On 18 September 2012 16:26, Manik Surtani <manik(a)jboss.org&gt; >>>>> wrote: >>>>>> Agreed. >>>>>> >>>>>> This locking appears to be in the 2LC integration code to guard >>>>>> a >>>>>> local >>>>>> collection though - there must be better, non-blocking ways to >>>>>> guard this - >>>>>> if it is needed at all for RO entities. >>>>>> >>>>>> - M >>>>>> >>>>>> On 18 Sep 2012, at 15:18, Andrig Miller <anmiller(a)redhat.com&gt; >>>>>> wrote: >>>>>> >>>>>> What I find interesting, is that in this use case, READ_ONLY >>>>>> concurrency >>>>>> strategy and a local cache (no invalidation and no replication), >>>>>> there is no >>>>>> need to do any locking. >>>>>> >>>>>> If we can get to a solution without any locking that would be >>>>>> ideal. >>>>>> >>>>>> Andy >>>>>> >>>>>> ________________________________ >>>>>> >>>>>> From: "Manik Surtani" <manik(a)jboss.org&gt; >>>>>> To: "Ståle W. Pedersen" <spederse(a)redhat.com&gt; >>>>>> Cc: "Galder Zamarreño" <galder(a)redhat.com&gt;, "John O'Hara" >>>>>> <johara(a)redhat.com&gt;, "Jeremy Whiting" <jwhiting(a)redhat.com&gt;, >>>>>> "Andrig Miller" >>>>>> <anmiller(a)redhat.com&gt;, "Steve Ebersole" <steve(a)hibernate.org&gt;, >>>>>> "infinispan-dev" <infinispan-dev(a)lists.jboss.org&gt; >>>>>> Sent: Tuesday, September 18, 2012 8:13:43 AM >>>>>> Subject: Re: Issue with cache blocks for local read-only cache >>>>>> >>>>>> >>>>>> Looking at your profiler snapshot, these locks are in the >>>>>> Hibernate 2nd >>>>>> level cache implementation for Infinispan. Galder, any ideas? >>>>>> >>>>>> - M >>>>>> >>>>>> On 18 Sep 2012, at 13:43, Ståle W. Pedersen >>>>>> <spederse(a)redhat.com&gt; >>>>>> wrote: >>>>>> >>>>>> hi galder and manik, sorry for sending this mail to so many, but >>>>>> we've ran >>>>>> into a issue that prevents us from further scaling of the >>>>>> specjenterprise2010 benchmark. >>>>>> >>>>>> so when doing specjenterprise2010 benchmark testing we've seen a >>>>>> lot of >>>>>> blocks caused by the entity/query cache. we've been testing with >>>>>> only >>>>>> caching a simple entity bean that's read-only and queries >>>>>> related >>>>>> to this >>>>>> entity (selects). >>>>>> >>>>>> here is a screenshot of the hotspot >>>>>> found:https://dl.dropbox.com/u/30971563/specjent_block.png >>>>>> >>>>>> here is the standalone.xml: >>>>>> https://dl.dropbox.com/u/30971563/standalone-full.xml >>>>>> >>>>>> here is the orm.xml: >>>>>> https://dl.dropbox.com/u/30971563/order_orm.xml >>>>>> >>>>>> what we don't understand is why there are so many puts into the >>>>>> cache for an >>>>>> object that is marked as read-only. when we're testing without >>>>>> caching we do >>>>>> not see any blocks. >>>>>> >>>>>> any help/ideas would be great. if anyone want a jprofiler >>>>>> snapshot >>>>>> of the >>>>>> run, let me know. >>>>>> >>>>>> regards, ståle >>>>>> -- >>>>>> JBoss Performance Team Lead >>>>>> JBoss by Red Hat >>>>>> >>>>>> >>>>>> -- >>>>>> Manik Surtani >>>>>> manik(a)jboss.org >>>>>> twitter.com/maniksurtani >>>>>> >>>>>> Platform Architect, JBoss Data Grid >>>>>> http://red.ht/data-grid >>>>>> >>>>>> >>>>>> -- >>>>>> Manik Surtani >>>>>> manik(a)jboss.org >>>>>> twitter.com/maniksurtani >>>>>> >>>>>> Platform Architect, JBoss Data Grid >>>>>> http://red.ht/data-grid >>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> infinispan-dev mailing list >>>>>> infinispan-dev(a)lists.jboss.org >>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>>>> >>>>> _______________________________________________ >>>>> infinispan-dev mailing list >>>>> infinispan-dev(a)lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>>> >>>> >>>> -- >>>> Galder Zamarreño >>>> galder(a)redhat.com >>>> twitter.com/galderz >>>> >>>> Project Lead, Escalante >>>> http://escalante.io >>>> >>>> Engineer, Infinispan >>>> http://infinispan.org >>>> >>>> >> >> >> -- >> Galder Zamarreño >> galder(a)redhat.com >> twitter.com/galderz >> >> Project Lead, Escalante >> http://escalante.io >> >> Engineer, Infinispan >> http://infinispan.org >> >> > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev

On 24 Sep 2012, at 11:01, Galder Zamarreño <galder(a)redhat.com&gt; wrote: > > On Sep 21, 2012, at 3:43 PM, Sanne Grinovero <sanne(a)infinispan.org&gt; wrote: > >> On 20 September 2012 17:38, Andrig Miller <anmiller(a)redhat.com&gt; wrote: >>> >>> >>> ----- Original Message ----- >>>> From: "Galder Zamarreño" <galder(a)redhat.com&gt; >>>> To: "Andrig Miller" <anmiller(a)redhat.com&gt; >>>> Cc: "Steve Ebersole" <steve(a)hibernate.org&gt;, "John O'Hara" <johara(a)redhat.com&gt;, "Jeremy Whiting" >>>> <jwhiting(a)redhat.com&gt;, "infinispan -Dev List" <infinispan-dev(a)lists.jboss.org&gt; >>>> Sent: Thursday, September 20, 2012 6:48:59 AM >>>> Subject: Re: [infinispan-dev] Issue with cache blocks for local read-only cache >>>> >>>> >>>> On Sep 19, 2012, at 4:20 PM, Andrig Miller <anmiller(a)redhat.com&gt; >>>> wrote: >>>> >>>>> Yes, I can see how that can happen, if the data is deleted from >>>>> outside the application. >>>> >>>> ^ The issue does not only happen if the data is deleted outside the >>>> application. As indicated in >>>> https://hibernate.onjira.com/browse/HHH-3817, this can happen with >>>> two competing transactions. >>>> >>>>> If you cache something as READ_ONLY, and it gets deleted, that >>>>> doesn't fit the definition of READ_ONLY though. You are using the >>>>> wrong cache concurrency strategy. >>>>> >>>>> Even that issue outlines the scenario where the collection is >>>>> updated, which means its not a READ_ONLY. >>>> >>>> I think the update is irrelevant here. The issue is related to >>>> putFromLoad + remove, which both AFAIK, are allowed in READ_ONLY >>>> (remember that we had the discussion on whether remove should be >>>> allowed in a READ_ONLY cache: >>>> https://hibernate.onjira.com/browse/HHH-7350). >>>> >>> >>> Yes, remove can be done, its just update that matters to READ_ONLY. One thing I thought about was I thought we were using MVCC for this stuff. Any transaction that reads from the cache, while something is being added/removed, should be reading the read consistent image, and should never wait on a lock, correct? We see all the threads in our thread pool sitting in a blocked state based on this locking. > > I'm not 100% sure which locking are you talking about, but if you're refering to the lock in https://dl.dropbox.com/u/30971563/specjent_block.png, that's related to the 2LC integration, not Infinispan itself. Yes, we're analysing the 2LC impl as well as Infinispan. > If you're talking about threads waiting for a lock somewhere else, please provide more details. > > I have some short-term ideas to improve the 2LC integration code, but I wanna check with Brian first. > > Long term, I think https://issues.jboss.org/browse/ISPN-506 will be necessary to provide a lock-free solution to these edge cases in such way that 'newer' removes cannot be overridden by 'old' putFromLoad calls. However, I'm intrigued by the fact that JBoss Cache OL had the capability of being given a version externally, but the 2LC code for JBoss Cache OL still used this PutFromLoadValidator logic. Again, something I need to check with Brian. ISPN-506 will only help in the clustered case.

And SpecJ - and most other use cases - are not clustered. Maybe we need two 2LC implementations, one for clustered and one for non-clustered use? There are a lot of unnecessary locks that can be removed in the local case. Sanne and I were having a chat about this yesterday. - M -- Manik Surtani manik(a)jboss.org twitter.com/maniksurtani Platform Architect, JBoss Data Grid http://red.ht/data-grid _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

On 25 Sep 2012, at 13:48, Galder Zamarreño <galder(a)redhat.com&gt; wrote: > > On Sep 24, 2012, at 12:27 PM, Manik Surtani <manik(a)jboss.org&gt; wrote: > >> >> On 24 Sep 2012, at 11:01, Galder Zamarreño <galder(a)redhat.com&gt; wrote: >> >>> >>> On Sep 21, 2012, at 3:43 PM, Sanne Grinovero <sanne(a)infinispan.org&gt; wrote: >>> >>>> On 20 September 2012 17:38, Andrig Miller <anmiller(a)redhat.com&gt; wrote: >>>>> >>>>> >>>>> ----- Original Message ----- >>>>>> From: "Galder Zamarreño" <galder(a)redhat.com&gt; >>>>>> To: "Andrig Miller" <anmiller(a)redhat.com&gt; >>>>>> Cc: "Steve Ebersole" <steve(a)hibernate.org&gt;, "John O'Hara" <johara(a)redhat.com&gt;, "Jeremy Whiting" >>>>>> <jwhiting(a)redhat.com&gt;, "infinispan -Dev List" <infinispan-dev(a)lists.jboss.org&gt; >>>>>> Sent: Thursday, September 20, 2012 6:48:59 AM >>>>>> Subject: Re: [infinispan-dev] Issue with cache blocks for local read-only cache >>>>>> >>>>>> >>>>>> On Sep 19, 2012, at 4:20 PM, Andrig Miller <anmiller(a)redhat.com&gt; >>>>>> wrote: >>>>>> >>>>>>> Yes, I can see how that can happen, if the data is deleted from >>>>>>> outside the application. >>>>>> >>>>>> ^ The issue does not only happen if the data is deleted outside the >>>>>> application. As indicated in >>>>>> https://hibernate.onjira.com/browse/HHH-3817, this can happen with >>>>>> two competing transactions. >>>>>> >>>>>>> If you cache something as READ_ONLY, and it gets deleted, that >>>>>>> doesn't fit the definition of READ_ONLY though. You are using the >>>>>>> wrong cache concurrency strategy. >>>>>>> >>>>>>> Even that issue outlines the scenario where the collection is >>>>>>> updated, which means its not a READ_ONLY. >>>>>> >>>>>> I think the update is irrelevant here. The issue is related to >>>>>> putFromLoad + remove, which both AFAIK, are allowed in READ_ONLY >>>>>> (remember that we had the discussion on whether remove should be >>>>>> allowed in a READ_ONLY cache: >>>>>> https://hibernate.onjira.com/browse/HHH-7350). >>>>>> >>>>> >>>>> Yes, remove can be done, its just update that matters to READ_ONLY. One thing I thought about was I thought we were using MVCC for this stuff. Any transaction that reads from the cache, while something is being added/removed, should be reading the read consistent image, and should never wait on a lock, correct? We see all the threads in our thread pool sitting in a blocked state based on this locking. >>> >>> I'm not 100% sure which locking are you talking about, but if you're refering to the lock in https://dl.dropbox.com/u/30971563/specjent_block.png, that's related to the 2LC integration, not Infinispan itself. >> >> Yes, we're analysing the 2LC impl as well as Infinispan. >> >>> If you're talking about threads waiting for a lock somewhere else, please provide more details. >>> >>> I have some short-term ideas to improve the 2LC integration code, but I wanna check with Brian first. >>> >>> Long term, I think https://issues.jboss.org/browse/ISPN-506 will be necessary to provide a lock-free solution to these edge cases in such way that 'newer' removes cannot be overridden by 'old' putFromLoad calls. However, I'm intrigued by the fact that JBoss Cache OL had the capability of being given a version externally, but the 2LC code for JBoss Cache OL still used this PutFromLoadValidator logic. Again, something I need to check with Brian. >> >> ISPN-506 will only help in the clustered case. > > ^ I disagree. It might help with the edge case highlighted in https://hibernate.onjira.com/browse/HHH-3817 which happens in a local cache. There is a much easier way to solve this: pessimistic locking + an eager cache.lock() command before retrieving the collection from the database. This will prevent the race defined in the HHH-3817.

-- Manik Surtani manik(a)jboss.org twitter.com/maniksurtani Platform Architect, JBoss Data Grid http://red.ht/data-grid _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

Not sure if it is pertinent here (sure seems so to my layman understanding), but what we have talked about for many years was caching (originally TreeCache) keeping a marker entry around when entries are removed from such cases.

On 25 September 2012 16:41, Galder Zamarreño <galder(a)redhat.com&gt; wrote: > > On Sep 25, 2012, at 4:51 PM, Manik Surtani <manik(a)jboss.org&gt; wrote: > >> >> On 25 Sep 2012, at 13:48, Galder Zamarreño <galder(a)redhat.com&gt; wrote: >> >>> >>> On Sep 24, 2012, at 12:27 PM, Manik Surtani <manik(a)jboss.org&gt; wrote: >>> >>>> >>>> On 24 Sep 2012, at 11:01, Galder Zamarreño <galder(a)redhat.com&gt; wrote: >>>> >>>>> >>>>> On Sep 21, 2012, at 3:43 PM, Sanne Grinovero <sanne(a)infinispan.org&gt; wrote: >>>>> >>>>>> On 20 September 2012 17:38, Andrig Miller <anmiller(a)redhat.com&gt; wrote: >>>>>>> >>>>>>> >>>>>>> ----- Original Message ----- >>>>>>>> From: "Galder Zamarreño" <galder(a)redhat.com&gt; >>>>>>>> To: "Andrig Miller" <anmiller(a)redhat.com&gt; >>>>>>>> Cc: "Steve Ebersole" <steve(a)hibernate.org&gt;, "John O'Hara" <johara(a)redhat.com&gt;, "Jeremy Whiting" >>>>>>>> <jwhiting(a)redhat.com&gt;, "infinispan -Dev List" <infinispan-dev(a)lists.jboss.org&gt; >>>>>>>> Sent: Thursday, September 20, 2012 6:48:59 AM >>>>>>>> Subject: Re: [infinispan-dev] Issue with cache blocks for local read-only cache >>>>>>>> >>>>>>>> >>>>>>>> On Sep 19, 2012, at 4:20 PM, Andrig Miller <anmiller(a)redhat.com&gt; >>>>>>>> wrote: >>>>>>>> >>>>>>>>> Yes, I can see how that can happen, if the data is deleted from >>>>>>>>> outside the application. >>>>>>>> >>>>>>>> ^ The issue does not only happen if the data is deleted outside the >>>>>>>> application. As indicated in >>>>>>>> https://hibernate.onjira.com/browse/HHH-3817, this can happen with >>>>>>>> two competing transactions. >>>>>>>> >>>>>>>>> If you cache something as READ_ONLY, and it gets deleted, that >>>>>>>>> doesn't fit the definition of READ_ONLY though. You are using the >>>>>>>>> wrong cache concurrency strategy. >>>>>>>>> >>>>>>>>> Even that issue outlines the scenario where the collection is >>>>>>>>> updated, which means its not a READ_ONLY. >>>>>>>> >>>>>>>> I think the update is irrelevant here. The issue is related to >>>>>>>> putFromLoad + remove, which both AFAIK, are allowed in READ_ONLY >>>>>>>> (remember that we had the discussion on whether remove should be >>>>>>>> allowed in a READ_ONLY cache: >>>>>>>> https://hibernate.onjira.com/browse/HHH-7350). >>>>>>>> >>>>>>> >>>>>>> Yes, remove can be done, its just update that matters to READ_ONLY. One thing I thought about was I thought we were using MVCC for this stuff. Any transaction that reads from the cache, while something is being added/removed, should be reading the read consistent image, and should never wait on a lock, correct? We see all the threads in our thread pool sitting in a blocked state based on this locking. >>>>> >>>>> I'm not 100% sure which locking are you talking about, but if you're refering to the lock in https://dl.dropbox.com/u/30971563/specjent_block.png, that's related to the 2LC integration, not Infinispan itself. >>>> >>>> Yes, we're analysing the 2LC impl as well as Infinispan. >>>> >>>>> If you're talking about threads waiting for a lock somewhere else, please provide more details. >>>>> >>>>> I have some short-term ideas to improve the 2LC integration code, but I wanna check with Brian first. >>>>> >>>>> Long term, I think https://issues.jboss.org/browse/ISPN-506 will be necessary to provide a lock-free solution to these edge cases in such way that 'newer' removes cannot be overridden by 'old' putFromLoad calls. However, I'm intrigued by the fact that JBoss Cache OL had the capability of being given a version externally, but the 2LC code for JBoss Cache OL still used this PutFromLoadValidator logic. Again, something I need to check with Brian. >>>> >>>> ISPN-506 will only help in the clustered case. >>> >>> ^ I disagree. It might help with the edge case highlighted in https://hibernate.onjira.com/browse/HHH-3817 which happens in a local cache. >> >> There is a much easier way to solve this: pessimistic locking + an eager cache.lock() command before retrieving the collection from the database. This will prevent the race defined in the HHH-3817. > > Hmmm, I'm not so sure about that. It's an interesting idea but let me explain what PFLV does so that the rest understand: > > What PutFromLoadValidator does is the following: if cache.get() misses, it assumes there might a putFromLoad() coming. This put might never come (i.e. if the database has no data, but this will eventually be cleaned up), but it offers up a barrier in case a removal comes before the actual putFromLoad(). The effect is that if the removal comes before putFromLoad(), it will invalidate the put and when putFromLoad() comes, it won't be able to store stale things in memory. If the removal comes after putFromLoad(), then not a problem. Is this limited to removal of entries only? Looks like a similar pattern could happen with any update. > So, I see some problems with lock: > > 1. Can you lock() a non-existing key? (quickly browsed the code and couldn't say for certain…). Even if you can, 2 consecutive lock() calls for the same key from different threads would result in 2nd thread blocking. Definitely not good for a get() op. Good point, I hope we can lock() a non-existing key as that's useful for many other cases; shouldn't be hard since we decoupled the lock from the entries for the single-lock owner design.

Anyway, this is a lock which is going to operate on a specific key. Definitely better than the global lock we're experiencing today!

The comments in the code seem to remind a per-key lock isn't being used as a deadlock could happen; can't we just reorder the keys to be locked? > 2. How do you unlock the key if after a while there has not been a putFromLoad()? We have no unlock() call. I wouldn't lock it but put a marker in it, you could even store a version in your marker.. the marker will eventually be evicted if not useful.

> > 3. This is a solution that still requires locking. A lock-free solution, where we can capture the version when an entry is going to be deleted (via lockItem() implementation in 2LC), and we can compare it with the one from the putFromLoad() would surely be performant IMO. Btw, I've assigned https://issues.jboss.org/browse/ISPN-506 to myself in order to investigate this and move towards such a solution. > > Cheers, > >> >> -- >> Manik Surtani >> manik(a)jboss.org >> twitter.com/maniksurtani >> >> Platform Architect, JBoss Data Grid >> http://red.ht/data-grid >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > > -- > Galder Zamarreño > galder(a)redhat.com > twitter.com/galderz > > Project Lead, Escalante > http://escalante.io > > Engineer, Infinispan > http://infinispan.org > > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

On Sep 25, 2012, at 6:07 PM, Sanne Grinovero <sanne(a)infinispan.org&gt; wrote: > On 25 September 2012 16:41, Galder Zamarreño <galder(a)redhat.com&gt; wrote: >> On Sep 25, 2012, at 4:51 PM, Manik Surtani <manik(a)jboss.org&gt; wrote: >> >>> On 25 Sep 2012, at 13:48, Galder Zamarreño <galder(a)redhat.com&gt; wrote: >>> >>>> On Sep 24, 2012, at 12:27 PM, Manik Surtani <manik(a)jboss.org&gt; wrote: >>>> >>>>> On 24 Sep 2012, at 11:01, Galder Zamarreño <galder(a)redhat.com&gt; wrote: >>>>> >>>>>> On Sep 21, 2012, at 3:43 PM, Sanne Grinovero <sanne(a)infinispan.org&gt; wrote: >>>>>> >>>>>>> On 20 September 2012 17:38, Andrig Miller <anmiller(a)redhat.com&gt; wrote: >>>>>>>> >>>>>>>> ----- Original Message ----- >>>>>>>>> From: "Galder Zamarreño" <galder(a)redhat.com&gt; >>>>>>>>> To: "Andrig Miller" <anmiller(a)redhat.com&gt; >>>>>>>>> Cc: "Steve Ebersole" <steve(a)hibernate.org&gt;, "John O'Hara" <johara(a)redhat.com&gt;, "Jeremy Whiting" >>>>>>>>> <jwhiting(a)redhat.com&gt;, "infinispan -Dev List" <infinispan-dev(a)lists.jboss.org&gt; >>>>>>>>> Sent: Thursday, September 20, 2012 6:48:59 AM >>>>>>>>> Subject: Re: [infinispan-dev] Issue with cache blocks for local read-only cache >>>>>>>>> >>>>>>>>> >>>>>>>>> On Sep 19, 2012, at 4:20 PM, Andrig Miller <anmiller(a)redhat.com&gt; >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> Yes, I can see how that can happen, if the data is deleted from >>>>>>>>>> outside the application. >>>>>>>>> ^ The issue does not only happen if the data is deleted outside the >>>>>>>>> application. As indicated in >>>>>>>>> https://hibernate.onjira.com/browse/HHH-3817, this can happen with >>>>>>>>> two competing transactions. >>>>>>>>> >>>>>>>>>> If you cache something as READ_ONLY, and it gets deleted, that >>>>>>>>>> doesn't fit the definition of READ_ONLY though. You are using the >>>>>>>>>> wrong cache concurrency strategy. >>>>>>>>>> >>>>>>>>>> Even that issue outlines the scenario where the collection is >>>>>>>>>> updated, which means its not a READ_ONLY. >>>>>>>>> I think the update is irrelevant here. The issue is related to >>>>>>>>> putFromLoad + remove, which both AFAIK, are allowed in READ_ONLY >>>>>>>>> (remember that we had the discussion on whether remove should be >>>>>>>>> allowed in a READ_ONLY cache: >>>>>>>>> https://hibernate.onjira.com/browse/HHH-7350). >>>>>>>>> >>>>>>>> Yes, remove can be done, its just update that matters to READ_ONLY. One thing I thought about was I thought we were using MVCC for this stuff. Any transaction that reads from the cache, while something is being added/removed, should be reading the read consistent image, and should never wait on a lock, correct? We see all the threads in our thread pool sitting in a blocked state based on this locking. >>>>>> I'm not 100% sure which locking are you talking about, but if you're refering to the lock in https://dl.dropbox.com/u/30971563/specjent_block.png, that's related to the 2LC integration, not Infinispan itself. >>>>> Yes, we're analysing the 2LC impl as well as Infinispan. >>>>> >>>>>> If you're talking about threads waiting for a lock somewhere else, please provide more details. >>>>>> >>>>>> I have some short-term ideas to improve the 2LC integration code, but I wanna check with Brian first. >>>>>> >>>>>> Long term, I think https://issues.jboss.org/browse/ISPN-506 will be necessary to provide a lock-free solution to these edge cases in such way that 'newer' removes cannot be overridden by 'old' putFromLoad calls. However, I'm intrigued by the fact that JBoss Cache OL had the capability of being given a version externally, but the 2LC code for JBoss Cache OL still used this PutFromLoadValidator logic. Again, something I need to check with Brian. >>>>> ISPN-506 will only help in the clustered case. >>>> ^ I disagree. It might help with the edge case highlighted in https://hibernate.onjira.com/browse/HHH-3817 which happens in a local cache. >>> There is a much easier way to solve this: pessimistic locking + an eager cache.lock() command before retrieving the collection from the database. This will prevent the race defined in the HHH-3817. >> Hmmm, I'm not so sure about that. It's an interesting idea but let me explain what PFLV does so that the rest understand: >> >> What PutFromLoadValidator does is the following: if cache.get() misses, it assumes there might a putFromLoad() coming. This put might never come (i.e. if the database has no data, but this will eventually be cleaned up), but it offers up a barrier in case a removal comes before the actual putFromLoad(). The effect is that if the removal comes before putFromLoad(), it will invalidate the put and when putFromLoad() comes, it won't be able to store stale things in memory. If the removal comes after putFromLoad(), then not a problem. > Is this limited to removal of entries only? Looks like a similar > pattern could happen with any update. No, it's not limited to puts. Of course updates could cause the same effect. >> So, I see some problems with lock: >> >> 1. Can you lock() a non-existing key? (quickly browsed the code and couldn't say for certain…). Even if you can, 2 consecutive lock() calls for the same key from different threads would result in 2nd thread blocking. Definitely not good for a get() op. > Good point, I hope we can lock() a non-existing key as that's useful > for many other cases; shouldn't be hard since we decoupled the lock > from the entries for the single-lock owner design. > > Anyway, this is a lock which is going to operate on a specific key. > Definitely better than the global lock we're experiencing today! +1. I've got a patch to remove the current contention point which I don't think it's necessary. And I have other ideas to avoid a global lock at all, such as using some sort of concurrent queue for both pending queues and transfer from one to the other, but I'm waiting to see first what Andy/Stale show in testing the patch I sent them first. > The comments in the code seem to remind a per-key lock isn't being > used as a deadlock could happen; can't we just reorder the keys to be > locked? Hmmm, not sure, I'll investigate this... >> 2. How do you unlock the key if after a while there has not been a putFromLoad()? We have no unlock() call. > I wouldn't lock it but put a marker in it, you could even store a > version in your marker.. the marker will eventually be evicted if not useful. This is not far off from what PFLV does, except that you use the cache as the vehicle for it. I'm making a note to try it... >> 3. This is a solution that still requires locking. A lock-free solution, where we can capture the version when an entry is going to be deleted (via lockItem() implementation in 2LC), and we can compare it with the one from the putFromLoad() would surely be performant IMO. Btw, I've assigned https://issues.jboss.org/browse/ISPN-506 to myself in order to investigate this and move towards such a solution. >> >> Cheers, >> >>> -- >>> Manik Surtani >>> manik(a)jboss.org >>> twitter.com/maniksurtani >>> >>> Platform Architect, JBoss Data Grid >>> http://red.ht/data-grid >>> >>> _______________________________________________ >>> infinispan-dev mailing list >>> infinispan-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >> >> -- >> Galder Zamarreño >> galder(a)redhat.com >> twitter.com/galderz >> >> Project Lead, Escalante >> http://escalante.io >> >> Engineer, Infinispan >> http://infinispan.org >> >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev -- Galder Zamarreño galder(a)redhat.com twitter.com/galderz Project Lead, Escalante http://escalante.io Engineer, Infinispan http://infinispan.org

On Sep 21, 2012, at 3:43 PM, Sanne Grinovero <sanne(a)infinispan.org&gt; wrote: > On 20 September 2012 17:38, Andrig Miller <anmiller(a)redhat.com&gt; wrote: >> >> >> ----- Original Message ----- >>> From: "Galder Zamarreño" <galder(a)redhat.com&gt; >>> To: "Andrig Miller" <anmiller(a)redhat.com&gt; >>> Cc: "Steve Ebersole" <steve(a)hibernate.org&gt;, "John O'Hara" <johara(a)redhat.com&gt;, "Jeremy Whiting" >>> <jwhiting(a)redhat.com&gt;, "infinispan -Dev List" <infinispan-dev(a)lists.jboss.org&gt; >>> Sent: Thursday, September 20, 2012 6:48:59 AM >>> Subject: Re: [infinispan-dev] Issue with cache blocks for local read-only cache >>> >>> >>> On Sep 19, 2012, at 4:20 PM, Andrig Miller <anmiller(a)redhat.com&gt; >>> wrote: >>> >>>> Yes, I can see how that can happen, if the data is deleted from >>>> outside the application. >>> >>> ^ The issue does not only happen if the data is deleted outside the >>> application. As indicated in >>> https://hibernate.onjira.com/browse/HHH-3817, this can happen with >>> two competing transactions. >>> >>>> If you cache something as READ_ONLY, and it gets deleted, that >>>> doesn't fit the definition of READ_ONLY though. You are using the >>>> wrong cache concurrency strategy. >>>> >>>> Even that issue outlines the scenario where the collection is >>>> updated, which means its not a READ_ONLY. >>> >>> I think the update is irrelevant here. The issue is related to >>> putFromLoad + remove, which both AFAIK, are allowed in READ_ONLY >>> (remember that we had the discussion on whether remove should be >>> allowed in a READ_ONLY cache: >>> https://hibernate.onjira.com/browse/HHH-7350). >>> >> >> Yes, remove can be done, its just update that matters to READ_ONLY. One thing I thought about was I thought we were using MVCC for this stuff. Any transaction that reads from the cache, while something is being added/removed, should be reading the read consistent image, and should never wait on a lock, correct? We see all the threads in our thread pool sitting in a blocked state based on this locking. I'm not 100% sure which locking are you talking about, but if you're refering to the lock in https://dl.dropbox.com/u/30971563/specjent_block.png, that's related to the 2LC integration, not Infinispan itself. If you're talking about threads waiting for a lock somewhere else, please provide more details. I have some short-term ideas to improve the 2LC integration code, but I wanna check with Brian first. Long term, I think https://issues.jboss.org/browse/ISPN-506 will be necessary to provide a lock-free solution to these edge cases in such way that 'newer' removes cannot be overridden by 'old' putFromLoad calls. However, I'm intrigued by the fact that JBoss Cache OL had the capability of being given a version externally, but the 2LC code for JBoss Cache OL still used this PutFromLoadValidator logic. Again, something I need to check with Brian. >> >> That really shouldn't be happening with MVCC. > > This would be correct, if only Infinispan would support MVCC, but > that's not the case today. Sanne, don't understand why you say that. In Infinispan, we make copies when writing so that existing transactions can carry on working with a read value without problems and no locking.