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

Database: Rewrite system statistics query to perform better #19178

Merged
merged 3 commits into from
Sep 19, 2019

Conversation

papagian
Copy link
Contributor

What this PR does / why we need it:
This fix modifies the statistics queries for addressing efficiency issues following to this suggestion.

Which issue(s) this PR fixes:

Fixes #19137

Special notes for your reviewer:
Opening this as a draft since I'm not still able to evaluate the performance; I need to populate my database with more users and roles.

@papagian papagian changed the title Rewrite statistics query SQL: Rewrite statistics query Sep 18, 2019
@papagian
Copy link
Contributor Author

The execution plans for the old and the new query are:

mysql> EXPLAIN SELECT COUNT(*) FROM `user` as u WHERE (SELECT COUNT(*) FROM org_user  WHERE org_user.user_id=u.id AND org_user.role='Admin')>0;
+----+--------------------+----------+-------+---------------+----------------+---------+------+------+--------------------------+
| id | select_type        | table    | type  | possible_keys | key            | key_len | ref  | rows | Extra                    |
+----+--------------------+----------+-------+---------------+----------------+---------+------+------+--------------------------+
|  1 | PRIMARY            | u        | index | NULL          | UQE_user_login | 762     | NULL |    1 | Using where; Using index |
|  2 | DEPENDENT SUBQUERY | org_user | ALL   | NULL          | NULL           | NULL    | NULL |    1 | Using where              |
+----+--------------------+----------+-------+---------------+----------------+---------+------+------+--------------------------+
2 rows in set (0.00 sec)

mysql> EXPLAIN SELECT COUNT(*) FROM `user` as u, org_user WHERE ( org_user.user_id=u.id AND org_user.role='Admin' );
+----+-------------+----------+-------+---------------+----------------+---------+------+------+----------------------------------------------------+
| id | select_type | table    | type  | possible_keys | key            | key_len | ref  | rows | Extra                                              |
+----+-------------+----------+-------+---------------+----------------+---------+------+------+----------------------------------------------------+
|  1 | SIMPLE      | u        | index | PRIMARY       | UQE_user_login | 762     | NULL |    1 | Using index                                        |
|  1 | SIMPLE      | org_user | ALL   | NULL          | NULL           | NULL    | NULL |    1 | Using where; Using join buffer (Block Nested Loop) |
+----+-------------+----------+-------+---------------+----------------+---------+------+------+----------------------------------------------------+
2 rows in set (0.00 sec)


@papagian papagian marked this pull request as ready for review September 19, 2019 08:22
@marefr marefr self-requested a review September 19, 2019 10:01
Copy link
Member

@marefr marefr left a comment

Choose a reason for hiding this comment

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

Great 👍

@marefr marefr added this to the 6.3.6 milestone Sep 19, 2019
@papagian papagian merged commit 56f5106 into master Sep 19, 2019
@papagian papagian deleted the 19137-rewrite-stats-query branch September 19, 2019 11:15
@papagian papagian changed the title SQL: Rewrite statistics query Database: Rewrite system statistics query to perform better Sep 19, 2019
papagian added a commit that referenced this pull request Sep 19, 2019
* Rewrite statistics query

(cherry picked from commit 56f5106)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Sep 19, 2019
* grafana/master: (39 commits)
  LDAP: Show non-matched groups returned from LDAP (grafana#19208)
  plugins: expose whole rxjs to plugins (grafana#19226)
  SQL: Rewrite statistics query (grafana#19178)
  CI: Update frontend ci metrics for strict null checks
  grafana/ui: Add disabled prop on LinkButton (grafana#19192)
  Cloudwatch: Fix autocomplete for Gamelift dimensions (grafana#19145) (grafana#19146)
  Backend: Remove redundant condition of `ROLE_VIEWER` (grafana#19211)
  FieldDisplay: Update title variable syntax (grafana#19217)
  Docs: Note when using For and No Data in alert rule (grafana#19185)
  Docker: Upgrade packages to resolve reported vulnerabilities (grafana#19188)
  MSSQL: Revert usage of new connectionstring format (grafana#19203)
  Prometheus: datasource config with custom parameters string (grafana#19121)
  Contributing: Add guidelines for contributing docs (grafana#19108)
  LDAP debug page: deduplicate errors (grafana#19168)
  Menu: fix menu button in the mobile view (grafana#19191)
  Dashboard: Fixes back button styles in kiosk mode (grafana#19165)
  API: adds redirect helper to simplify http redirects (grafana#19180)
  docs: image rendering (grafana#19183)
  Chore: Update latest.json (grafana#19177)
  Chore: Update version to next (grafana#19169)
  ...
papagian added a commit that referenced this pull request Sep 23, 2019
* Rewrite statistics query

(cherry picked from commit 56f5106)
@marefr marefr modified the milestones: 6.3.6, 6.4.0-beta2 Sep 24, 2019
marefr pushed a commit that referenced this pull request Sep 24, 2019
* Rewrite statistics query
(cherry picked from commit 56f5106)
torkelo pushed a commit that referenced this pull request Sep 25, 2019
* API: adds redirect helper to simplify http redirects (#19180)

(cherry picked from commit dd79462)

* Dashboard: Fixes back button styles in kiosk mode (#19165)

Fixes: #18114
(cherry picked from commit 38e948a)

* Menu: fix menu button in the mobile view (#19191)

* replace "sandwich" (menu) button with logo(back home) if kiosk=tv
* update navbar initialize padding-left befause menu button is overlapped by the navbar
(cherry picked from commit 5ef40b2)

* LDAP debug page: deduplicate errors (#19168)

(cherry picked from commit 6b2e95a)

* MSSQL: Revert usage of new connectionstring format (#19203)

This reverts commit 2514209 from #18384. Reason is that it doesn't
work due to xorm 0.7.1 which doesn't support this new connectionstring
format.

Fixes #19189
Ref #18384
Ref #17665
(cherry picked from commit 0f524fc)

* Docker: Upgrade packages to resolve reported vulnerabilities (#19188)

Fixes #19186
(cherry picked from commit 4d96bc5)

* FieldDisplay: Update title variable syntax (#19217)

(cherry picked from commit 14f1cf2)

* Cloudwatch: Fix autocomplete for Gamelift dimensions (#19145) (#19146)

(cherry picked from commit 79f8433)

* grafana/ui: Add disabled prop on LinkButton (#19192)

(cherry picked from commit f445369)

* plugins: expose whole rxjs to plugins (#19226)

(cherry picked from commit 98c95a8)

* Snapshots: store DataFrameDTO instead of MutableDataFrame in snapshot data (#19247)

(cherry picked from commit be8097f)

* grafana/toolkit: Add plugin scaffolding (#19207)

(cherry picked from commit 54ebf17)

* Alerting: Truncate PagerDuty summary when greater than 1024 characters (#18730)

Requests to PagerDuty fail with an HTTP 400 if the `summary`
attribute contains more than 1024 characters, this fixes this.
API spec:
https://v2.developer.pagerduty.com/docs/send-an-event-events-api-v2

Fixes #18727
(cherry picked from commit 8a99124)

* grafana/toolkit: Fix toolkit not building @grafana/toolkit (#19253)

* Fix toolkit not building

Weird TS didn't pick this up...

* Update packages/grafana-toolkit/src/cli/index.ts

(cherry picked from commit 809e2ca)

* Docs: Update theming docs (#19248)

(cherry picked from commit 9feac77)

* Explore: live tail UI fixes and improvements (#19187)

(cherry picked from commit bf24cbb)

* Graphite: Changed range expansion from 1m to 1s (#19246)

Fixes #11472
(cherry picked from commit d95318b)

* MySQL, Postgres, MSSQL: Only debug log when in development (#19239)

Found some additional debug statements in relation to #19049 that
can cause memory issues.

Ref #19049
(cherry picked from commit 19f3ec4)

* Vector: remove toJSON() from interface (#19254)

(cherry picked from commit 6787e7b)

* Update changelog task to generate toolkit changelog too (#19262)

(cherry picked from commit b7752b8)

* Dashboard: Hides alpha icon for visualization that is not in alpha/beta stage #19300

Fixes #19251
(cherry picked from commit f01836c)

* Build: Split up task in the CI pipeline to ease running outside circleci (#18861)

* build: make sign rpm packages not depend on checking out private key

* build: move commands from circleci config into verify signed packages script

* build: split update and publish of deb and rpm into two scripts

* use files argument for sign and verify packages

* validate files argument for sign and verify packages

* update test publish of deb/rpm readme

(cherry picked from commit 4386604)

* Admin/user: fix textarea postion in 'Pending Invites' to avoid page scrolling (#19288)

* hide textarea element after click 'Copy Invite' button on firefox
(cherry picked from commit 50b4695)

* Alerting: Prevents creating alerts from unsupported queries (#19250)

* Refactor: Makes PanelEditor use state and shows validation message on AlerTab

* Refactor: Makes validation message nicer looking

* Refactor: Changes imports

* Refactor: Removes conditional props

* Refactor: Changes after feedback from PR review

* Refactor: Removes unused action

(cherry picked from commit 9bd6ed8)

* Chore: Update Slate to 0.47.8 (#19197)

* Chore: Update Slate to 0.47.8
Closes #17430

(cherry picked from commit 68d6da7)

* DataLinks: Small UX improvements to DataLinksInput (#19313)

Closes #19257
(cherry picked from commit feb6bc6)

* Multi-LDAP: Do not fail-fast on invalid credentials (#19261)

* Multi-LDAP: Do not fail-fast on invalid credentials

When configuring LDAP authentication, it is very common to have multiple
servers configured. When using user bind (authenticating with LDAP using
the same credentials as the user authenticating to Grafana) we don't
expect all the users to be on all LDAP servers.

Because of this use-case, we should not fail-fast when authenticating on
multiple LDAP server configurations. Instead, we should continue to try
the credentials with the next LDAP server configured.

Fixes #19066
(cherry picked from commit 279249e)

* Explore: Fix unsubscribing from Loki websocket (#19263)

(cherry picked from commit 4c1bc59)

* Plugins: Skips existence of module.js for renderer plugins (#19318)

* Fix: Skips test for module.js for plugins of renderer type
Fixes #19130

* Refactor: Changes after PR comments

* Chore: Fixes go lint issue

(cherry picked from commit 75dcaec)

* Keybindings: Improve esc / exit / blur logic (#19320)

* Keybindings: Improve esc / exit / blur logic

* Slight modifications

* removed use of jquery

(cherry picked from commit 08cc4f0)

* Select: Set placeholder color (#19309)

(cherry picked from commit 2c9577f)

* Azure Monitor: Revert support for cross resource queries (#19115)" (#19346)

This reverts commit 8805125.
(cherry picked from commit 4dbedb8)

* Dashboard: Fix export for sharing when panels use default data source (#19315)

* PanelModel: moved datasource: null away from defaults that are removed

* Added unit test

(cherry picked from commit ac3fb64)

* Heatmap: use DataFrame rather than LegacyResponseData (#19026)

* merge master

* TimeSeries: datasources with labels should export tags (not labels) (#18977)

* merge master

* export prometheus tags

* Annotations: Add annotations support to Loki (#18949)

* Explore: Unify background color for fresh logs (#18973)

* Singlestat: render lines on the panel when sparklines are enabled (#18984)

* Image rendering: Add deprecation warning when PhantomJS is used for rendering images (#18933)

* Add deprecation warning

* Update pkg/services/rendering/rendering.go

Co-Authored-By: Marcus Efraimsson <marcus.efraimsson@gmail.com>

* Units: Adding T,P,E,Z,and Y bytes (#18706)

* Adding T and P for bytes

Luckily, all the hard work was done before; just added in these prefixes for our production environment.

* Future-proofing with other values (why not?)

* Yottaflops?

* Cutting back down to Peta sizes, except for hashes

* Refactor: move ScopedVars to grafana/data (#18992)

* Refactor: Move sql_engine to sub package of tsdb (#18991)

this way importing the tsdb package does not come with xorm dependencies

* use DataFrame in heatmaps

* actually use the setting :)

* remove unused timeSrv

* merge with master / useDataFrames

* fix test function

* merge master

* fix datasource type on snapshot

* reuse DataFrame calcs from graph panel

* update comments

(cherry picked from commit 2474511)

* Explore: Do not send explicit maxDataPoints for logs. (#19235)

(cherry picked from commit f203e82)

* MySQL, Postgres, MSSQL: Fix validating query with template variables in alert  (#19237)

Adds support for validating query in alert for mysql,
postgres and mssql.

Fixes #13155
(cherry picked from commit 96046a7)

* MySQL, Postgres: Update raw sql when query builder updates (#19209)

Raw sql now updates when changing query using
graphical query editor for mysql and postgres.

Fixes #19063
(cherry picked from commit 7c499ff)

* MySQL: Limit datasource error details returned from the backend (#19373)

Only return certain mysql errors from backend.
The following errors is returned as is from backend:
error code 1064 (parse error)
error code 1054 (bad column/field selected)
error code 1146 (table not exists)
Any other errors is logged and returned as a generic
error.
Restrict use of certain functions:
Do not allow usage of the following in query:
system_user()
session_user()
current_user() or current_user
user()
show grants

Fixes #19360
(cherry picked from commit 3de693a)

* SQL: Rewrite statistics query (#19178)

* Rewrite statistics query
(cherry picked from commit 56f5106)

* Release v6.4.0-beta2

* ValueFormats: check for inf (#19376)


(cherry picked from commit 32b73bb)

* Build: Fix correct sort order of merged pr's in cherrypick task (#19379)


(cherry picked from commit c4a03f4)
teodoryantcheff pushed a commit to teodoryantcheff/grafana that referenced this pull request Dec 18, 2019
* API: adds redirect helper to simplify http redirects (grafana#19180)

(cherry picked from commit dd79462)

* Dashboard: Fixes back button styles in kiosk mode (grafana#19165)

Fixes: grafana#18114
(cherry picked from commit 38e948a)

* Menu: fix menu button in the mobile view (grafana#19191)

* replace "sandwich" (menu) button with logo(back home) if kiosk=tv
* update navbar initialize padding-left befause menu button is overlapped by the navbar
(cherry picked from commit 5ef40b2)

* LDAP debug page: deduplicate errors (grafana#19168)

(cherry picked from commit 6b2e95a)

* MSSQL: Revert usage of new connectionstring format (grafana#19203)

This reverts commit 2514209 from grafana#18384. Reason is that it doesn't
work due to xorm 0.7.1 which doesn't support this new connectionstring
format.

Fixes grafana#19189
Ref grafana#18384
Ref grafana#17665
(cherry picked from commit 0f524fc)

* Docker: Upgrade packages to resolve reported vulnerabilities (grafana#19188)

Fixes grafana#19186
(cherry picked from commit 4d96bc5)

* FieldDisplay: Update title variable syntax (grafana#19217)

(cherry picked from commit 14f1cf2)

* Cloudwatch: Fix autocomplete for Gamelift dimensions (grafana#19145) (grafana#19146)

(cherry picked from commit 79f8433)

* grafana/ui: Add disabled prop on LinkButton (grafana#19192)

(cherry picked from commit f445369)

* plugins: expose whole rxjs to plugins (grafana#19226)

(cherry picked from commit 98c95a8)

* Snapshots: store DataFrameDTO instead of MutableDataFrame in snapshot data (grafana#19247)

(cherry picked from commit be8097f)

* grafana/toolkit: Add plugin scaffolding (grafana#19207)

(cherry picked from commit 54ebf17)

* Alerting: Truncate PagerDuty summary when greater than 1024 characters (grafana#18730)

Requests to PagerDuty fail with an HTTP 400 if the `summary`
attribute contains more than 1024 characters, this fixes this.
API spec:
https://v2.developer.pagerduty.com/docs/send-an-event-events-api-v2

Fixes grafana#18727
(cherry picked from commit 8a99124)

* grafana/toolkit: Fix toolkit not building @grafana/toolkit (grafana#19253)

* Fix toolkit not building

Weird TS didn't pick this up...

* Update packages/grafana-toolkit/src/cli/index.ts

(cherry picked from commit 809e2ca)

* Docs: Update theming docs (grafana#19248)

(cherry picked from commit 9feac77)

* Explore: live tail UI fixes and improvements (grafana#19187)

(cherry picked from commit bf24cbb)

* Graphite: Changed range expansion from 1m to 1s (grafana#19246)

Fixes grafana#11472
(cherry picked from commit d95318b)

* MySQL, Postgres, MSSQL: Only debug log when in development (grafana#19239)

Found some additional debug statements in relation to grafana#19049 that
can cause memory issues.

Ref grafana#19049
(cherry picked from commit 19f3ec4)

* Vector: remove toJSON() from interface (grafana#19254)

(cherry picked from commit 6787e7b)

* Update changelog task to generate toolkit changelog too (grafana#19262)

(cherry picked from commit b7752b8)

* Dashboard: Hides alpha icon for visualization that is not in alpha/beta stage grafana#19300

Fixes grafana#19251
(cherry picked from commit f01836c)

* Build: Split up task in the CI pipeline to ease running outside circleci (grafana#18861)

* build: make sign rpm packages not depend on checking out private key

* build: move commands from circleci config into verify signed packages script

* build: split update and publish of deb and rpm into two scripts

* use files argument for sign and verify packages

* validate files argument for sign and verify packages

* update test publish of deb/rpm readme

(cherry picked from commit 4386604)

* Admin/user: fix textarea postion in 'Pending Invites' to avoid page scrolling (grafana#19288)

* hide textarea element after click 'Copy Invite' button on firefox
(cherry picked from commit 50b4695)

* Alerting: Prevents creating alerts from unsupported queries (grafana#19250)

* Refactor: Makes PanelEditor use state and shows validation message on AlerTab

* Refactor: Makes validation message nicer looking

* Refactor: Changes imports

* Refactor: Removes conditional props

* Refactor: Changes after feedback from PR review

* Refactor: Removes unused action

(cherry picked from commit 9bd6ed8)

* Chore: Update Slate to 0.47.8 (grafana#19197)

* Chore: Update Slate to 0.47.8
Closes grafana#17430

(cherry picked from commit 68d6da7)

* DataLinks: Small UX improvements to DataLinksInput (grafana#19313)

Closes grafana#19257
(cherry picked from commit feb6bc6)

* Multi-LDAP: Do not fail-fast on invalid credentials (grafana#19261)

* Multi-LDAP: Do not fail-fast on invalid credentials

When configuring LDAP authentication, it is very common to have multiple
servers configured. When using user bind (authenticating with LDAP using
the same credentials as the user authenticating to Grafana) we don't
expect all the users to be on all LDAP servers.

Because of this use-case, we should not fail-fast when authenticating on
multiple LDAP server configurations. Instead, we should continue to try
the credentials with the next LDAP server configured.

Fixes grafana#19066
(cherry picked from commit 279249e)

* Explore: Fix unsubscribing from Loki websocket (grafana#19263)

(cherry picked from commit 4c1bc59)

* Plugins: Skips existence of module.js for renderer plugins (grafana#19318)

* Fix: Skips test for module.js for plugins of renderer type
Fixes grafana#19130

* Refactor: Changes after PR comments

* Chore: Fixes go lint issue

(cherry picked from commit 75dcaec)

* Keybindings: Improve esc / exit / blur logic (grafana#19320)

* Keybindings: Improve esc / exit / blur logic

* Slight modifications

* removed use of jquery

(cherry picked from commit 08cc4f0)

* Select: Set placeholder color (grafana#19309)

(cherry picked from commit 2c9577f)

* Azure Monitor: Revert support for cross resource queries (grafana#19115)" (grafana#19346)

This reverts commit 8805125.
(cherry picked from commit 4dbedb8)

* Dashboard: Fix export for sharing when panels use default data source (grafana#19315)

* PanelModel: moved datasource: null away from defaults that are removed

* Added unit test

(cherry picked from commit ac3fb64)

* Heatmap: use DataFrame rather than LegacyResponseData (grafana#19026)

* merge master

* TimeSeries: datasources with labels should export tags (not labels) (grafana#18977)

* merge master

* export prometheus tags

* Annotations: Add annotations support to Loki (grafana#18949)

* Explore: Unify background color for fresh logs (grafana#18973)

* Singlestat: render lines on the panel when sparklines are enabled (grafana#18984)

* Image rendering: Add deprecation warning when PhantomJS is used for rendering images (grafana#18933)

* Add deprecation warning

* Update pkg/services/rendering/rendering.go

Co-Authored-By: Marcus Efraimsson <marcus.efraimsson@gmail.com>

* Units: Adding T,P,E,Z,and Y bytes (grafana#18706)

* Adding T and P for bytes

Luckily, all the hard work was done before; just added in these prefixes for our production environment.

* Future-proofing with other values (why not?)

* Yottaflops?

* Cutting back down to Peta sizes, except for hashes

* Refactor: move ScopedVars to grafana/data (grafana#18992)

* Refactor: Move sql_engine to sub package of tsdb (grafana#18991)

this way importing the tsdb package does not come with xorm dependencies

* use DataFrame in heatmaps

* actually use the setting :)

* remove unused timeSrv

* merge with master / useDataFrames

* fix test function

* merge master

* fix datasource type on snapshot

* reuse DataFrame calcs from graph panel

* update comments

(cherry picked from commit 2474511)

* Explore: Do not send explicit maxDataPoints for logs. (grafana#19235)

(cherry picked from commit f203e82)

* MySQL, Postgres, MSSQL: Fix validating query with template variables in alert  (grafana#19237)

Adds support for validating query in alert for mysql,
postgres and mssql.

Fixes grafana#13155
(cherry picked from commit 96046a7)

* MySQL, Postgres: Update raw sql when query builder updates (grafana#19209)

Raw sql now updates when changing query using
graphical query editor for mysql and postgres.

Fixes grafana#19063
(cherry picked from commit 7c499ff)

* MySQL: Limit datasource error details returned from the backend (grafana#19373)

Only return certain mysql errors from backend.
The following errors is returned as is from backend:
error code 1064 (parse error)
error code 1054 (bad column/field selected)
error code 1146 (table not exists)
Any other errors is logged and returned as a generic
error.
Restrict use of certain functions:
Do not allow usage of the following in query:
system_user()
session_user()
current_user() or current_user
user()
show grants

Fixes grafana#19360
(cherry picked from commit 3de693a)

* SQL: Rewrite statistics query (grafana#19178)

* Rewrite statistics query
(cherry picked from commit 56f5106)

* Release v6.4.0-beta2

* ValueFormats: check for inf (grafana#19376)

(cherry picked from commit 32b73bb)

* Build: Fix correct sort order of merged pr's in cherrypick task (grafana#19379)

(cherry picked from commit c4a03f4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spike of heavy MySQL queries after upgrade from 6.2.5 to 6.3.5
2 participants