Migrate Jetty Proxy to Airlift

[oneonestar]

Description

Second part of Airlift migration #41. Depends on #317.
This PR migrate Jetty Proxy (ie. ProxyHandler and ProxyServletImpl) to Airlift.

Motivation

• Fix Can't access application after configure SSL #242. Merge requestRouter, applicationConnectors and adminConnectors ports into one single port managed by Airlift. The TLS issues caused by routing between these ports will be gone.
• Better control of the proxy request and response logic. The old implementation used Jetty ProxyServlet and overwrote different methods, making it difficult to add new features. By handling the request and response manually, we have full control of the request/response flow. This makes implementing query retries or query result caching much easier.
• All configurations related to networking and TLS are done by Airlift config, which aligns with Trino. For example, the following config enable TLS for gateway:

httpConfig:
    http-server.http.enabled: false
    http-server.https.enabled: true
    http-server.https.port: 9080
    http-server.https.keystore.path: cert.jks
    http-server.https.keystore.key: changeme
    node.environment: test

TODO

• Update docs on how to config & run
• Document the incompatible changes and provide a migration guide.

Release notes

( ) Release notes are required, with the following suggested text:

* Replace Jetty Proxy with Airlift
* (TODO) Document the incompatible changes

[mosabua]

I merged #317 now. Once you rebase here you might be able to address my comment about the config here. I think we should end up with the same config as in Trino ideally. So

jvm.config
config.properties
node.properties (maybe not even necessary)
log.properties

If we leave the http config in this PR in the YAML file for now I am good with that but maybe we can already have all of them under one config node or so

[oneonestar]

• jvm.config
  • This is read by launcher.py. We can't set JVM config the config file unless we write our own version of launcher.py which supports YAML.
• node.properties (maybe not even necessary)
  • Similar to above, launcher.py read this file and prepare the directories and symlinks. We can set log.output-file in httpConfig without using node.properties.
• log.properties
  • Airlift expects this as a file LoggingConfiguration.java. We can allow setting log level in YAML, read the YAML, write them to a properties file and sent the file path to Airlift. I'm not sure if we want to do this.
• config.properties
  • Everything that can be set in config.properties for Airlift can be set in httpConfig.

[mosabua]

• jvm.config

  • This is read by launcher.py. We can't set JVM config the config file unless we write our own version of launcher.py which supports YAML.

Yeah.. we can do that later .. ideally NOT with a python requirement.

• node.properties (maybe not even necessary)

  • Similar to above, launcher.py read this file and prepare the directories and symlinks. We can set log.output-file in httpConfig without using node.properties.

• log.properties

Agreed

• Airlift expects this as a file LoggingConfiguration.java. We can allow setting log level in YAML, read the YAML, write them to a properties file and sent the file path to Airlift. I'm not sure if we want to do this.

Probably not in this PR. We will work with the airlift team to figure out how to proceed in terms of supporting yaml or toml or whatever to do next.

• config.properties

  • Everything that can be set in config.properties for Airlift can be set in httpConfig.

Given the scope is way larger than just configuring HTTP I would hope we can come up with a better name.. just config or globalConfig or I dont know..

[oneonestar]

How about serverConfig?

[mosabua]

How about serverConfig?

Thats good with me. @vishalya @willmostly @Chaho12 ?

[Chaho12]

How about serverConfig?

sounds good as it replaces requestRouter & server configs in yaml.

Btw why is historySize and requestBufferSize removed? does gateway now read all data from history?

[oneonestar]

For historySize, AFAIK we never use this value. No code is using this value since the initial git commit.

The equivalent for requestBufferSize is http-client.request-buffer-size in Airlift. Let me update the doc for that.

[wendigo]

@mosabua Let's merge it and improve in follow ups

[mosabua]

We decided in team sync today that we will merge early next week to give @vishalya and others a chance to look.

[wendigo]

@mosabua sure, I'd prefer to merge it now and improve it right away but I'm fine with delaying that

[mosabua]

@mosabua sure, I'd prefer to merge it now and improve it right away but I'm fine with delaying that

Monday ;-)

[wendigo]

@mosabua ages :P

[vishalya]

The changes look good. I was able to run it locally with the instructions provided on the PR.
So I am with merging this PR.

[mosabua]

Alright .. lets ship this. Anybody .. please send any follow up fixes as new PRs.

[mosabua]

Also .. @vishalya .. please also approve the PR.

[mosabua]

Here you go @wendigo and @oneonestar .. it hit earlier than Monday after all ;-)

[v2kk]

Hi guys, the current version of

trino-gateway/gateway-ha/src/main/java/io/trino/gateway/proxyserver/RouteToBackendResource.java

Line 56 in 4553893

@POST

does not handle proxy for DELETE method, so that client can not cancel query on Trino backend, it causes queries submitted from airflow will be failed