geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jacob Barrett <jbarr...@pivotal.io>
Subject Re: [DISCUSS] Improvements on client function execution API
Date Thu, 22 Aug 2019 19:16:37 GMT


> On Aug 21, 2019, at 8:49 AM, Alberto Gomez <alberto.gomez@est.tech> wrote:
> 2. Timeout in ResultCollector::getResult() and Execution::execute() blocking
> 
> Regarding the timeout in the ResultCollector::getResult() method problem and the blocking/non-blocking
confusion for Execution::execute() two alternatives are considered:
> 
> a) Remove the possibility of setting a timeout on the ResultCollector::getResult() method
on the client side as with the current client implementation it is useless. This could be
done by removing the method with the timeout parameter from the public API.
> 
> It would be advisable to make explicit in the documentation that the getResult() method
does not wait for results to arrive as that should have already been done in the Execution::execute()
invocation.
> 
> This alternative is very simple and would keep things pretty much as they are today.

To be honest I think approach would go against what a user “thinks” is going on. Given
that there hasn’t been a timeout on execute I assumed it was asynchronous and that the getResult
blocked until timeout or results arrived. Typically these two calls were done back to back.


> b) Transform the Execution::execute() method on the client side into a non-blocking method.
> 
> This alternative is more complex and requires changes in all the clients. Apart from
that it has implications on the public client API it requires moving the exceptions offered
currently by the Execution::execute() method to the ResultCollector::getResult() and new threads
will have to be managed.

I think this is more in line with what users expect is going on based on the current API,
I know I have. If were are going to make any change I think this is the one. I don’t think
the behavior change is a problem since it's what is expected to be happening anyway.

> An outline of a possible implementation for option b) would be:
> 
>  *   Instead of invoking the ServerRegionProxy::executeFunction() directly as it is done
today, create a Future that invokes this method and returns the resultCollector passed as
parameter.

Do you really think we need to introduce Futures here into the API? I feel like the ResultCollector
acts as the Future. I don’t think any change needs to be made to the API in this regard.
The ResultCollector implementation would just simply block as implied by the api for the timeout
period. I would change the method to have units though and deprecate the method without units.

-Jake



Mime
View raw message