On 25 September 2012 16:41, Galder Zamarreño <gal...@redhat.com> wrote: > > On Sep 25, 2012, at 4:51 PM, Manik Surtani <ma...@jboss.org> wrote: > >> >> On 25 Sep 2012, at 13:48, Galder Zamarreño <gal...@redhat.com> wrote: >> >>> >>> On Sep 24, 2012, at 12:27 PM, Manik Surtani <ma...@jboss.org> wrote: >>> >>>> >>>> On 24 Sep 2012, at 11:01, Galder Zamarreño <gal...@redhat.com> wrote: >>>> >>>>> >>>>> On Sep 21, 2012, at 3:43 PM, Sanne Grinovero <sa...@infinispan.org> wrote: >>>>> >>>>>> On 20 September 2012 17:38, Andrig Miller <anmil...@redhat.com> wrote: >>>>>>> >>>>>>> >>>>>>> ----- Original Message ----- >>>>>>>> From: "Galder Zamarreño" <gal...@redhat.com> >>>>>>>> To: "Andrig Miller" <anmil...@redhat.com> >>>>>>>> Cc: "Steve Ebersole" <st...@hibernate.org>, "John O'Hara" >>>>>>>> <joh...@redhat.com>, "Jeremy Whiting" >>>>>>>> <jwhit...@redhat.com>, "infinispan -Dev List" >>>>>>>> <infinispan-dev@lists.jboss.org> >>>>>>>> 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 <anmil...@redhat.com> >>>>>>>> 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 >> ma...@jboss.org >> twitter.com/maniksurtani >> >> Platform Architect, JBoss Data Grid >> http://red.ht/data-grid >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev@lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > > -- > Galder Zamarreño > gal...@redhat.com > twitter.com/galderz > > Project Lead, Escalante > http://escalante.io > > Engineer, Infinispan > http://infinispan.org > > > _______________________________________________ > 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