Support and enable HTTP/2

[mosabua]

As discussed in numerous contributor and maintainer meetings, we plan to move Trino towards HTTP/2. Patches to support that and use it in production exist already in the user and contributor community. Experience in terms of performance gains is very promising and a desire to have this feature in Trino itself is shared across numerous stakeholders.

This issue aims to pull the various efforts together and serve as communication tool.

Following are a couple of tasks and aspects to consider:

• Upgrade to Jetty 12 for solid HTTP/2 support, done?
• Receive approval at community contributor company
• Issue PR from community contributor
• Test HTTP2 for internal communication, and maybe enable by default
• Test with secured internal communication, and maybe enable by default
• Expand towards using for client communication
• Expand towards using for data source (connector-level) communication
• Expand towards other HTTP client usage
• Configuration will be necessary to finely control where HTTP2 is used, ideally have a naming convention and consistent behavior and defaults
• For many usages it will be possible and probably beneficial to support and use longer polled sockets that stay open

[wendigo]

Jetty 12 is done but some changes in the airlift would be still needed (ALPN, HTTP/3?)

[wendigo]

airlift/airlift#1156

[mosabua]

Related NPE fix in Airlift - airlift/airlift#1154

[mosabua]

Related issue in Jetty jetty/jetty.project#11631

Fix will be out with 12.0.9

[mosabua]

Related Trino isssue - #21735

[wendigo]

@mosabua I'd say that 21735 is unrelated to HTTP/2 itself - it applies to HTTP/1 and any other protocol as well

[wendigo]

I've experimented with HTTP/2 support in the Airlift and I came to the following conclusions that I'd like to discuss and seek approval from @dain @electrum.

Let's start with the features that we have implemented and knobs and switches that we expose right now:

1. Airlift's http-server supports:

• http/1.1+TLS for secure connections
• http/1.1 and h2c for insecure ones

2. Airlift's http-client supports:

• http/1.1+TLS for secure connections
• http/1.1 for insecure connections
• http/2+TLS and h2c (HTTP/2 over clear-text) if http-client.http2.enabled is enabled (off by default)

If the default configuration is used for both server and client:

• HTTP/1.1 and HTTP/1.1+TLS are only used for communication

If the http-client.http2.enabled is enabled:

• H2C is used for insecure communication
• HTTP/1.1+TLS is used for secure communication.

In order to introduce HTTP/2 support I'd like to introduce following changes:

• Remove support for HTTP/2 over clear-text (H2C) altogether from the server and tests. It's causing issues and there is no benefit of keeping it. It was never fully enabled and tested in production.
• Change Jetty's client to use HttpClientTransportDynamic with HTTP/2 and HTTP/1.1 protocols supported (HTTP/2 preferred over HTTP/1.1). HttpClientTransportDynamic is using ALPN to negotiate preferred protocol on the server side.
• Enable http-client.http2.enabled by default. Without the explicit request downgrade to HTTP/1.1 (next point) this is backward-incompatible change. In case of the HTTP/1.1 servers over clear-text this will try to negotiate H2C and fail as there is no way for Jetty's client to negotiate protocol without ALPN over clear text - protocols are used in the order of the preference in that case.
• Change Jetty's client to enforce HTTP/1.1 for insecure requests with the http scheme. This can be still overridden with the Request.version(HttpVersion) manual protocol upgrade method that I've added if we want to explicitly use H2C for insecure connections. This will make HttpClient work with the HTTP/1.1 endpoints without a need for the manual protocol negotiation.
• Add a HTTP/2 support for secure connections along with the ALPN to the server with following protocols preference: SslConnectionFactory(sslContext, alpn.getProtocol()), alpn, http2, http11. This means that first ALPN is used to negotiate protocol. If client doesn't support ALPN it will use HTTP/1.1. If it supports ALPN - it will negotiate protocol in the order of the preference. For Jetty client it will be HTTP/2 since it's preferred.
• Enable HTTP/2 support for the server by default (with a kill-switch httpBinder(binder).disableHttp2()). That means that the default Jetty's client configuration will use HTTP/2. If the kill-switch is used, client will fallback to HTTP/1.1.
• Enable HTTP/2 support in the client by default (flip the config flag to true, deprecate it but allow it to be switched off for some time)

Does it sound reasonable @dain @electrum @martint ?

cc @mosabua for visibility

[electrum]

This sounds reasonable to me -- summarized as remove H2C and make HTTP/2 the default on both client and server.

My only comment is that for the server side, we should also have an enable flag for HTTP/2 rather than a binder, since this something an administrator would control -- not something that the application code would depend on.

[mosabua]

Related to that I need some input and review on #22166

Once that is in place we can also expand to add the http/2 config stuff in there.. and the existing http-client page. And probably also stuff in the internal communication page.

[mosabua]

Questions -

how do we enable http/2 by default for internal communication?
do we want it to be on default?
do we need a fallback switch?
do we want this as as separate config from server and client
also ... if we enable http/2 for http-client as default.. how do we test this for the various usages of client .. (I assume we will be able to disable per prefix)
How does this affect the cert stuff?

[dain]

how do we enable http/2 by default for internal communication?
do we want it to be on default? do we need a fallback switch?

Good questions. @wendigo I assume we would want to use this by default when internal-communication.https.required is set. Basically we would always use http/2 when https is enabled, or do we still want the internal-communication.http2.enabled flag?
In general, I'd like move to always using http/2 internally because the multiplexed nature means we can use long polling everywhere without worrying about running out of sockets. That would mean we move to always using https internally. I'm less concerned about this now that we have the auto-keying feature.

[wendigo]

@dain with the implementation that I've added in airlift we can have HTTP/2 always on for both secure and insecure internal communication. We don't need https always on for that.

[mosabua]

So are we plunging right into using it without a switch to disable @wendigo @dain ? And this is for internal communication.. what about other HTTP usage

[mosabua]

This will definitely need a release notes entry @colebow for 451

[wendigo]

Merged to master.