Skip to content

feat(postgres): support query_timeout dialect option#13258

Merged
sdepold merged 3 commits into
sequelize:mainfrom
sincerekamal:main
Oct 17, 2021
Merged

feat(postgres): support query_timeout dialect option#13258
sdepold merged 3 commits into
sequelize:mainfrom
sincerekamal:main

Conversation

@sincerekamal
Copy link
Copy Markdown
Contributor

Pull Request check-list

Please make sure to review and check all of these items:

  • Does npm run test or npm run test-DIALECT pass with this change (including linting)?
  • Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
  • Have you added new tests to prevent regressions?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Did you update the typescript typings accordingly (if applicable)?
  • Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description of change

In this PR, I have added enabled the dialectOptions to override the postgres' query_timeout option. pg will close the query request when the query runs more than the configured time limit (in ms).

Copy link
Copy Markdown
Member

@papb papb left a comment

Choose a reason for hiding this comment

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

Hello! I see you are a first-time contributor, thank you for taking the time to help Sequelize! I hope to see more PRs from you in the future!

Can you fix the linting issues? Otherwise this PR is good to be merged :)

@papb papb changed the title Add query_timeout option to postgres dialectOptions feat(postgres): support query_timeout dialect option Jun 26, 2021
@papb papb added dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action type: feature DEPRECATED: replace with the "feature" issue type labels Jun 26, 2021
@sincerekamal
Copy link
Copy Markdown
Contributor Author

@papb I have resolved the lint issues

@github-actions github-actions Bot removed the status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action label Jun 26, 2021
@sincerekamal sincerekamal requested a review from papb July 9, 2021 18:21
@sdepold
Copy link
Copy Markdown
Member

sdepold commented Oct 17, 2021

Thanks for your contribution!

@sdepold sdepold merged commit 3ca085d into sequelize:main Oct 17, 2021
@github-actions
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 6.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

aliatsis pushed a commit to creditiq/sequelize that referenced this pull request Jun 2, 2022
* Add query_timeout option to postgres dialectOptions

* fix: lint issues in the test file

Co-authored-by: Kamalakannan Jayaraman <kamalakannan.j@brillio.com>
Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com>
harrykao added a commit to harrykao/sequelize that referenced this pull request 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
WikiRik added a commit that referenced this pull request 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 pull request 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
WikiRik pushed a commit that referenced this pull request 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

Labels

dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). released type: feature DEPRECATED: replace with the "feature" issue type

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants