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
And following are the native client problems and solution applies to native-client? - Timeout in ResultCollector::getResult() and Execution::execute() blocking -Anil. On Wed, Aug 21, 2019 at 8:49 AM Alberto Gomez <alberto.go...@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 > > >