press icon indicating copy to clipboard operation
press copied to clipboard

feat: add support to record performance report of database

Open tanmoysrt opened this issue 1 year ago • 3 comments

Related #723

Required agent PR https://github.com/frappe/agent/pull/129

Changes -

  • Added fetch_performance_report method in Database Server doctype
  • Added Performance Report doctype which will contain performance schema reports
  • Added all the reports (available in mysql workbench) as child table of Performance Report
  • Anyone can enable/disable specific performance report from `Database Server > Mariadb Settings"
  • Added cronjob to fetch reports every 15 minutes

Preview

https://github.com/frappe/press/assets/57363826/29ff4bed-484f-4f3e-aedd-0079d419dc63

tanmoysrt avatar Jan 29 '24 06:01 tanmoysrt

Codecov Report

Attention: Patch coverage is 3.34928% with 202 lines in your changes are missing coverage. Please review.

Project coverage is 39.50%. Comparing base (3e2f95b) to head (7ad47d9). Report is 32 commits behind head on master.

:exclamation: Current head 7ad47d9 differs from pull request most recent head 559984b

Please upload reports for the commit 559984b to get more accurate results.

Files Patch % Lines
...s/press/doctype/database_server/database_server.py 5.88% 112 Missing :warning:
...ctype/global_waits_by_time/global_waits_by_time.py 0.00% 3 Missing :warning:
...r_stats_by_schema/innodb_buffer_stats_by_schema.py 0.00% 3 Missing :warning:
...fer_stats_by_table/innodb_buffer_stats_by_table.py 0.00% 3 Missing :warning:
...a/doctype/performance_report/performance_report.py 0.00% 3 Missing :warning:
...schema_index_statistics/schema_index_statistics.py 0.00% 3 Missing :warning:
...schema_table_statistics/schema_table_statistics.py 0.00% 3 Missing :warning:
...ffer/schema_table_statistics_with_innodb_buffer.py 0.00% 3 Missing :warning:
...a/doctype/statement_analysis/statement_analysis.py 0.00% 3 Missing :warning:
...time/statements_in_highest_5_percent_by_runtime.py 0.00% 3 Missing :warning:
... and 21 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1395      +/-   ##
==========================================
- Coverage   39.73%   39.50%   -0.23%     
==========================================
  Files         331      347      +16     
  Lines       26996    25096    -1900     
==========================================
- Hits        10726     9915     -811     
+ Misses      16270    15181    -1089     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 30 '24 07:01 codecov[bot]

@tanmoysrt awesome stuff!

I've some queries and suggestions.

  1. How much time this takes to fetch from DB and roughly what's size of data per day (assuming 15 min interval)
  2. Does this capture events_statemens_summary_by_digest or equivalent digest summaries? (found it to be most useful replacement for slow query logs)
  3. If you implement summary by digest you'll need to keep truncating the stats and regenerate it on our end using historical data. The digest has some hard limit (10K queries) after which it stops capturing details and stores them against NULL digest.
  4. Can we setup log clearing so this doctype and all child tables get cleared at certain interval. Ref: https://github.com/frappe/frappe/pull/17159 see how it's done for email queue. We will need fast clearing in raw SQL without touching ORM.

ankush avatar Mar 18 '24 05:03 ankush

  1. It takes 5~6 seconds to fetch from DB and then couple of seconds to fetch from server to press end. Although it may vary for large database as I was testing on just development setup. Although if we start checking Innodb buffer stats, it can slowdown perf schema query + normal queries according to docs
  2. We are not collecting events_statements_summary_by_digest but we can add that. just a little change required. Check this PR, you will find out the sql statements of available reports, that we are fetching from DB
  3. Need to check
  4. Yup, we can setup log clearing, will check the PR.

cc @ankush

tanmoysrt avatar Mar 18 '24 06:03 tanmoysrt