[infinispan-dev] AtomicMapCache in transactional mode returning null
Hi,
This is related to https://community.jboss.org/message/541023#541023 thread. Once
https://jira.jboss.org/jira/browse/ISPN-418 fails, another issue appears whereby a null
delta is written to the cache store. The reason for this is that the code in
testRestoreTransactionalAtomicMap actually generates two modifications in the transaction.
The first one comes from the putIfAbsent below that can be found in AtomicHashMap:
public static AtomicHashMap newInstance(Cache cache, Object cacheKey) {
AtomicHashMap value = new AtomicHashMap();
Object oldValue = cache.putIfAbsent(cacheKey, value);
if (oldValue != null) value = (AtomicHashMap) oldValue;
return value;
}
And the second modification comes from AtomicHashMapProxy.getDeltaMapForWrite call by the
code below. This comes from the map put call:
public V put(K key, V value) {
try {
startAtomic();
InvocationContext ctx = icc.createInvocationContext();
AtomicHashMap<K, V> deltaMapForWrite = getDeltaMapForWrite(ctx);
return deltaMapForWrite.put(key, value);
}
finally {
endAtomic();
}
}
On one side, these two modifications do exactly the same, a put with the atomic hash map,
so it's not very efficient. One put would be enough for the test to work. However, the
reason the test fails is after writing the first modification via:
public void writeObject(ObjectOutput output, Object subject) throws IOException {
DeltaAware dw = (DeltaAware) subject;
output.writeObject(dw.delta());
}
dw.delta() call resets the delta and sets it to null. So, when the second modification is
written, delta is null and that's the final value written. Obviously, when data is
read, null delta is retrieved and hence the test fails.
So IMO, one of the two modifications must go and my vote is for the one in newInstance. I
don't really understand why we should put when creating a new instance of
AtomicHashMap. It makes more sense to do the put when you actually modify the map via a
put call for example. Manik, any idea why you added the putIfAbsent to newInstance?
As a side note, and this might be related, the javadoc of newInstance needs updating since
AtomicMapCache does not exist. It should probably refer to AtomicMapLookup instead of
AtomicMapCache.
/**
* Construction only allowed through this factory method. This factory is intended for
use internally by the
* CacheDelegate. User code should use {@link
org.infinispan.atomic.AtomicMapCache#getAtomicMap(Object)}.
*/
Cheers,
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache