On 28/09/15 10:13, Simone Bordet wrote:
Hi,
On Mon, Sep 28, 2015 at 10:45 AM, Michael McMahon
<michael.x.mcma...@oracle.com> wrote:
The API has already been approved and the first version to
be integrated won't have all suggestions incorporated.
But, it should be possible to address many of them in time
for JDK 9 anyway.
Okay.
A) lack of backpressure,
My current thoughts on this is to have credit based flow control
window (similar to the Flow API) indicating the number of bytes
that a HttpResponseBodyProcessor is willing to accept and this
maps directly on to the HTTP/2 flow control window and the socket
buffer when doing HTTP/1.
Bear in mind that application must have control over this, so you have
to expose an API that application can use to notify that they are done
with the content.
I am not sure the number of bytes is the right unit - my experience is
that you want to trade ByteBuffers as a unit, not the bytes they
contain.
if the units are ByteBuffers say, then how do you convert a notification
of n units into a HTTP/2 window update (which has to be in bytes)?
Perhaps, if n is required always to be equal to 1, then you could do it
because you would always be referring to the last buffer received
but it seems trickier if n > 1.
This is also critical for buffer reuse and to avoid nasty surprises on
who owns the buffer.
To be clear:
class MyResponseProcessor implements HttpResponseBodyProcessor
{
void onResponseBodyChunk(ByteBuffer buffer)
{
writeAsyncToDisk(buffer, new CompletionHandler()
{
success: <tell implementation it can call onResponseBodyChunk
again>
failure: <tell implementation to abort response>
});
}
}
B) missing "request complete" event
When you suggested this, I decided to split the API so that requests
and responses complete independently, but this was too drastic a change
and most use-cases are satisfied by completion of both request and response.
I am not sure I follow here.
There is currently no way for an application to be notified of request
completion, so there is no way to be notified of completion of both
request and response, only of response.
Are you saying that most use cases will require this, and so it needs
to be addressed, or are you saying that it is already done in a way
that I could not see ?
Can you expand on this ?
What I was saying was that completion of response also implies
competion of request (ie that it isn't notified until after both happens)
It's also possible to be notified of request completion through the
request processor. It should be straight forward to wrap any request
processor
in a processor which provides a CompletableFuture for completion of the
request.
You mean HttpRequestBodyProcessor ?
I don't think it's invoked if there is no body, and it has no
onRequestBodyComplete() method.
Can you expand again ?
It doesn't have a onComplete() type method. The notification
is implied by the final call to onRequestBodyChunk(). But, I accept
that doesn't actually mean the data has been fully written.
So, maybe an onComplete() method is useful ...
C) missing timeouts (CompletableFuture.get(time) is obviously not the
right semantic)
CompletableFuture.get() is at least useful for notifying timeouts.
There is the need for a completely async timeout.
Plus, the semantic of Future.get() is that the time you specified to
get() has passed.
There is no implied semantic that the request has been aborted, and
aborting the request via cancel() would break the Future.get(time)
contract.
Why would it break the contract? Note, I am referring to
HttpRequest.cancel() not CompletableFuture.cancel()
Really, Future.get(time) is the wrong semantic, you don't want to use that.
I suggest that blocking methods throw TimeoutException, and that async
methods get notified with a TimeoutException on their
CompletableFuture.
The question then is what to do next and what was missing was
cancellation, and that has been added. Does that not suffice?
Nope. As said above, there is no async timeout, no idle timeout, no
total (request + response) timeout, which are in my experience the 3
most requested features about timeouts.
