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

Dan Berindei dan.berindei at gmail.com
Mon Mar 2 06:08:46 EST 2015 

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 at redhat.com> 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 at redhat.com
>> <mailto:paul.ferraro at redhat.com>> wrote:
>>
>>     ----- Original Message -----
>>     > From: "Galder Zamarreño" <galder at redhat.com
>>     <mailto:galder at redhat.com>>
>>     > To: "infinispan -Dev List" <infinispan-dev at lists.jboss.org
>>     <mailto:infinispan-dev at lists.jboss.org>>, "Paul Ferraro"
>>     <paul.ferraro at redhat.com <mailto:paul.ferraro at 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 at redhat.com <mailto:galder at redhat.com>
>>     > twitter.com/galderz <http://twitter.com/galderz>
>>     >
>>     >
>>
>>     _______________________________________________
>>     infinispan-dev mailing list
>>     infinispan-dev at lists.jboss.org <mailto:infinispan-dev at lists.jboss.org>
>>     https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>>
>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> Radim Vansa <rvansa at redhat.com>
> JBoss Performance Team
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

More information about the infinispan-dev mailing list