Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Global configurable timeouts #862

Open
jalaziz opened this issue Apr 2, 2018 · 7 comments
Open

Global configurable timeouts #862

jalaziz opened this issue Apr 2, 2018 · 7 comments

Comments

@jalaziz
Copy link

jalaziz commented Apr 2, 2018

Currently, it seems that the only way to set a query timeout is to set the query timeout on each query or to capture the executor and cancel the underlying statement after a timeout.

It would be great if a default query timeout could be configured globally.

This could use the setQueryTimeout option, but it may be possible to handle this at the execution layer to avoid issues with drivers not supporting setQueryTimeout. I have not dug too deep into the scalikejdbc source code, but from what I have see, it seems the former may be easier.

@seratch
Copy link
Member

seratch commented Apr 15, 2018

Having a global setting for it may be okay. However, I am not willing to have it so far. If you're using MySQL / PostgreSQL, both of them support socketTimeout parameter in JDBC URL. In my opinion, using it would be enough for most cases.

@jalaziz
Copy link
Author

jalaziz commented Apr 15, 2018

The issue with socketTimeout is that it's really a last resort. You end up killing the connection instead of just the query. Everything I've read suggests using the query timeout as first line of defense and socketTimeout as a last line of defense.

@seratch
Copy link
Member

seratch commented Apr 16, 2018

I see. Fair enough.

@seratch
Copy link
Member

seratch commented Apr 16, 2018

If we have the setting globally, having it per a connection pool would be straight-forward.

I have one concern about the possible implementation. ConnectionPoolSettings already has connectionTimeoutMillis to set timeout duration in milliseconds while queryTimeout in JDBC is in seconds. I am not comfortable with the inconsistency, but it would be acceptable for many people if we name it as queryTimeoutSeconds.

@jalaziz
Copy link
Author

jalaziz commented Apr 16, 2018

Yeah, I agree, the inconsistency is annoying. queryTimeoutSeconds is reasonable though.

I can try to take a stab at this if it doesn't require any major refactoring.

@seratch seratch added the core label Sep 5, 2018
@nanajia984
Copy link

how is going with queryTimeoutSeconds? we need that feature as well

@seratch
Copy link
Member

seratch commented Feb 20, 2020

We haven't implemented it yet. I'm open to pull requests from the community!

By the way, I revisited this now and came to wonder if there is an equivalent setting in HikariCP. https://github.com/brettwooldridge/HikariCP

The Commons DBCP has defaultQueryTimeout. http://commons.apache.org/proper/commons-dbcp/configuration.html

If HikariCP doesn't have such, there is no other way to ignore the queryTimeout value with it (we can output warnings tho).

@seratch seratch added this to the version 4.1.x milestone Jun 12, 2021
@seratch seratch modified the milestones: version 4.1.x, version 4.2.x Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants