Galder, Stale is not well today. When did you send the patch through?
If you could point me towards the patch I can get some stats through today Thanks John On 09/27/2012 12:17 PM, Galder Zamarreño wrote: > On Sep 25, 2012, at 6:07 PM, 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. > 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 >>>> [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 > > -- > Galder Zamarreño > [email protected] > twitter.com/galderz > > Project Lead, Escalante > http://escalante.io > > Engineer, Infinispan > http://infinispan.org > -- John O'Hara [email protected] JBoss, by Red Hat Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United Kingdom. Registered in UK and Wales under Company Registration No. 3798903 Directors: Michael Cunningham (USA), Charlie Peters (USA), Matt Parsons (USA) and Brendan Lane (Ireland). _______________________________________________ infinispan-dev mailing list [email protected] https://lists.jboss.org/mailman/listinfo/infinispan-dev
