feat: Support configuring the external endpoint of Trino clusters for ackUri rewriting #100

sbernauer · 2026-01-14T08:54:46Z

Part of #97

Additionally, sometimes the trino-client sends a HEAD request, which we need to proxy as HEAD (ant not GET) as well:

… ackUri rewriting

…ndpoint

sbernauer · 2026-01-14T10:51:11Z

Pushed the current version as oci.stackable.tech/sandbox/sbernauer/trino-lb:0.0.0-pr100

sbernauer · 2026-01-20T10:04:36Z

Updated oci.stackable.tech/sandbox/sbernauer/trino-lb:0.0.0-pr100 with the current state of the PR

sbernauer · 2026-01-30T16:27:37Z

Customer confirmed this solves their issues :)

trino-lb-core/src/trino_api.rs

Techassi · 2026-02-02T10:48:49Z

trino-lb-core/src/trino_api.rs

+        trino_lb_addr: &Url,
+        external_trino_addr: Option<&Url>,


question: Why are these parameters called *_addr if they contain URLs? I think their names should be updated to reflect this.

See 54b235d

Techassi · 2026-02-02T10:50:51Z

trino-lb-core/src/trino_api.rs

+    ) -> Result<(), Error> {
+        // Point nextUri to trino-lb
+        if let Some(next_uri) = &self.next_uri {
+            let next_uri = Url::parse(next_uri).context(ParseNextUriFromTrinoSnafu)?;


note: Immediately parsing this as a URL when deserializing TrinoQueryApiResponse might be a better idea and also gets rid of the to_string call below.

See ee5a8d4 and 875fb24

Techassi · 2026-02-02T10:53:37Z

trino-lb-core/src/trino_api.rs

+#[instrument(
+    fields(next_uri = %next_uri, trino_lb_addr = %trino_lb_addr),
+)]
 fn change_next_uri_to_trino_lb(next_uri: &Url, trino_lb_addr: &Url) -> Url {


note: We should not take &Url for trino_lb_addr here if the first thing we do is to clone it.

The trino_lb_addr comes directly from the pared YAML config of trino-lb, so we always need to clone it.
This way it's lazy and we don't eagerly clone it somewhere up the chain and don't actually use it.
But I don't really care, so picked in f40cabd ;)

Techassi · 2026-02-02T10:57:19Z

trino-lb-core/src/trino_query.rs

    /// Endpoint of the Trino cluster the query is running on.
    pub trino_endpoint: Url,

+    /// (Optionally, if configured) public endpoint of the Trino cluster.


suggestion: Slight reword. The fact that this option is optional implies that it only takes effect when configured.

Suggested change

/// (Optionally, if configured) public endpoint of the Trino cluster.

/// Optional public endpoint of the Trino cluster.

I prefer my variant as it makes it a bit more clear.
Every Trino always has a public endpoint. The question is, if trino-lb was configured to know about it or not

Techassi · 2026-02-02T11:03:02Z

trino-lb-persistence/src/postgres/mod.rs

+                Url::parse(&trino_external_endpoint)
+                    .context(ParseClusterExternalEndpointFromStoredQuerySnafu)?,


note: Such a bummer that have to do this every time. Can we maybe natively support Url to be used as part of query!?

Is this the same problem as in #100 (comment)?

Techassi · 2026-02-02T11:06:27Z

trino-lb/src/http_server/v1/statement.rs

+/// special handling.
 #[instrument(
-    name = "GET /v1/statement/executing/{queryId}/{slug}/{token}",
+    name = "GET (or HEAD) /v1/statement/executing/{queryId}/{slug}/{token}",


note: Ideally we set this to the appropriate HTTP method in the function itself, but I'm unsure if this is supported (without using the special otel.name field/attribute).

Techassi · 2026-02-02T11:08:29Z

trino-lb/src/http_server/v1/statement.rs

+    name = "GET (or HEAD) /v1/statement/executing/{queryId}/{slug}/{token}",
    skip(state, headers)
 )]
 pub async fn get_trino_executing_statement(


note: This function needs to be renamed because it now now only handles GET requests, but HEAD as well.

That was the very surprising implementation details of axum, which caused the "bug" in the first place.
Even if you register it via get(), is still get's HEADs.
We register the fn with

.route( "/v1/statement/executing/{query_id}/{slug}/{token}", get(v1::statement::get_trino_executing_statement), )

We do this many times with lot's of functions, so we would need to change all of them.
And I currently find the registration code readable

Techassi · 2026-02-02T11:09:55Z

trino-lb/src/http_server/v1/statement.rs

    uri: Uri,
-) -> Result<(HeaderMap, Json<TrinoQueryApiResponse>), Error> {
+) -> Result<Response, Error> {
+    if method == http::Method::HEAD {


note: I would like to see a match here instead to explicitly handle GET and HEAD and error/warn on all other methods.

note: It might additionally also make sense to move GET and HEAD code into separate functions respectively.

Moved to match in e9b150b

Techassi · 2026-02-02T11:12:19Z

trino-lb/src/http_server/v1/statement.rs

    Ok((trino_headers, Json(trino_query_api_response)))
 }

+async fn handle_head_request_to_trino(


note: I think this function is poorly named. It implies it handles the complete request and response while it only returns a list of headers. This should be made clear.

I mean there is really nothing to return except the headers, as HEAD requests never return a body ^^
The handle_query_running_on_trino fn also only returns the headers and the Json from the body.

Co-authored-by: Techassi <git@techassi.dev>

sbernauer added 6 commits January 14, 2026 08:49

feat: Support configuring the external endpoint of Trino clusters for…

0f67ea2

… ackUri rewriting

Merge remote-tracking branch 'origin/main' into feat/trino-external-e…

d17d00e

…ndpoint

Remove unnecessary file

f72c9ad

changelog

ec1c67a

fix trailing whitespace

7c17b4e

markdown linter

26e2840

sbernauer and others added 3 commits January 16, 2026 12:30

Merge branch 'main' into feat/trino-external-endpoint

e761bb1

fix: Correctly proxy HEAD requests

2615b9c

fix: Only append HTTP headers

5557722

sbernauer self-assigned this Jan 21, 2026

sbernauer added this to Stackable Engineering Jan 21, 2026

sbernauer moved this to Development: In Progress in Stackable Engineering Jan 21, 2026

sbernauer moved this from Development: In Progress to Development: Track in Stackable Engineering Jan 26, 2026

sbernauer moved this from Development: Track to Development: Waiting for Review in Stackable Engineering Jan 30, 2026

Techassi moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Feb 2, 2026

Techassi requested changes Feb 2, 2026

View reviewed changes

sbernauer and others added 6 commits February 3, 2026 13:55

Update trino-lb-core/src/trino_api.rs

cfc1ede

Co-authored-by: Techassi <git@techassi.dev>

Serde as Url directly (instead as String)

ee5a8d4

Remove unused error

875fb24

Pass Url instead of &Url

f40cabd

Convert HTTP method handling to match

e9b150b

external_trino_addr -> external_trino_endpoint

54b235d

sbernauer requested a review from Techassi February 5, 2026 11:55

	/// (Optionally, if configured) public endpoint of the Trino cluster.
	/// Optional public endpoint of the Trino cluster.

		Url::parse(&trino_external_endpoint)
		.context(ParseClusterExternalEndpointFromStoredQuerySnafu)?,

Uh oh!

feat: Support configuring the external endpoint of Trino clusters for ackUri rewriting #100

Are you sure you want to change the base?

feat: Support configuring the external endpoint of Trino clusters for ackUri rewriting #100

Uh oh!

Conversation

sbernauer commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbernauer commented Jan 14, 2026

Uh oh!

sbernauer commented Jan 20, 2026

Uh oh!

sbernauer commented Jan 30, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbernauer Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sbernauer commented Jan 14, 2026 •

edited

Loading

sbernauer Feb 3, 2026 •

edited

Loading