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

[Galder Zamarreno]

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