[infinispan-dev] Sequential interceptors API

[Galder Zamarreño]

Hey Radim,

Sorry for the delay having a look at this (Summit/DevNation then vacation).

I agree with you that the way you're passing down the handler down the stack and accross lambdas is a bit counter-productive, because it means the lambda has to capture the handler, which in turn means that each lambda execution will result in a lambda instantiation.

Looking at AsyncInvocationContext, methods like this feel a bit of an anti-pattern:

CompletableFuture<Void> forkInvocation(VisitableCommand newCommand, AsyncInterceptor.ForkReturnHandler forkReturnHandler)

The problem here is that it mixes the two main ways of dealing with async computation:

1. Callback handlers [1].
2. Promises (~ CompletableFuture) [2].

Normally, you either use one or the other, but here both are in use because the method returns a CompletableFuture (e.g. like a JS/Scala promise), and then is taking a callback handler as well.

If you go down the path of promises, the normal thing would be:

CompletableFuture<Void> forkInvocation(VisitableCommand newCommand)

And then you chain (e.g. thenCompose or similar) the return function when the fork has completed. I assume that going down this route causes unnecessary allocations?

If we're worried about allocations, I wonder whether a pure handler based approach might work better? It might not be as user-friendly but if we want to avoid CompletableFuture issues internally, then maybe that's the way? e.g.

void forkInvocation(VisitableCommand newCommand, AsyncInterceptor.ForkReturnHandler forkReturnHandler);

While at Summit/DevNation I had a chat with Clement from Vert.x team who had some reservations about CompletableFutures themselves. I don't remember the details right now (that's what vacation does to you!) but I've pinged him to find out more.

In general though, there are multiple issues associated with the handler approach, including:

* Lack of central error handling logic.
* Callback readibility, when multiple handlers are specified in one go [3].

Cheers,

[1] http://www.servicedesignpatterns.com/WebServiceInfrastructures/AsyncResponseHandler
[2] http://exploringjs.com/es6/ch_promises.html
[3] http://callbackhell.com
--
Galder Zamarreño
Infinispan, Red Hat

> On 22 Jun 2016, at 17:52, Radim Vansa <rvansa at redhat.com> wrote:
> 
> Yes [1]. The longest chaining of operations I had in [2], basically 
> during ST I have to load a value locally*, perform a unicast/broadcast 
> to read different value and then execute the original one.
> 
> * I shouldn't load it just from DC, as it could be in cache store, too; 
> though without persistence (which I don't deal with properly in 
> scattered cache yet) it would be more efficient to do the DC lookup 
> directly.
> 
> Radim
> 
> [1] https://github.com/rvansa/infinispan/tree/ISPN-6645
> [2] 
> https://github.com/rvansa/infinispan/blob/ISPN-6645/core/src/main/java/org/infinispan/interceptors/impl/PrefetchInvalidationInterceptor.java
> 
> On 06/17/2016 10:52 AM, Galder Zamarreño wrote:
>> Radim, do you have a branch where you have been trying these things out? I'd like to play with what you're trying to do.
>> 
>> Cheers,
>> --
>> Galder Zamarreño
>> Infinispan, Red Hat
>> 
>>> On 8 Jun 2016, at 14:23, Radim Vansa <rvansa at redhat.com> wrote:
>>> 
>>> Hi,
>>> 
>>> I would like to encourage you to play with the (relatively) new API for
>>> sequential interceptors, and voice your comments - especially you corish
>>> devs, and Galder who has much experience with async invocations and that
>>> kind of stuff from JS-world.
>>> 
>>> I am now trying to use the asynchronous methods only (the
>>> forkInvocationSync() is only temporary helper!); Dan has made it this
>>> way as he wanted to avoid unnecessary allocations, and I welcome this
>>> GC-awareness, but regrettably I find it rather hard to use, due to its
>>> handler-style nature. For the simplest style interceptors (do this,
>>> invoke next interceptor, and process result) it's fine, but when you
>>> want to do something like:
>>> 
>>> visitFoo(cmd) {
>>>   Object x = null;
>>>   if (/* ... */) {
>>>       x = invoke(new OtherCommand());
>>>   }
>>>   invoke(new DifferentCommand(x));
>>>   Object retval = invoke(cmd);
>>>   return wrap(retval);
>>> }
>>> 
>>> I find myself passing handlers deep down. There is allocation cost for
>>> closures, so API that does not allocate CompletableFutures does not pay off.
>>> 
>>> I don't say that I could improve it (I have directed my comments to Dan
>>> on IRC when I had something in particular), I just say that this is very
>>> important API for further Infinispan development and everyone should pay
>>> attention before it gets final.
>>> 
>>> So please, play with it, and show your opinion.
>>> 
>>> Radim
>>> 
>>> -- 
>>> 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
>> 
>> _______________________________________________
>> 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