On 25 Sep 2012, at 17:07, Sanne Grinovero <[email protected]> wrote:
> On 25 September 2012 16:41, Galder Zamarreño <[email protected]> wrote: >> >> On Sep 25, 2012, at 4:51 PM, Manik Surtani <[email protected]> wrote: >> >>> >>> On 25 Sep 2012, at 13:48, Galder Zamarreño <[email protected]> wrote: >>> >>>> >>>> On Sep 24, 2012, at 12:27 PM, Manik Surtani <[email protected]> wrote: >>>> >>>>> >>>>> On 24 Sep 2012, at 11:01, Galder Zamarreño <[email protected]> wrote: >>>>> >>>>>> >>>>>> On Sep 21, 2012, at 3:43 PM, Sanne Grinovero <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> On 20 September 2012 17:38, Andrig Miller <[email protected]> wrote: >>>>>>>> >>>>>>>> >>>>>>>> ----- Original Message ----- >>>>>>>>> From: "Galder Zamarreño" <[email protected]> >>>>>>>>> To: "Andrig Miller" <[email protected]> >>>>>>>>> Cc: "Steve Ebersole" <[email protected]>, "John O'Hara" >>>>>>>>> <[email protected]>, "Jeremy Whiting" >>>>>>>>> <[email protected]>, "infinispan -Dev List" >>>>>>>>> <[email protected]> >>>>>>>>> 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 <[email protected]> >>>>>>>>> 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. Yes we can. The lock is held on the key. > Anyway, this is a lock which is going to operate on a specific key. > Definitely better than the global lock we're experiencing today! Yep. > 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. … and the lock is released when your tx completes (successfully or not). > >> >> 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 >>> [email protected] >>> twitter.com/maniksurtani >>> >>> Platform Architect, JBoss Data Grid >>> http://red.ht/data-grid >>> >>> _______________________________________________ >>> infinispan-dev mailing list >>> [email protected] >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >> >> >> -- >> Galder Zamarreño >> [email protected] >> twitter.com/galderz >> >> Project Lead, Escalante >> http://escalante.io >> >> Engineer, Infinispan >> http://infinispan.org >> >> >> _______________________________________________ >> infinispan-dev mailing list >> [email protected] >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > _______________________________________________ > infinispan-dev mailing list > [email protected] > https://lists.jboss.org/mailman/listinfo/infinispan-dev -- Manik Surtani [email protected] twitter.com/maniksurtani Platform Architect, JBoss Data Grid http://red.ht/data-grid
_______________________________________________ infinispan-dev mailing list [email protected] https://lists.jboss.org/mailman/listinfo/infinispan-dev
