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

[Manik Surtani]

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