geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alberto Gomez <alberto.go...@est.tech>
Subject Re: [DISCUSS] Improvements on client function execution API
Date Thu, 22 Aug 2019 07:08:17 GMT
Hi,

Please, see my answers below.

- Alberto

On 21/8/19 20:30, Anilkumar Gingade wrote:
> Just to be clear between java and native-client api:
>
> - Read timeout in function execution Java client API - This is to change
> the java client behavior
Yes
>
> And following are the native client problems and solution applies to
> native-client?
> - Timeout in ResultCollector::getResult() and Execution::execute() blocking
This applies to all the clients, both to the Java and the native one 
(C++ and C# actually).
>
> -Anil.
>
>
> On Wed, Aug 21, 2019 at 8:49 AM Alberto Gomez <alberto.gomez@est.tech>
> wrote:
>
>> Hi,
>>
>> I have just added the following proposal in the wiki for discussion and
>> would like to get feedback from the community.
>>
>>
>> https://cwiki.apache.org/confluence/display/GEODE/%5BDiscussion%5D+Improvements+on+client+Function+execution+API
>>
>> Problem
>>
>> The client API for function execution is inconsistent in the following
>> aspects:
>>
>> 1.Read timeout in function execution client API
>>
>> The client API for Function execution allows to set a timeout to wait for
>> the execution of a function.
>>
>> Setting this timeout has the effect of setting a read timeout in the
>> socket. If the timeout expires during the execution of a function before
>> data has been received, the connection is closed and if the number of
>> retries is reached, the execute method throws an exception.
>>
>> Nevertheless, how this timeout is set is not uniform across the different
>> clients:
>>
>>    *   In the native C++ and C# clients, the timeout can be set on a per
>> function execution basis by passing the timeout to the Execution::execute()
>> method.
>>    *   In the Java API, the timeout can only be set globally by a system
>> property when starting the client process.
>>
>> 2. Timeout in ResultCollector::getResult()
>>
>> Apart from the timeout on the function execution, the client API offers
>> the possibility of setting a timeout on the getResult() method of the
>> ResultCollector object returned by the Execution::execute() method.
>>
>> Given that this method is blocking when invoked from a client (until all
>> results have been received) then the setting of this timeout has no effect
>> at all. In fact, the DefaultResultCollector in the Java API just ignores
>> the value of the timeout.
>>
>> Note that this timeout in the ResultCollector::getResult() method is
>> useful when used inside a peer as function invocations are not blocking.
>>
>> 3. Blocking  Execution::execute()
>>
>> As mentioned above the Execution::execute() method behavior is different
>> when invoked from clients than from peers. When invoked from clients it is
>> blocking until all results are received while when invoked from peers it is
>> non-blocking and the ResultCollector::getResult() method is used to wait to
>> get all the results.
>>
>> This is not explicit in the documentation of the interface and it has
>> already been captured in the following ticket:
>> https://issues.apache.org/jira/browse/GEODE-3817
>>
>> Anti-Goals
>>
>> -
>>
>> Solution
>>
>> In order to make the API more coherent two actions are proposed:
>>
>> 1. Read timeout in function execution Java client API
>>
>> Add two new Execution::execute() methods in the Java client to offer the
>> possibility to set the timeout for the socket just as it is done in the C++
>> and C# clients.
>>
>> This action is simple to implement and there are no adverse effects
>> attached to it.
>>
>> 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.
>>
>> 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.
>>
>> 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.
>>    *   Create a new class (ProxyResultCollector) that will hold a Future
>> for a ResultCollector and whose getResult() methods implementation would be
>> something like:
>>
>> return this.future.get().getResult();
>>
>>    *   After creating the future that invokes
>> ServerRegionFunction::executeFunction() create an instance of
>> ProxyResultCollector and call the setFuture() method on it passing the just
>> created future and return it.
>>
>> Changes and Additions to Public Interfaces
>>
>> Regarding the first action, the Java client API would grow by adding the
>> following two methods:
>>
>>    *
>>
>> ResultCollector<OUT, AGG> execute(String functionId, long timeout,
>> TimeUnit unit) throws FunctionException;
>>
>>    *
>>
>> ResultCollector<OUT, AGG> execute(Function function, long timeout,
>> TimeUnit unit) throws FunctionException;
>>
>> Depending on the option selected on the second action:
>>
>>    *   For option a) the methods to set a timeout on the
>> ResultCollector::getResult() would be removed on the client side or it
>> would be documented that the timeout has no effect
>>    *   For option b) the client documentation would be updated to indicate
>> that the Execute::execute() methods are non-blocking.
>>
>> Performance Impact
>>
>> No foreseen impacts on performance.
>>
>> Backwards Compatibility and Upgrade Path
>>
>> Depends on the option selected.
>>
>> If the timeout for the ResultCollector::getResult() is deprecated, this
>> could be done by updating the documentation in which case there will be no
>> impacts on clients.
>>
>> If the Execution::execute() method is made non-blocking there could be
>> impacts on clients that expected the method to be blocking. In this case,
>> it may be necessary to offer a configuration setting to offer both
>> behaviors (blocking/non-blocking) in the release where the blocking version
>> is deprecated.
>>
>> Thanks!
>>
>>
>> - Alberto G
>>
>>
>>
Mime
View raw message