Skip to content

Conversation

@willmostly
Copy link
Contributor

Add a timeout for the backend stats collection query.

Default the stats collection query to use EXECUTE IMMEDIATE by default, with the option to use explicit PREPARE if desired

Description

The stats collection query uses the traditional PREPARE/EXECUTE sequence to collect stats on the number of queued and running queries. There is a 10 second timeout on the PREPARE, however there is no timeout on the EXECUTE phase. This can cause issues on clusters that may be able to process the PREPARE quickly, but not the EXECUTE. This is configured as a Duration:

monitorConfiguration:
   queryTimeout: "10s"

In addition, the default mode has been updated to use EXECUTE IMMEDIATE to skip the prepare step. This has been available since 431. If needed to support earlier versions of Trino, explicit PREPARE can be enabled by setting:

monitorConfiguration:
   explicitPrepare: True

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

* Configure query timeout and use EXECUTE IMMEDIATE for backend stats collection

@cla-bot cla-bot bot added the cla-signed label Jan 2, 2025
@willmostly willmostly force-pushed the will/jdbc-stats-timeout branch from b005207 to 4d3a8a4 Compare January 2, 2025 16:49
@willmostly willmostly force-pushed the will/jdbc-stats-timeout branch from 4d3a8a4 to 69d1e4d Compare January 7, 2025 14:49
@willmostly willmostly requested a review from oneonestar January 7, 2025 21:50
Copy link
Member

@oneonestar oneonestar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Just want to confirm my understanding. Is this correct?
explicitPrepare is True by default in Trino JDBC.
explicitPrepare is False by default in Trino Gateway.

@willmostly
Copy link
Contributor Author

LGTM.

Just want to confirm my understanding. Is this correct? explicitPrepare is True by default in Trino JDBC. explicitPrepare is False by default in Trino Gateway.

@oneonestar your understanding is correct. Trino JDBC is being more conservative about preserving compatibility with older versions by default. I think it is a nice optimization since it reduces the number of health check queries by a factor of 2, and it has been available for >1 year now. But I could easily be convinced to default it to True if you think we should go the safe route

@willmostly willmostly force-pushed the will/jdbc-stats-timeout branch from 69d1e4d to 80fe966 Compare January 9, 2025 02:37
@willmostly willmostly requested a review from ebyhr January 9, 2025 02:38
@oneonestar
Copy link
Member

But I could easily be convinced to default it to True if you think we should go the safe route

Unlike clients, admins have full control over the version of Trino and Trino Gateway(and the JDBC driver embedded). I think it's okay to use 'False' as the default in the Trino Gateway. Maybe consider adding some docs.

Default the stats collection query to use EXECUTE IMMEDIATE by default,
with the option to use explicit PREPARE if desired
@willmostly willmostly force-pushed the will/jdbc-stats-timeout branch from 80fe966 to 160cc96 Compare January 9, 2025 18:07
@willmostly
Copy link
Contributor Author

@oneonestar documentation added

@willmostly willmostly requested a review from ebyhr January 10, 2025 15:04
@willmostly willmostly force-pushed the will/jdbc-stats-timeout branch from 160cc96 to 662314d Compare January 14, 2025 16:19
@willmostly willmostly merged commit 64dcbd5 into trinodb:main Jan 14, 2025
2 checks passed
@willmostly willmostly deleted the will/jdbc-stats-timeout branch January 14, 2025 18:41
@github-actions github-actions bot added this to the 14 milestone Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants