Hi Dan, Please, see my answers below.
On 22/8/19 20:29, Dan Smith wrote: > For the two new execute() methods you are proposing, are you also > suggesting that they be implemented on the server? I was only thinking about the client. > > I took a look at the C++ API. The C++ API also has a > ResultCollector.getResult(timeout) method. Does that timeout do anything, > given that execute(timeout) already waited for the given timeout? The DefaultResultCollector in the C++ API handles the timeout with a condition variable in the getResult method (unlike the Java API) but as Jacob Barrett pointed out on another e-mail, given that the execute method is blocking, the timeout is pointless here too. > > Long term, are you thinking that we should remove execute() with no > parameters and getResult(timeout) and just have these two blocking > execute(timeout) methods on the server and clients? I think you are right > that we shouldn't change the behavior of the existing execute() to avoid > breaking our users. But I also want to understand what our long term plan > is for a consistent and sensible API. It depends on the option that gets selected. If we go for option a), the idea would be to keep in clients execute() with no parameters and have also execute() with timeout, and remove the getResult(timeout) method keeping just the getResult() one (which would be non-blocking as it is today). On the servers, I would leave things as they are, execute() is not blocking. Not sure if it makes sense to keep these differences between clients and servers and if it does, how we could maintain them in the API. The fact is that currently execute is very different in clients and in servers. If we go for option b), the idea would be to keep in clients execute() with no parameters and have also execute() with timeout in order not to lose the feature of closing connections if read timeout expires. The socket timeout exception would have to be thrown by the getResult() method instead of the execute method. The execute() methods would be non-blocking and it would be getResult() the one which is blocking, both on servers and clients. For option b) in order to deprecate the blocking behavior in execute() we could keep both implementations (blocking and non-blocking) and have a system property to select the behavior, which could be the blocking one by default in the release this is introduced. But long term, the aim would be to have just the non-blocking implementation of execute(). > > -Dan - Alberto
