Skip to content

Conversation

@redox
Copy link

@redox redox commented Jul 23, 2025

Purpose

This PR ensures a single Faraday HTTP client is used per Trino::Client instance.

Overview

For now, each start, resume or kill query would instantiate a new Faraday client by calling Trino::Client.faraday_client at the query level.

This PR refactors the logic of the Trino::Client to ensure a single Faraday client is used per instance of Trino::Client and set all the query (and user) specific HTTP headers at query time. This avoids the need to reconnect to the Trino server dealing with all the SSL handshake processing time at each query + allow the usage of faraday-net_http_persistent to benefit from HTTP keep-alive.

While the benefit of saving a few ms at each Trino statement query might not be that visible for the "long running queries" use-cases (where the actual Trino query anyway takes up to a few seconds to answer), we've seen large improvements for use-cases where the queries are taking a few dozens of ms.

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary

For now, each `start`, `resume` or `kill` query would instantiate a new
`Faraday` client by calling `Trino::Client.faraday_client` at the query
level.

This PR refactors the logic of the `Trino::Client` to ensure a single `Faraday`
client is used per instance of `Trino::Client` and set all the query
(and user) specific HTTP headers at query time. This avoids the need to
reconnect to the Trino server dealing with all the SSL handshake
processing time at each query + allow the usage of
[faraday-net_http_persistent](https://github.com/lostisland/faraday-net_http_persistent)
to benefit from HTTP keep-alive.

While the benefit of saving a few ms at each Trino statement query
might not be that visible for the "long running queries" use-cases (where the
actual Trino query anyway takes up to a few seconds to answer), we've seen large
improvements for use-cases where the queries are taking a few dozens of
ms.
@redox redox requested a review from a team as a code owner July 23, 2025 15:41
@github-actions github-actions bot added the doc label Jul 23, 2025
@redox redox force-pushed the su/single-faraday-client branch from 1c08bf4 to 58561ed Compare July 23, 2025 15:48
@yuokada
Copy link
Collaborator

yuokada commented Aug 4, 2025

@redox Thanks for your contribution. Could you check and fix the errors reported on GitHub Actions?

@redox
Copy link
Author

redox commented Aug 4, 2025

@redox Thanks for your contribution. Could you check and fix the errors reported on GitHub Actions?

Yeah not sure why this started complaining but I'm adding the libcurl4-openssl-dev deps which should solve this. Never easy to iterate as for every single commit I add to the PR it requires your approval to run the CI pipeline :/

@yuokada
Copy link
Collaborator

yuokada commented Aug 5, 2025

Could you reformat files with the command below?

bundle exec standardrb --fix

@redox
Copy link
Author

redox commented Aug 5, 2025

Could you reformat files with the command below?

Gosh of course!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants