On 26-09-2016 08:36, Radim Vansa wrote: > Hi all, > > I have realized that fine grained maps don't work reliably with > write-skew check. This happens because WSC tries to load the entry from > DC/cache-store, compare versions and store it, assuming that this > happens atomically as the entry is locked. However, as fine grained maps > can lock two different keys and modify the same entry, there is a risk > that the check & store won't be atomic. Right now, the update itself > won't be lost, because fine grained maps use DeltaAwareCacheEntries > which apply the updates DC's lock (there can be some problems when > passivation is used, though, [1] hopefully deals with them).
Aren't you getting a ClassCastException (re: ISPN-2729)? BTW, why not removing the FineGrainedAtomicMap? The grouping API should provide similar semantics and it would be simpler to handle/use. Also, it provides method to return and remove all keys associated to the group (AdvancedCache#getGroup() and AdvancedCache#removeGroup()). > > I have figured this out when trying to update the DeltaAware handling to > support more than just atomic maps - yes, there are special branches for > atomic maps in the code, which is quite ugly design-wise, IMO. My > intention is to do similar things like WSC for replaying the deltas, but > this, obviously, needs some atomicity. > > IIUC, fine-grained locking was introduced back in 5.1 because of > deadlocks in the lock-acquisition algorithm; the purpose was not to > improve concurrency. Luckily, the days of deadlocks are far back, now we > can get the cluster stuck in more complex ways :) Therefore, with a > correctness-first approach, in optimistic caches I would lock just the > main key (not the composite keys). The prepare-commit should be quite > fast anyway, and I don't see how this could affect users > (counter-examples are welcome) but slightly reduced concurrency. > > In pessimistic caches we have to be more cautious, since users > manipulate the locks directly and reason about them more. Therefore, we > need to lock the composite keys during transaction runtime, but in > addition to that, during the commit itself we should lock the main key > for the duration of the commit if necessary - pessimistic caches don't > sport WSC, but I was looking for some atomicity options for deltas. > > An alternative would be to piggyback on DC's locking scheme, however, > this is quite unsuitable for the optimistic case with a RPC between WSC > and DC store. In addition to that, it doesn't fit into our async picture > and we would send complex compute functions into the DC, instead of > decoupled lock/unlock. I could also devise another layer of locking, but > that's just madness. > > I am adding Sanne to recipients as OGM is probably the most important > consumer of atomic hash maps. > > WDYT? > > Radim > > [1] > https://github.com/infinispan/infinispan/pull/4564/commits/2eeb7efbd4e1ea3e7f45ff2b443691b78ad4ae8e > _______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev
