HTTP 2 client API

[Michael McMahon]

On 31/07/15 16:06, Simone Bordet wrote:
> Hi,
>
> On Fri, Jul 31, 2015 at 3:56 PM, Michael McMahon
> <michael.x.mcmahon at oracle.com> wrote:
>> Ok. Now, on the whole flow-control issue, here is my thinking.
>> First that isFast() method is a relic from a previous version and should not
>> have been there.
>>
>> My thinking is that the body processing methods of the response
>> body processor should be simply allowed to block. That exerts back pressure
>> in HTTP/1 by the socket buffer filling up and TCP flow control kicking in.
>> For HTTP/2, the protocol's own buffering and window management functions
>> achieve the same
>> thing.
> Sure, if an application use blocking APIs, backpressure it achieved.
>
> But as a generic library designer, it's not possible to know if the
> application will use blocking APIs. If they don't want to, they can't
> use JDK's HttpClient.
> That may be ok, but would be a shame.
> I brought this up because it's an API design issue, not something that
> you can change with a better implementation while keeping the same
> APIs.
>
> If the API supports an asynchronous mode, it's trivial to implement a
> blocking mode, see for example:
> https://github.com/eclipse/jetty.project/blob/master/jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java#L238-L255
>
>> For sending, we already control the rate of outgoing data by how regularly
>> the implementation pulls data from the processor.
> But you may need to notify the producer asynchronously.
>
> Imagine you have HttpClient1 downloading data from server1, and
> HttpClient2 that must upload this data to server2.
> How would you do this with the current APIs in a completely asynchronous way ?
>

Well, it needs co-operation between the producer and the consumer
obviously. But, the onResponseBodyChunk() method could write to a queue
and block if it reaches a certain size. The onRequestBodyChunk() would
read off the data and send it on. When the queue reaches a lower limit it
notifies the producer to continue. It's not asynchronous, but is possible
and I'm not sure we're aiming at this type of application.
Having said that, this is an area we planned to look at again, after the
initial putback.

>> Yes, the API is designed that responses are handled in two stages
>> response headers and then response body.
>>
>> Using the asynchronous API it is easy to create a composition of
>> the two stages and have one CompletableFuture object complete
>> after both stages. Probably, the most useful will be CompletableFuture<T>
>> where T is the actual body type. Similar can be achieved with the blocking
>> API.
> I'm not sure you got my point, I apologize if I did not understand your answer.
>
> The issue is not about the 2 stages of a response.
> It's about *request* and *response*, together as an exchange,
> completeness, for which there is no API.
> Bad things happens when the response is complete and you think the
> request is completed too, but it's not.
>
> I'm not saying it's impossible to code this with the current APIs, but
> I wanted to raise the issue.
>
> Example: client uploads a video of 500 GiB using a single mapped
> buffer from a file over a slow connection;
> server has a bug and throws a NPE while reading the first content byte;
> the NPE is converted to a HTTP response 500, which is downloaded
> immediately to the client;
> the client receives the full response 500 with its body, but the
> request is still uploading the file.
>
> An application would be interested in knowing when the whole exchange
> (request + response) is complete, not when the response alone is
> complete.
> An application may also want to apply timeout to the whole exchange
> (see below re: timeouts).

I see what you mean. There seems to be two issues here:

1) notification of request complete, which we don't have
     an explicit API for, but request processors can do something
     about that. Regardless maybe it is worth making this more explicit
     in the API.

2) cancellation of such requests. See below.

>> Cancellation is part of the CompletableFuture API
> But it has the wrong semantic, if you refer to Future.cancel(boolean).
> Future.cancel(boolean) does not have any effect on the underlying
> operation (in this case if the request has already started).
>
>> but not in the blocking API currently. HTTP/2 should be
>> able to implement it by resetting streams.
> See above it's a semantic problem.
> If you implement it respecting the Future semantic, it's not the right
> semantic for HttpClient, and viceversa.

Strangely, Future.cancel() doesn't specify any such restriction, but
CompletableFuture.cancel() does. In theory, one could sub-class
CompletableFuture and implement cancellation behavior that way,
but the intent does seem to be that cancellation should come from outside
and just be reported through this class. And considering the point above
about stopping runaway request sends then maybe an explicit
cancellation API would be useful.

>> I think the docs could use a simple example, but I thought
>> it seemed a useful addition.
>>
>> eg builder.headers("Foo", "foovalue", "Bar", "barvalue).
>>
>> The effect is additive. So, there can be multiple values
>> for any header name
> How do you know if a string is a name or a value ?
>
> builder.headers("Foo", "Bar", "Baz")
>
> sets header Foo with values Bar and Baz, or sets header Foo with value
> Bar, and header Baz with value... what ?
>
> IMHO the API is confusing, I prefer just doing builder.header("Foo",
> "foovalue").header("Bar", "barvalue").

the parameters have to be specified in pairs
with (header name first, then value) * n

>>> == Timeouts
>>> There are no definition of timeouts, including idle timeouts and
>>> exchange timeouts ?
>>
>> Timeouts are all handled by the CompletableFuture API
> But again they have the wrong semantic, if you refer to
> Future.get(long, TimeUnit).
> The expiration of that timeout does not mean that the request is
> cancelled or anything else.
> It just means that that amount of time has passed.
>
> You can happily write:
>
> while (true)
> {
>      try
>      {
>          future.get(1, SECONDS);
>          break;
>      }
>      catch (TimeoutException x)
>      {
>          // Not done yet, try again.
>      }
> }
>
> without implying any effect on the actual underlying operation.
> This means that the timeout expires, but the request is not cancelled,
> and if it is never responded, it will occupy resources forever.
>

Yes, this is true also. Good points.

Thanks!
Michael