[infinispan-dev] Passivation and BCHM WAS: fixing eviction with transactions (critical for Hibernate Search)

[Manik Surtani]

Anyway, to summarise, does this cover everything?

1)  Passivate before evict, as described below
2)  AsyncStore checks async queue before delegating to actual cache store on load

Cheers
Manik

On 10 Jun 2011, at 14:18, Sanne Grinovero wrote:

> It looks as a good improvement, especially as we remove the locking,
> but I still have some doubts:
> 
> 1) as the passivation happens in the eviction thread, is that going to
> be fast enough or hindering the eviction performance? especially with
> LIRS, being this the user thread it means that put operations suffer a
> mayor performance hit (for async caheloaders, see next point). Well
> this was the case already, I guess.
> 
> 2) with async stores, the issue is back. The value is not going to be
> found while it's "in flight" in the async storage queue; so unless you
> want to avoid supporting it for now, the actual removal from the
> container should be performed from the async cache loader code.
> 
> So it looks like that - when passivation is enabled - the proper way
> to implement this is to use the eviction policy only to identify
> what's going to be removed from the container, but not to actually
> remove it.
> 
> I'd still focus on having the best performance possible for the case
> in which passivation is not enabled, that looks like the "performance
> needing" setup, or maybe we need separate eviction strategies fro when
> passivation is enabled.
> 
> 
> 3) so every time we evict an entry, we write it to the store.. I guess
> that leads to re-write quite often values which are already in the
> store, and for which there's no need to store again. I think this
> argument came up before, but we didn't want to track an additional
> boolean to keep the memory need low. Still when we're using it as a
> cache with eviction rules in place, it seems to me that overall
> performance would be better by avoiding more writes at the cost of
> being able to store some less entries in memory.
> 
> Cheers.
> Sanne
> 
> 2011/6/10 Manik Surtani <manik at jboss.org>:
>> So I've taken on this sub-task from Sanne (ISPN-1169), and here is the deal:
>> BCHM analyses whether or not to evict stuff every time an operation (put,
>> get, replace) is run, based on the configured eviction policy (LIRS, LRU or
>> NONE).  It does so by calling evictionStrategy.execute(), which returns a
>> Set of hash entries removed from the BCHM.
>> https://github.com/infinispan/infinispan/blob/master/core/src/main/java/org/infinispan/util/concurrent/BoundedConcurrentHashMap.java#L341
>> Note that execute() actually removes entries from the BCHM, so after
>> execute() is called, these entries are no longer in the BCHM!
>> This is set is then sent to the EvictionManager
>> https://github.com/infinispan/infinispan/blob/master/core/src/main/java/org/infinispan/eviction/EvictionManager.java#L61
>> which in turn passivates the evicted entries:
>> https://github.com/infinispan/infinispan/blob/master/core/src/main/java/org/infinispan/eviction/EvictionManagerImpl.java#L189
>> So we have a window where the entry is neither accessible from memory or
>> from a cache store, since it is "in-fly".
>> My proposed fix is to:
>> 1) Make sure the PassivationManager is passed in to the DataContainer, and
>> available to any EvictionStrategy
>> 2) EvictionStrategies ensure passivation occurs *before* removing an entry
>> from the BCHM
>> 3) The additional window of a reader thread in the ActivationInterceptor
>> determining that an entry is in memory and bypasses activation, only to find
>> that the entry has been removed from the BCHM by the time it gets to it,
>> should not be a problem since when testing the DataContainer in the
>> ActivationInterceptor, the entry is placed in the caller's InvocationContext
>> so a handle on the entry is there.
>> Does anyone have any concerns with this?  Vladimir, does this adversely
>> affect the BCHM/EvictionStrategy design?
>> Cheers
>> Manik
>> On 8 Jun 2011, at 19:15, Vladimir Blagojevic wrote:
>> 
>> On 11-06-08 12:10 PM, Sanne Grinovero wrote:
>> 
>> Yes, we're on the same channel now. That's exactly what I meant
>> 
>> yesterday on IRC when mentioning the BoundedCHM, and what I meant
>> 
>> above with "we should acquire it before it's actually evicted".
>> 
>> But this issue is getting complex, I've split it in 4 different
>> 
>> problems and I'm going to solve the first three soon, the BoundedCHM
>> 
>> is the fourth and I'm not sure I'll fix that today.
>> 
>> Sanne,
>> 
>> Sounds good, please consult us when it comes to changing BCHM!
>> 
>> Cheers.
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> 
>> --
>> Manik Surtani
>> manik at jboss.org
>> twitter.com/maniksurtani
>> Lead, Infinispan
>> http://www.infinispan.org
>> 
>> 
>> 
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> 
> 
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Manik Surtani
manik at jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org