apache / pekko-http

The Streaming-first HTTP server/module of Apache Pekko
https://pekko.apache.org/
Apache License 2.0
149 stars 36 forks source link

support making HTTP client requests using HTTP/2 #484

Open pjfanning opened 6 months ago

pjfanning commented 6 months ago

See https://github.com/matsluni/aws-spi-akka-http/pull/226#issuecomment-1943538007 for context.

It appears that Http().singleRequest(...) only supports sending HTTP/1.1 requests.

We have samples that use HTTP/2 requests but it might be nice to make it more straightforward.

samueleresca commented 6 months ago

Is this issue available for being picked up? I should have time in the next few weeks.

pjfanning commented 6 months ago

Is this issue available for being picked up? I should have time in the next few weeks.

Sure - this is open for anyone to pick it up

samueleresca commented 5 months ago

I was taking a look at this. Http().singleRequest(...) a.k.a Request-Level Client-Side API uses a connection pool (see: PoolInterface) to reuse connections. In order to expose an single request Http2 API(i.e.:Http2().singleRequest(HttpRequest(uri = "http://localhost"))) and keeping the same behavior of Http().singleRequest I think we can proceed as follow:

Is my understanding / approach right here? Tagging explicitly @raboof as I saw his name in some of the files involved in the changes + some HTTP/2 related work.

jrudolph commented 4 months ago

A few thoughts here:

What we already have for HTTP/2:

What is missing:

In terms of essential functionality, it seems that not much is missing. Many backend usages that need an HTTP/2 client are already supported if you can do the "dispatch to the right host connection" (which only requires that you can keep the single managed connection as a state in your client shim).

jrudolph commented 4 months ago

I think we can proceed as follow

I don't think we should do it like this. Especially, we don't want another entry point like Http2(). If we want to allow HTTP2 to be integrated more deeply, it should be done transparently using the existing APIs.

In theory, all existing APIs can be supported by "just" allowing PoolInterface to deal with HTTP/2. This might not be a trivial task because of assumptions between the PoolInterfaceStage and NewHostConnectionPool. Let's say, we ignore automatic negotiation for now and require that you need to configure whether you want to connect to a host via HTTP/1.1 or HTTP/2 (i.e. a new setting in host-connection-pool`).

What needs to be done when HTTP/2 is selected is to replace the code at PoolInterface.apply, to create an alternative connection flow that uses a single managed persistent HTTP/2 connection instead of the pool of HTTP/1.1 connections.

https://github.com/apache/pekko-http/blob/87f55da5817b20be224b96d87e984bdfbf883f79/http-core/src/main/scala/org/apache/pekko/http/impl/engine/client/PoolInterface.scala#L76-L79

On the surface, it might be enough to switch out those two lines with the HTTP/2 equivalent. I wouldn't surprised if there are some subtle issues/bugs remaining in managedPersistentHttp2 which was a late addition to HTTP/2 support in akka-http.

Some notes on automatic negotiation:

You can do protocol negotiation, either by being told that HTTP/2 protocol is available using the alt-svc HTTP header, or by participating in TLS / ALPN negotiation. In any case, you only get told that HTTP/2 is available relatively late in the connection setup. Since the flows for HTTP/1.1 and HTTP/2 need to be wired differently, it is hard to negotiate on-the-fly and then wire things correctly in a pool setup. That means, you need to run a pre-flight request to figure out whether a certain host would support HTTP/2 and keep that information around for the rest of the session / application runtime. I would see that as an additional future feature. We could keep the http2 selection setting in host-connection-pool as selecting from http1 / http2 and then later add auto or preflight.

jrudolph commented 4 months ago

What needs to be done when HTTP/2 is selected is to replace the code at PoolInterface.apply, to create an alternative connection flow that uses a single managed persistent HTTP/2 connection instead of the pool of HTTP/1.1 connections.

@samueleresca this is the first thing I would try. Hopefully, the flows implemented by OutgoingConnection.managedPersistentHttp2 and NewHostConnectionPool are compatible enough for it to work.

jrudolph commented 4 months ago

OutgoingConnection.managedPersistentHttp2 and NewHostConnectionPool are compatible enough for it to work

Which is not quite the case right now. NewHostConnectionPool is Flow[RequestContext, ResponseContext] while managedPersistentHttp2 is Flow[HttpRequest, HttpResponse]. That's probably not a blocker, it's more an artifact of the HTTP/1.1 low-level impl not (yet) supporting the more general RequestResponseAssociation protocol to associated requests with responses that HTTP/2 supports.

samueleresca commented 4 months ago

@jrudolph Thanks for the steering.

I don't think we should do it like this. Especially, we don't want another entry point like Http2(). If we want to allow HTTP2 to be integrated more deeply, it should be done transparently using the existing APIs.

ACK on the above. So the apporach you are suggesting is to skip the initialization of the NewHostConnectionPool in case http2 is specified and to use directly the flow coming from managedPersistentHttp2 for constructing the PoolInterface?

Which is not quite the case right now. NewHostConnectionPool is Flow[RequestContext, ResponseContext] while managedPersistentHttp2 is Flow[HttpRequest, HttpResponse]. That's probably not a blocker, it's more an artifact of the HTTP/1.1 low-level impl not (yet) supporting the more general RequestResponseAssociation protocol to associated requests with responses that HTTP/2 supports.

What is your recommendation here? I'm seeing that [Request|Response]Context is a case class wrapping a Http[Request|Response]. Should I create a new PoolInterfaceStage implementation that handle Flow[HttpRequest, HttpResponse] instead of Flow[RequestContext, ResponseContext]?