[infinispan-dev] Today's topic: eviction

[Pedro Ruivo]

On 11/21/2013 10:08 PM, Sanne Grinovero wrote:
> Hi Pedro,
> great analysis.
> Conceptually I agree, and since I'm not too familiar with the core
> code, I actually expected this to be the case as we discussed the
> approach when we first discussed the single-lock-owner pattern.
>
> More specifically on the DataContainer: I don't think you should lock
> the segment, you need to lock the key entry only as the CacheStore
> could be quite slow so that is a "long" lock at least relatively to
> normal usage.

Agree with you. I based my solution on the current 
BoundConcurrentHashMap [1] implementation that uses locks per segment. 
Also, the eviction is kicked inside this locks so we already have a 
"long" lock to write the entry in the cache writer.

[1] Segment.put(): 
https://github.com/infinispan/infinispan/blob/master/core/src/main/java/org/infinispan/util/concurrent/BoundedConcurrentHashMap.java#L1554

>
> This should NOT be the normal key lock however, as you might remember
> I'm since long time advocating that there needs to be a split in
> logical "lock" as the user sees it (and as used by transactions) vs
> the locks we use internally to keep our very own data structure
> consistency.
>
> It's trivial to apply if you use
>
>     EquivalentConcurrentHashMapV8.computeIfAbsent(K, Fun<? super K, ? extends V>)

This is a great idea :)

The EquivalentConcurrentHashMapV8 will solve the problem for unbounded 
data container but we still have to do more work with the bounded data 
container I'm afraid...

>
> Doesn't sound like a patch which requires a major release?
>
> Sanne
>
>
>
>
> On 21 November 2013 18:48, Pedro Ruivo <pedro at infinispan.org> wrote:
>> (re: https://issues.jboss.org/browse/ISPN-3048)
>> (ps. sorry for the long mail. I hope that is clear)
>>
>> * Automatic eviction:
>>
>> I've been writing some scenario involving eviction and concurrent
>> operations. This test shows a very reliable eviction but we have a
>> problem when passivation is enabled. The passivation phase is ok but
>> when we need to activate a key, we have a huge window in which the
>> read/write operation does not find the key neither in-memory data
>> container neither in the cache loader. This happens because the check
>> in-memory data container is not synchronized with the check in cache loader.
>>
>> Passivation phase works fine, as I said, because it happens inside the
>> data container, i.e., the bounded concurrent hash map (BCHM) evict the
>> key under the segment lock.
>>
>> * Manual eviction
>>
>> I haven't finish my test suite but so far it does not work as expected.
>> If you are a backup owner, you are unable to evict any keys (because the
>> EvictCommand is handled as a RemoveCommand and the key is not wrapped in
>> EntryWrappingInterceptor). In addition, I believe that it has the same
>> problems with activation as above.
>>
>> Also, I believe that passivation phase does not work well because it
>> first tries to passivate than it removes from DataContainer. Nothing is
>> preventing to passivate an old value, a put occurs and modifies the data
>> in DataContainer and finally the EvictCommand removes the new value
>> (that is lost).
>>
>> * New implementation
>>
>> Since we are stating a new major release, I would like to "propose" the
>> following improvements. One aspect of the implementation is the dropping
>> of the *Cache[Writer or Loader]Interceptor and the EvictCommand. Also,
>> it would add a new method in DataContainer (void evict(K key)) and
>> possible change the interface in PersistenceManager.
>>
>> My idea is based on the current implementation of the BCHM. The BCHM
>> implementation is aware of the persistence and it performs internally
>> the passivate() and activate() operations. I would like to extend this
>> and make it full aware of the PersistenceManager. Also, it would be
>> required to have ConcurrentHashMap (CHM) with the same features
>>
>> Enter in more detail, the (B)CHM.get() will be responsible to load the
>> key from persistence (and activate it) if it is not found in-memory.
>> Also, it stores the entry in memory. In other hand, the put() stores the
>> entry in-memory and in the persistence (if passivation==false) and we
>> add an evict(k) to the CHM interface to remove from memory and passivate
>> it to persistence a single key.
>>
>> * Pros
>>
>> ** solves all the problems :)
>> ** remove all the interceptors related to cache writer and cache loader
>> => small interceptor chain
>> ** remove evict command => lower process time(?)
>> ** cache writer and cache loader does not need to have per key lock
>> based (however, they should be thread safe)
>> ** when passivation==true, we don't loose any key/value (yey)
>> ** no need to acquire locks in lock manager (as the current manual
>> eviction does)
>> ** all the logic for cache loader/writer will be located in a single mode
>>
>> * Cons
>>
>> ** difficult to maintain. We have to maintain the code for the (B)CHM
>> ** new API and contract for DataContainer interface => it should handle
>> all the load/write from cache loader/writer
>> ** new API for PersistenceManager
>>
>> toughs?
>>
>> Cheers
>> Pedro
>>
>> * Open question in case this is going forward
>>
>> ** should contains(key) put the entry loaded in-memory (if loaded from
>> cache loader)?
>> ** how iterator(), keys(), values() and entrySet() should behave? should
>> they put the entries in memory? And size()?
>>
>> * API changes
>>
>> DataContainer
>>
>> (new) void evict(K key)
>>
>> PersistenceManager
>>
>> (new) Entry onRead(K key) //shoud read the key. if passivation, remove
>> from loader
>> (new) void onWrite(K key, Entry entry) //if passivation, then do
>> nothing, else store it
>> (new) Entry onRemove(K key) //remove and return old
>> (new) void onEvict(K key, Entry entry) //if passivation, then store it
>>
>> * Some (B)CHM pseudo-code
>>
>> V get(K key)
>>     Segment s = segment(key)
>>     V value = s.get(key)
>>     if (value == null) {
>>       s.lock()
>>       //recheck if s.get(key) is still null
>>       value = persistenceManager.onLoad(key); //this activates the key if
>> needed
>>       if (value != null) s.put(key, value)
>>       s.unlock()
>>     }
>>     return value
>>
>> V put(K key, V value)
>>     Segment s = segment(key)
>>     s.lock()
>>     V oldValue = s.get(key)
>>     persistenceManager.onWrite(key, value) //first try the persistence
>>     s.put(key, value) //can trigger eviction
>>     s.unlock()
>>
>> void evict(K key, V value)
>>     Segment s = segment(key)
>>     s.lock()
>>     persistenceManager.onEvict(key, value) //first try the persistence
>>     s.remove(key) //remove from memory if peristence is successful (we
>> don't wanna loose the value, right?)
>>     s.unlock()
>> _______________________________________________
>> 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
>