[infinispan-dev] [ISPN-116] Async cache store: aggregation of multiple changes on a single key

[Manik Surtani]

Nope, never got the email below.

Anyway, I always thought the intention was to enhance or improve the  
AsyncCacheStore, not to create another one.

Cheers
Manik

On 20 Jul 2009, at 10:55, Galder Zamarreno wrote:

> Did other list readers ever got this message?
>
> I did send it last week but never actually got it via the dev list, so
> double checking...
>
> On 07/15/2009 07:58 PM, Galder Zamarreno wrote:
>> Hi,
>>
>> As part of this JIRA, I'm thinking whether we wanna leave the current
>> queue based async store.
>>
>> For the work I've done, I've created a new class called
>> CoalescedAsyncStore that works in the way agreed below, iow,  
>> instead of
>> queying mods, it keeps a ConcurrentMap and which is swapped by a new
>> instance when the async thread is going to apply the changes.
>>
>> I've run some perf tests between CoalescedAsyncStore and AsyncStore  
>> and
>> when doing 1 million store where each key is different, the  
>> performance
>> is fairly similar. The difference comes when trying to run a similar
>> test where the same key is always updated, this results in lesser
>> underlying store calls and hence CoalescedAsyncStore solution is  
>> faster
>> here.
>>
>> So, rather than keeping both, I'd suggest replacing AsyncStore with  
>> the
>> impl in CoalescedAsyncStore. We also need to look at the  
>> configuration
>> for the new AsyncStore:
>>
>> Configuration wise in AsyncStoreConfig, I'd leave threadPoolSize,
>> enabled and add the read/write lock acquisition timeout. The rest I'd
>> get rid as no longer apply:
>>
>> int batchSize = 100;
>> long pollWait = 100;
>> int queueSize = 10000;
>>
>> By default, I'd give 5000ms to the read/write lock acquisition  
>> timeout.
>>
>> Thoughts?
>>
>> On 07/09/2009 01:08 PM, Manik Surtani wrote:
>>>
>>> On 9 Jul 2009, at 11:37, Galder Zamarreno wrote:
>>>
>>>>
>>>>
>>>> On 07/09/2009 11:55 AM, Manik Surtani wrote:
>>>>>
>>>>> On 8 Jul 2009, at 19:53, Jason T. Greene wrote:
>>>>>
>>>>>> Manik Surtani wrote:
>>>>>>> * Make the concurrent map volatile
>>>>>>> * When iterating, first create a new ConcurrentMap and replace  
>>>>>>> the
>>>>>>> old one with the new one so all concurrent threads write to  
>>>>>>> the new
>>>>>>> Map
>>>>>>> * Iterate over the old map
>>>>>>
>>>>>> That would lead to race conditions since a concurrent writing  
>>>>>> thread
>>>>>> could write to the "old" map, either by getting a recent yet  
>>>>>> incorrect
>>>>>> read off the volatile, or reading it right before it changes.
>>>>>
>>>>> True, since referencing the map and writing to it isn't atomic.
>>>>>
>>>>> We could guard access to the map with a read/write lock. Safe,  
>>>>> if a
>>>>> little heavy-handed... map writers would acquire a RL (since we  
>>>>> want
>>>>> concurrent access here) but the async flushing thread would need  
>>>>> to
>>>>> wait
>>>>> for a WL to swap the map reference, releasing the lock after the  
>>>>> map
>>>>> reference has been swapped.
>>>>
>>>> Yeah, I don't think it could be volatile because it only applies to
>>>> the variable itself, not the contents pointed at. The R/W lock
>>>> approach looks like a valid one and better than using synchronized
>>>> blocks. Another thing I had in my mind is whether we need the  
>>>> copy to
>>>> be a ConcurrentMap, we could probably just use a
>>>> Immutables.immutableMapCopy().
>>>
>>> Sync blocks would suck since it is effectively an exclusive lock. We
>>> want non-exclusive (read) locks for threads writing to this map - we
>>> want multiple application threads to do this concurrently  
>>> otherwise this
>>> becomes a bottleneck. And hence the need for a ConcurrentMap.
>>>
>>> And we don't make a copy at all - all the async flushing thread  
>>> does is
>>> that it creates a new, empty ConcurrentMap and swaps the 'async  
>>> queue'
>>> Map so that application threads can continue logging their changes
>>> somewhere while old changes can be iterated through and flushed.  
>>> And for
>>> this, the field still needs to be volatile (not strictly so, since  
>>> we
>>> can rely on the fencing that will happen in the lock as a side- 
>>> effect)
>>>
>>>> I looked at the two implementations (ConcurrentHashMap(Map)
>>>> constructor vs Immutables.immutableMapCopy() which would
>>>> Object.clone() on the map) but I'm not sure which one would be  
>>>> faster.
>>>> Since clone() would rely on a native clone() impl, I'd imagine  
>>>> that to
>>>> be faster.
>>>
>>> Like I said, we don't need a copy. We need a new, empty map for  
>>> threads
>>> to log changes to while the old one's being flushed.
>>>
>>>>
>>>>> --
>>>>> Manik Surtani
>>>>> manik at jboss.org
>>>>> Lead, Infinispan
>>>>> Lead, JBoss Cache
>>>>> http://www.infinispan.org
>>>>> http://www.jbosscache.org
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> infinispan-dev mailing list
>>>>> infinispan-dev at lists.jboss.org
>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>
>>>> --
>>>> Galder Zamarreño
>>>> Sr. Software Engineer
>>>> Infinispan, JBoss Cache
>>>
>>> --
>>> Manik Surtani
>>> manik at jboss.org
>>> Lead, Infinispan
>>> Lead, JBoss Cache
>>> http://www.infinispan.org
>>> http://www.jbosscache.org
>>>
>>>
>>>
>>>
>>
>
> -- 
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Manik Surtani
manik at jboss.org
Lead, Infinispan
Lead, JBoss Cache
http://www.infinispan.org
http://www.jbosscache.org