[infinispan-dev] Today's topic: eviction

Dan Berindei dan.berindei at gmail.com
Wed Nov 27 04:09:18 EST 2013 

On Wed, Nov 27, 2013 at 10:15 AM, Galder Zamarreño <galder at redhat.com>wrote:

>
> 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.
>
>
+1 to write a BoundedEquivalentConcurrentHashMapV8, but changing the LRU
and LIRS implementations is going to be tricky business, so I'd rather we
do it separately from the DataContainer API change.


> >
> >>
> >> 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
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/infinispan-dev/attachments/20131127/cbdf147c/attachment-0001.html

More information about the infinispan-dev mailing list