[infinispan-dev] Today's topic: eviction
Galder Zamarreño
galder at redhat.com
Wed Nov 27 03:15:16 EST 2013
On Nov 22, 2013, at 12:31 PM, Pedro Ruivo <pedro at infinispan.org> wrote:
>
>
> 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 :)
+1
>
> 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…
We need a BoundedEquivalentConcurrentHashMapV8. The reason we haven't built one yet is because ConcurrentHashMapV8 has been in flux over past few months, so keeping it up to date required some effort. As it gets more stable, we can start looking into a BoundedEquivalentConcurrentHashMapV8. This might be a good time to do it.
>
>>
>> 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
>>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
galder at redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org
More information about the infinispan-dev
mailing list