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/a4d107c6783ee7db6bd1bec35b8...
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.