Re: [infinispan-dev] Asynchronous cache's "void put()" call expectations changed from 6.0.0 to 6.0.1/7.0

The AsyncCache interface already has an xxxAsync method for every write operation in the Cache interface. Those operations really behave just like the sync operations, except they return a NotifyingFuture - basically what .

The FORCE_ASYNCHRONOUS flag is different, because you have no way to find out if the operation was successful. In fact, it behaves as if the cache was REPL_ASYNC. This can indeed cause some confusion, as the async operation can break consistency guarantees for concurrent sync operations. I'm all for removing REPL_ASYNC and the FORCE_ASYNCHRONOUS flag, but I think users will complain about their removal because putAsync is slower and they don't care/know about potential consistency problems.

There's also the question of putForExternalRead, which is currently defined to always replicate the value asynchronously. If REPL_ASYNC is removed, it doesn't make sense to keep the async replication code only for putForExternalRead, so it would have to behave like putAsync (maybe with an extra CACHE_MODE_LOCAL put). Cheers Dan On Mon, Mar 2, 2015 at 12:20 PM, Radim Vansa <rvansa(a)redhat.com&gt; wrote: > I second Galder's opinion that sync/async should be rather a matter of > API than configuration, because you have to deal with that in code. > Therefore, when planning a behaviour of any operation, I would suggest > that it should be possible to host sync and async operations (currently > forcable through a flag) with the proper semantics - different paths of > messages would complicate this a lot (e.g. the guarantee that eventually > all owners share the same value). > > The most natural semantics would be IMO that async operations return > without any guarantee *when* this operation will be applied (and even > *if* since it can silently fail with error), with optional way to find > out that it has been applied (as soon as we add the Java 8 API). And the > effect should be the same as for sync version. It's a subject for > discussion whether the retry should be done upon topology change (but it > needs to be transparent to user anyway). > > My 2c > > Radim > > On 03/02/2015 09:55 AM, Dan Berindei wrote: >> Reviving the thread... >> >> Paul, this is a little more complicated than just the return value of >> the put. In Infinispan prior to 6.0.1, this would have always worked >> with a replicated asynchronous cache: >> >> cache.put(k, v); // return value ignored >> v1 = cache.get(k); >> assertEquals(v, v1); >> >> The behaviour of Infinispan prior to 6.0.1 was to write the value to >> the local cache synchronously, and only replicate the value to the >> other owners asynchronously. >> Of course, if the cache was distributed and the originator was not an >> owner, the assertion could have failed anyway, because the remote get >> could be executed before the remote put. >> The get(k) is not guaranteed to return the value inserted by the >> put(k, v), but it did work most of the time. >> >> Since 6.0.1, the originator doesn't update the local value >> synchronously any more, unless it is the primary owner. If it is a >> backup owner, it will send the update asynchronously to the primary >> owner, which will then update the backup owners (including the >> originator) synchronously, while holding the lock. This was supposed >> to change in 5.3.0, as replicated mode was already using lock >> delegation in 5.3.0, but EntryWrappingInterceptor had an outdated >> check that meant the originator did both the synchronous update and >> the asynchronous one. The fix in 6.0.1 was just to remove the >> synchronous update. >> >> Personally I think the pre-6.0.1 behaviour was a bit dangerous, >> because the get would return the updated value *almost* all of the >> time. So there was a chance that tests would work, and the app would >> only fail in production. putIfAbsent will also behave more like an >> atomic operation with 6.0.1+, at least as long as the primary owner of >> the key doesn't change. >> >> That's not to say the current behaviour is perfect. I'm open to >> changing REPL_ASYNC back to updating the local data container first >> (if owner) and then sending the write command asynchronously to all >> the members in parallel, like we did in 5.2.0 and before. >> >> We could also change DIST_ASYNC to behave the same, because the >> current behaviour is a little too close to what putAsync does in >> DIST_SYNC mode, and people using async replication should be ok with >> nodes applying modifications in a different order (it happens on >> primary owner change anyway). >> >> However, I'm not sure what we should do for atomic operations like >> replace(key, oldValue, newValues). I would suggest throwing a >> UnsupportedOperationException, because the operation is very unlikely >> to be atomic. It's true that PFER is also specified as behaving the >> same as putIfAbsent, but the PFER check is going to be on the >> originator only so we can just remove that bit from the Javadoc. >> >> Another option would be to change PFER only to behave like in 5.3.0 >> and 6.0.0. We could say PFER performs two put operations, one that's >> local-only and one that's replicated but asynchronous (updating the >> value on the originator a second time). Or we could say PFER always >> updates the value on the local node only... >> >> Cheers >> Dan >> >> On Tue, Sep 2, 2014 at 3:19 PM, Paul Ferraro <paul.ferraro(a)redhat.com >> <mailto:paul.ferraro@redhat.com>> wrote: >> >> ----- Original Message ----- >> > From: "Galder Zamarreño" <galder(a)redhat.com >> <mailto:galder@redhat.com>> >> > To: "infinispan -Dev List" <infinispan-dev(a)lists.jboss.org >> <mailto:infinispan-dev@lists.jboss.org>>, "Paul Ferraro" >> <paul.ferraro(a)redhat.com <mailto:paul.ferraro@redhat.com>> >> > Sent: Monday, September 1, 2014 11:08:45 AM >> > Subject: Asynchronous cache's "void put()" call expectations >> changed from 6.0.0 to 6.0.1/7.0 >> > >> > Hi all, >> > >> > @Paul, this might be important for WF if using async repl caches >> (the same I >> > think applies to distributed async caches too) >> >> Luckily, Dan warned me that of this behavioral change well in advance. >> Any time we need a reliable return value from a Cache.put(...), we >> use Flag.FORCE_SYNCHRONOUS so that the same code will work for >> sync and async caches alike. >> >> >> > Today I’ve been trying to upgrade Infinispan version in >> Hibernate master from >> > 6.0.0.Final to 7.0.0.Beta1. Overall, it’s all worked fine but >> there’s one >> > test that has started failing. >> > >> > Essentialy, this is a clustered test for a repl async cache (w/ >> cluster cache >> > loader) where a non-owner cache node does put() and immediately, >> on the same >> > cache, it calls a get(). The test is failing because the get() >> does not see >> > the effects of the put(), even if both operations are called on >> the same >> > cache instance. >> > >> > According to Dan, this should have been happening since [1] was >> implemented, >> > but it’s really started happening since [2] when lock delegation >> was enabled >> > for replicated caches >> (EntryWrappingInterceptor.isUsingLockDelegation is now >> > true whereas in 6.0.0 it was false). >> > >> > Not sure we set expectations in this regard, but clearly it’s >> big change in >> > terms of expectations on when “void put()” completes for async >> repl caches. >> > I’m not sure how we should handle this, but it definitely needs some >> > discussion and adjuts documentation/javadoc if needed. Can we do >> something >> > differently? >> > >> > Indepent of how we resolve this, this is the result of once >> again of trying >> > to shoehole async behaviour into sync APIs. Any async caches >> (DIST, INV, >> > REPL) should really be accessed exclusively via the AsyncCache >> API, where >> > you can return quickly and use the future, and any listener to >> attach to it >> > (a bit ala Java8’s CompletableFuture.map lambda calls) as a way >> to signal >> > that the operation has completed, and then you have an API and >> cache mode >> > that make sense and is consistent with how async APIs work. >> > >> > Right now, when a repl async cache’s “void put()” call is not >> very well >> > defined. Does it return when message has been put on the >> network? What >> > impact does it have in the local cache contents? >> > >> > Also, a very big problem of the change of behaviour is that if >> left like >> > that, you are forcing users to code differently, using the same >> “void put()” >> > API depending on the configuration (whether async/sync). As >> clearly shown by >> > the issue above, this is very confusing. It’s a lot more logical >> IMO, and >> > I’ve already sent an email on this very same topic [3] back in >> January, that >> > whether a cache is sync or async should be based purely on the >> API used and >> > forget about the static configuration flag on whether the cache >> is async or >> > sync. >> >> I would agree would this last statement. Consistent semantics are >> a good thing. >> If you do change this, however, just let me know well in advance. >> >> > Cheers, >> > >> > [1] https://issues.jboss.org/browse/ISPN-2772 >> > [2] https://issues.jboss.org/browse/ISPN-3354 >> > [3] >> http://lists.jboss.org/pipermail/infinispan-dev/2014-January/014448.html >> > -- >> > Galder Zamarreño >> > galder(a)redhat.com <mailto:galder@redhat.com> >> > twitter.com/galderz <http://twitter.com/galderz> >> > >> > >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev(a)lists.jboss.org <mailto:infinispan-dev@lists.jboss.org> >> https://lists.jboss.org/mailman/listinfo/infinispan-dev >> >> >> >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > -- > Radim Vansa <rvansa(a)redhat.com&gt; > JBoss Performance Team > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev