On 5 Oct 2011, at 22:58, Vladimir Blagojevic wrote:

Hey guys,

I about to integrate FineGrainedAtomicMap into master but before I do I
need your feedback. I particularly need feedback from users who where
eagerly awaiting this feature (Sanne and others) and possibly their
use/test cases of how they intended to use FineGrainedAtomicMap so we
can integrate these use/test casesinto the test suite.

I created a separate interface FineGrainedAtomicMap to distinguish it
from AtomicMap.
Why do we want to distinguish them? Is the FGAM significantly less performant than AtomicMap? Otherwise why not just make all the maps atomic.
And there's an bug with the current non-fine grained atomic maps maps: no WL is acquired when you write to such a map anymore. And it should be, here's the UT. https://gist.github.com/1267310
Yes, of course, if you  declared a variable of type Map
one could still assign both AtomicMap and FineGrainedAtomicMap to the
same variable but this way we force users to distinguish them at least a
bit. There is a separate factory method in AtomicLookup for
FineGrainedAtomicMap. The best way to see how this works is to look at
the test cases at
https://github.com/vblagoje/infinispan/commit/a4d107c6783ee7db6bd1bec35b8bef3fc2f51de7#diff-22
In order to understand the issue I needed to look into the entire code. Here are some observations:
- you shouldn't register a sync for applying the atomic map changes. Why? There are several reasons for that, the main one being that with this approach FGAM would't be able to participate in distributed transactions. 
I don't see where the FGAM's entries that are added within a transaction's scope are merged within the existing entries in the FGAM. What do code does is keep all transaction-scoped entries in the data container's FGAM instance, even if they are not committed.
This test fails and it obviously shouldn't: https://gist.github.com/1267303
I think before moving forward with the review you'd need to change the code to keep modifications in the transaction's scope and only apply them during commit.

Mircea, I need your help regarding concurrent tx and
FineGrainedAtomicMap. Have a look at
FineGrainedAtomicMapAPITest#testConcurrentTx(the above link) as it is
failing if I do *not* run two threads sequentially, i.e. there is a
sleep of 2 sec to prevent concurrent tx running. Any ideas why is this
happening? The test failed in the old locking architecture as well. In
the old architecture, an entry was not wrapped with MVCCEntry because we
do not look up (and lock) AtomicHashMap but we issues
"synthetic/composite" lock requests for keys in that AtomicHashMap. If
we look up and lock key for AtomicHashMap the the whole point of
FineGrainedMap is lost as we would lock entire map. I noticed that
AtomicHashMap was being wrapped with ImmortalEntry. Maybe this is why
changes are not seen with concurrent tx, but how is it that the changes
are seen if tx are not run concurrently? If you could demystify this for
me I'd be grateful.

Your test fails because on remote put you basically override the map that exist on a node with the map that is obtained from the remote node. 

The commit is here
https://github.com/vblagoje/infinispan/commit/a4d107c6783ee7db6bd1bec35b8bef3fc2f51de7

Regards,
Vladimir