-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Feature: query timeout #1713
Comments
Happy to see that ! Do you plan to add tests and send a PR ? |
Yes, please! |
I'm definitely down for a pull request for this feature. I think it'd be helpful & something the library should provide since it's pretty difficult to implement in your own code. |
Same problem with me, my temporaly solution:
|
Can this be closed now or there is still a problem with the sockets? |
yay! |
harrykao
added a commit
to harrykao/sequelize
that referenced
this issue
Oct 17, 2022
The `query_timeout` feature of the `pg` package helps handle stuck TCP connections more quickly and gracefully by implementing a client-side timeout: brianc/node-postgres#1713 Sequelize started passing this dialect-specific option through to `pg` here: sequelize#13258 I believe we also want to invalidate the connection when a client-side timeout occurs. We shouldn't try to reuse the stuck connection because...it's stuck. This PR updates the error handling code so that the connection is invalidated if the error matches the one thrown from here: https://github.com/brianc/node-postgres/blob/5538df6b446f4b4f921947b460fe38acb897e579/packages/pg/lib/client.js#L529
5 tasks
WikiRik
added a commit
to sequelize/sequelize
that referenced
this issue
Oct 24, 2022
* Invalidate connection after client-side timeout. The `query_timeout` feature of the `pg` package helps handle stuck TCP connections more quickly and gracefully by implementing a client-side timeout: brianc/node-postgres#1713 Sequelize started passing this dialect-specific option through to `pg` here: #13258 I believe we also want to invalidate the connection when a client-side timeout occurs. We shouldn't try to reuse the stuck connection because...it's stuck. This PR updates the error handling code so that the connection is invalidated if the error matches the one thrown from here: https://github.com/brianc/node-postgres/blob/5538df6b446f4b4f921947b460fe38acb897e579/packages/pg/lib/client.js#L529 * Add comment with link to PR. * Add tests, shorten comment. * Skip tests for postgres-native. * Update src/dialects/postgres/query.js Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com> Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
harrykao
added a commit
to harrykao/sequelize
that referenced
this issue
Nov 15, 2022
Merge ff43e8d from main: The `query_timeout` feature of the `pg` package helps handle stuck TCP connections more quickly and gracefully by implementing a client-side timeout: brianc/node-postgres#1713 Sequelize started passing this dialect-specific option through to `pg` here: sequelize#13258 I believe we also want to invalidate the connection when a client-side timeout occurs. We shouldn't try to reuse the stuck connection because...it's stuck. This PR updates the error handling code so that the connection is invalidated if the error matches the one thrown from here: https://github.com/brianc/node-postgres/blob/5538df6b446f4b4f921947b460fe38acb897e579/packages/pg/lib/client.js#L529
8 tasks
WikiRik
pushed a commit
to sequelize/sequelize
that referenced
this issue
Nov 15, 2022
* fix(postgres): invalidate connection after client-side timeout Merge ff43e8d from main: The `query_timeout` feature of the `pg` package helps handle stuck TCP connections more quickly and gracefully by implementing a client-side timeout: brianc/node-postgres#1713 Sequelize started passing this dialect-specific option through to `pg` here: #13258 I believe we also want to invalidate the connection when a client-side timeout occurs. We shouldn't try to reuse the stuck connection because...it's stuck. This PR updates the error handling code so that the connection is invalidated if the error matches the one thrown from here: https://github.com/brianc/node-postgres/blob/5538df6b446f4b4f921947b460fe38acb897e579/packages/pg/lib/client.js#L529 * fix syntax error * fix another syntax error * fix import
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I want to start a discussion about built in query timeout.
In our production environment, we encountered a problem with TCP connections getting stuck. Knex pool would fill up with connections that are not responsive. Note that this would be different from statement timeout, where DB decides to terminate query. Instead pg client would decide that connection is unresponsive and emit an error.
This situation can be simulated with firewall rule:
Or just use this code snippet:
A possible implementation can be found here: master...juliusza:query_timeout
I know I need to add tests and also edit the native driver code for a proper PR. But first I'm looking for feedback from the community to assert if such a thing would be useful.
Also perhaps
could be a viable solution. I need to look into that.
The text was updated successfully, but these errors were encountered: