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

[Amin Abbaspour]

Hi,

Thanks Galder. I will test CoalescedAsyncStore ASAP and let you know.

BTW Manik, I think the change is so much that we'd better see this as
a completely new store and later replace AsyncCacheStore or keep them
both.

Regards,
Amin Abbaspour

On Mon, Jul 20, 2009 at 2:47 PM, Manik Surtani<manik at jboss.org> wrote:
> 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
>
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>