8000 feat: Set default SQL statement timeouts by ankush · Pull Request #18771 · frappe/frappe · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: Set default SQL statement timeouts #18771

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

Merged
merged 2 commits into from
Nov 8, 2022
Merged

Conversation

ankush
Copy link
Member
@ankush ankush commented Nov 5, 2022

In "SOME" cases queries keep running even after client has disconnected or died.

If you have sent some crazy queries they can keep running for long time and wreak havoc.

This is a MySQL bug, which will likely never get fixed because as per mysql devs:

In my understanding (I may be wrong):

  • such behaviour is by design;
  • such behaviour is common for most socket programs;
  • the easiest way to work around such problem is to use TCP keepalive feature.

Reference: https://bugs.mysql.com/bug.php?id=78809

WARNING: Experimental and internal to framework, might get removed! 🤷

To enable set enable_db_statement_time to 1 in site or common config.

TODO:

  • Manual testing a bit
  • Is this REALLY required in postgres (?) yes
  • Does this add any significant overhead? Doesn't seem to.
  • Fixing PG tests
  • Disable by default, enable with config for now

Alternative: Suggest setting these globally in config files (?)

closes #18768

no-docs - not user facing.

@ankush ankush added defer backport Backports for some PR are deferred for a week or two to test them properly before releasing Run UI Tests Runs Server CI builds even if frontend changes not detected. labels Nov 5, 2022
@codecov
Copy link
codecov bot commented Nov 5, 2022

Codecov Report

Merging #18771 (54c2540) into develop (62c4a3c) will increase coverage by 0.08%.
The diff coverage is 80.64%.

❗ Current head 54c2540 differs from pull request most recent head f456562. Consider uploading reports for the commit f456562 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #18771      +/-   ##
===========================================
+ Coverage    63.53%   63.62%   +0.08%     
===========================================
  Files          748      748              
  Lines        67375    67900     +525     
  Branches      6011     6011              
===========================================
+ Hits         42810    43202     +392     
- Misses       21148    21286     +138     
+ Partials      3417     3412       -5     
Flag Coverage Δ
server-mariadb 67.30% <70.00%> (-0.03%) ⬇️
server-ui 31.57% <43.33%> (-0.05%) ⬇️
ui-tests 51.18% <ø> (+0.22%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@ankush ankush marked this pull request as ready for review November 7, 2022 07:55
@ankush ankush requested review from a team and shariquerik and removed request for a team November 7, 2022 07:55
@ankush ankush requested a review from adityahase November 7, 2022 07:58
@ankush ankush merged commit 3ea75f3 into frappe:develop Nov 8, 2022
@ankush ankush deleted the query_timeout branch November 8, 2022 09:47
@ankush ankush added backport version-13-hotfix backport version-14-hotfix backport to version 14 and removed defer backport Backports for some PR are deferred for a week or two to test them properly before releasing labels Nov 8, 2022
ankush added a commit that referenced this pull request Nov 8, 2022
…-18771

feat: Set default SQL statement timeouts (backport #18771)
ankush added a commit that referenced this pull request Nov 9, 2022
Co-authored-by: Ankush Menat <ankush@frappe.io>
frappe-pr-bot pushed a commit that referenced this pull request Nov 15, 2022
# [13.44.0](v13.43.2...v13.44.0) (2022-11-15)

### Bug Fixes

* page has an empty menu button ([637cb45](637cb45))
* reportview permlevel bug (backport [#18822](#18822)) ([#18827](#18827)) ([acb6f57](acb6f57))
* reset workspace ([271c5d0](271c5d0))
* **security:** prevent xss attack in search ([#18847](#18847)) ([#18849](#18849)) ([8b42091](8b42091))

### Features

* Set default SQL statement timeouts (backport [#18771](#18771)) ([#18800](#18800)) ([127763a](127763a))

### Performance Improvements

* **workflow:** get_transitions ([#18834](#18834)) ([9ec557d](9ec557d))
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14 Run UI Tests Runs Server CI builds even if frontend changes not detected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set max execution time to sql queries based on request/job timeout
1 participant
0