<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Very good point.<div>I rdeplied inline: <a href="https://github.com/infinispan/infinispan/pull/1221#discussion_r1310710">https://github.com/infinispan/infinispan/pull/1221#discussion_r1310710</a></div><div><br><div><div>On 6 Aug 2012, at 11:27, Sanne Grinovero wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>I didn't read all the details on the optimistick lock implementation,<br>but want to warn here against the interpretation of the<br>SKIP_REMOTE_LOOKUP flag for these purposes; it should only apply on<br>REMOTE lookups, not imply a general-purpose "I'm ignoring the return<br>value", that would be misleading for users as it's not how things<br>work, it's not automatically implying for example also a skip from<br>cache stores or even a local data container GET hit.. which could<br>affect access statistics and eviction decisions.<br><br><a href="https://github.com/infinispan/infinispan/pull/1221#r1310549">https://github.com/infinispan/infinispan/pull/1221#r1310549</a><br><br>On 25 July 2012 14:27, Mircea Markus <mircea.markus@jboss.com> wrote:<br><blockquote type="cite"><br></blockquote><blockquote type="cite">On 25 Jul 2012, at 12:26, Galder Zamarreņo wrote:<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">On Jul 25, 2012, at 1:14 PM, Mircea Markus wrote:<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">On 24 Jul 2012, at 20:44, Galder Zamarreņo wrote:<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Mircea, one last thing. Why is there a check for local here?<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">https://github.com/infinispan/infinispan/blob/master/core/src/main/java/org/infinispan/interceptors/locking/OptimisticLockingInterceptor.java#L95<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">That basically means that concurrent cluster wide conditional removes won't<br></blockquote><blockquote type="cite">work with OL + RR + writeSkew.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Is there a reason why you added this local check?<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">That code was added with ISPN-1941 and only handles write skew (ws) for<br></blockquote><blockquote type="cite">local caches, Manik might comment a bit more about it.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">The code that handles ws for distributed caches is in the<br></blockquote><blockquote type="cite">VersionedEntryWrappingInterceptor(VEWI).<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">I don't think that that code you pointed belongs to the<br></blockquote><blockquote type="cite">OptimisticLockingInterceptor (OLI) for two reasons: the OLI is should handle<br></blockquote><blockquote type="cite">the locking and write skew checking is an orthogonal concern. Also the rest<br></blockquote><blockquote type="cite">of write skew checking is handled in the VEWI, so this logic should be<br></blockquote><blockquote type="cite">placed there as well.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">TBH I think write skew check logic deserve its own dedicated interceptor as<br></blockquote><blockquote type="cite">the code in this area is spread over too many places: OLI, VEWI and some<br></blockquote><blockquote type="cite">not nice static methods in the ClusteringDependentLogic. At least for me it<br></blockquote><blockquote type="cite">is quite hard to follow it as it is now. Wdyt?<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Good points. So, we need a better way of doing write skew checks both local<br></blockquote><blockquote type="cite">and in cluster, from a more centralized place then?<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Let's see what Manik and others say, but won't be able to properly fix this<br></blockquote><blockquote type="cite">before I go on holidays. There's a valid workaround for it though, so not<br></blockquote><blockquote type="cite">massively urgent.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">+1.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">_______________________________________________<br></blockquote><blockquote type="cite">infinispan-dev mailing list<br></blockquote><blockquote type="cite">infinispan-dev@lists.jboss.org<br></blockquote><blockquote type="cite">https://lists.jboss.org/mailman/listinfo/infinispan-dev<br></blockquote><br>_______________________________________________<br>infinispan-dev mailing list<br>infinispan-dev@lists.jboss.org<br>https://lists.jboss.org/mailman/listinfo/infinispan-dev</div></blockquote></div><br></div></body></html>