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

[Sanne Grinovero]

2011/6/10 Manik Surtani <manik at jboss.org>:
> 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

Yes, looks good! 2) won't be easy, AFAIR the AsyncStore has several
regions / stages where the data might be. I'd say this is a second
issue we can work on later.
Sanne



>
> 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
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>