Skip to content

Conversation

@cristiangreco
Copy link
Contributor

@cristiangreco cristiangreco commented Aug 26, 2024

Extend the query on events_statements_summary_by_digest to fetch SUM_LOCK_TIME and SUM_CPU_TIME and return them as counters. These are useful to report time spent running on the cpu (e.g. to sort results or run computations) vs waiting on locks (e.g. to acquire write lock on a table or row), by query digest.

@cristiangreco cristiangreco force-pushed the cristian/add-lock-cpu-time branch from 9841b7b to 4212787 Compare August 26, 2024 14:11
Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

I think we originally intentionally left these out for cardinality reasons. Would you mind documenting how they they are additionally useful in the PR description? That way we can have better history of "why" these are added by default.

@cristiangreco
Copy link
Contributor Author

I think we originally intentionally left these out for cardinality reasons. Would you mind documenting how they they are additionally useful in the PR description? That way we can have better history of "why" these are added by default.

Hey @SuperQ thanks for having a look! These metrics would have the same cardinality (in terms of labels values) as the other sibling metrics created out of the query events_statements_summary_by_digest. Or are we worried about adding new metrics in general? (I wanted to submit a followup PR to additionally expose QUANTILE_{95,99,999} so it'd be good to understand what is the approach to take)

@SuperQ
Copy link
Member

SuperQ commented Aug 26, 2024

It's about adding new metrics in general. If you can explain what the metrics are useful for, that's all I'm asking about.

Exposing the quantiles is less useful, as they can't be aggregated between MySQL servers.

@cristiangreco
Copy link
Contributor Author

cristiangreco commented Aug 26, 2024

I've added a short note on the PR description, could you please check that?

@SuperQ
Copy link
Member

SuperQ commented Aug 27, 2024

Needs a rebase.

Extend the query on `events_statements_summary_by_digest` to fetch
`SUM_LOCK_TIME` and `SUM_CPU_TIME` and return them as counters.

Signed-off-by: Cristian Greco <[email protected]>
@cristiangreco cristiangreco force-pushed the cristian/add-lock-cpu-time branch from 4212787 to 3225ffa Compare August 27, 2024 09:42
@cristiangreco
Copy link
Contributor Author

Needs a rebase.

Thanks so much! Just rebased.

@SuperQ
Copy link
Member

SuperQ commented Aug 27, 2024

I had a think about the history of this. I wish we had written down the queries we experimented with in production at the time.

I think we tried adding CPU time originally, but we didn't really find it useful compared to total query time.

Lock time is probably a good one to have tho.

If cpu time = total time - lock time, maybe we don't need cpu time.

@cristiangreco
Copy link
Contributor Author

I think we tried adding CPU time originally, but we didn't really find it useful compared to total query time.

@SuperQ fair enough, if you prefer I can close this PR and open a new one adding just lock time?

@SuperQ
Copy link
Member

SuperQ commented Aug 27, 2024

I'm mostly looking for up-to-date expertise in MySQL here. It's been a while since I looked deeply at this data. I assume you've got data on the uselessness of these metrics.

@cristiangreco
Copy link
Contributor Author

I'm mostly looking for up-to-date expertise in MySQL here. It's been a while since I looked deeply at this data. I assume you've got data on the uselessness of these metrics.

So I'm not sure that cpu time = total time - lock time is true but can't disprove it either. I thought it was cheap enough that we could export both metrics anyway.

@SuperQ
Copy link
Member

SuperQ commented Aug 28, 2024

Ok, I guess this is fine as-is. If people complain about the additional cardinality we can look it over again.

@SuperQ SuperQ merged commit d65c5e6 into prometheus:main Aug 28, 2024
@SuperQ SuperQ mentioned this pull request Aug 29, 2024
cristiangreco added a commit to grafana/mysqld_exporter that referenced this pull request Nov 8, 2024
Changes:
* [FEATURE] Support for prometheus scrape timeout in probe endpoint prometheus#828
* [ENHANCEMENT] Support MySQL 8.4 replicas syntax prometheus#837
* [ENHANCEMENT] Fetch lock time and cpu time from performance schema prometheus#862
* [ENHANCEMENT] Add the instance struct to handle connections prometheus#859
* [ENHANCEMENT] Optimize code by using built-in constants in the standard lib prometheus#844
* [BUGFIX] Fix fetching tmpTables vs tmpDiskTables from performance_schema prometheus#853
* [BUGFIX] Skip SPACE_TYPE column for MariaDB >=10.5 prometheus#860
* [BUGFIX] Fixed parsing of timestamps with non-zero padded days prometheus#841
* [BUGFIX] Fix auto_increment metric collection errors caused by using collation in INFORMATION_SCHEMA searches prometheus#833
* [BUGFIX] Fix race condition in ReloadConfig prometheus#760
* [BUGFIX] Change processlist query to support ONLY_FULL_GROUP_BY sql_mode prometheus#684
* [BUGFIX] replication_applier_status_by_worker requires mysql 8.0 prometheus#683
* [BUGFIX] Update docker registry link in README.md prometheus#813
* [BUGFIX] Fix Docker run command and update documentation for cnf file handling prometheus#843
@cristiangreco cristiangreco mentioned this pull request Nov 8, 2024
cristiangreco added a commit to grafana/mysqld_exporter that referenced this pull request Nov 8, 2024
Changes:
* [FEATURE] Support for prometheus scrape timeout in probe endpoint prometheus#828
* [ENHANCEMENT] Support MySQL 8.4 replicas syntax prometheus#837
* [ENHANCEMENT] Fetch lock time and cpu time from performance schema prometheus#862
* [ENHANCEMENT] Add the instance struct to handle connections prometheus#859
* [ENHANCEMENT] Optimize code by using built-in constants in the standard lib prometheus#844
* [BUGFIX] Fix fetching tmpTables vs tmpDiskTables from performance_schema prometheus#853
* [BUGFIX] Skip SPACE_TYPE column for MariaDB >=10.5 prometheus#860
* [BUGFIX] Fixed parsing of timestamps with non-zero padded days prometheus#841
* [BUGFIX] Fix auto_increment metric collection errors caused by using collation in INFORMATION_SCHEMA searches prometheus#833
* [BUGFIX] Fix race condition in ReloadConfig prometheus#760
* [BUGFIX] Change processlist query to support ONLY_FULL_GROUP_BY sql_mode prometheus#684
* [BUGFIX] replication_applier_status_by_worker requires mysql 8.0 prometheus#683
* [BUGFIX] Update docker registry link in README.md prometheus#813
* [BUGFIX] Fix Docker run command and update documentation for cnf file handling prometheus#843

Signed-off-by: Cristian Greco <[email protected]>
cristiangreco added a commit to grafana/mysqld_exporter that referenced this pull request Nov 8, 2024
Changes:
* [FEATURE] Support for prometheus scrape timeout in probe endpoint prometheus#828
* [ENHANCEMENT] Support MySQL 8.4 replicas syntax prometheus#837
* [ENHANCEMENT] Fetch lock time and cpu time from performance schema prometheus#862
* [ENHANCEMENT] Add the instance struct to handle connections prometheus#859
* [ENHANCEMENT] Optimize code by using built-in constants in the standard lib prometheus#844
* [BUGFIX] Fix fetching tmpTables vs tmpDiskTables from performance_schema prometheus#853
* [BUGFIX] Skip SPACE_TYPE column for MariaDB >=10.5 prometheus#860
* [BUGFIX] Fixed parsing of timestamps with non-zero padded days prometheus#841
* [BUGFIX] Fix auto_increment metric collection errors caused by using collation in INFORMATION_SCHEMA searches prometheus#833
* [BUGFIX] Fix race condition in ReloadConfig prometheus#760
* [BUGFIX] Change processlist query to support ONLY_FULL_GROUP_BY sql_mode prometheus#684
* [BUGFIX] replication_applier_status_by_worker requires mysql 8.0 prometheus#683
* [BUGFIX] Update docker registry link in README.md prometheus#813
* [BUGFIX] Fix Docker run command and update documentation for cnf file handling prometheus#843

Signed-off-by: Cristian Greco <[email protected]>
cristiangreco added a commit to grafana/mysqld_exporter that referenced this pull request Nov 8, 2024
Changes:
* [CHANGE] Replace logging library go-kit/log with slog prometheus#885
* [FEATURE] Support for prometheus scrape timeout in probe endpoint prometheus#828
* [ENHANCEMENT] Support MySQL 8.4 replicas syntax prometheus#837
* [ENHANCEMENT] Fetch lock time and cpu time from performance schema prometheus#862
* [ENHANCEMENT] Add the instance struct to handle connections prometheus#859
* [ENHANCEMENT] Optimize code by using built-in constants in the standard lib prometheus#844
* [BUGFIX] Fix fetching tmpTables vs tmpDiskTables from performance_schema prometheus#853
* [BUGFIX] Skip SPACE_TYPE column for MariaDB >=10.5 prometheus#860
* [BUGFIX] Fixed parsing of timestamps with non-zero padded days prometheus#841
* [BUGFIX] Fix auto_increment metric collection errors caused by using collation in INFORMATION_SCHEMA searches prometheus#833
* [BUGFIX] Fix race condition in ReloadConfig prometheus#760
* [BUGFIX] Change processlist query to support ONLY_FULL_GROUP_BY sql_mode prometheus#684
* [BUGFIX] replication_applier_status_by_worker requires mysql 8.0 prometheus#683
* [BUGFIX] Update docker registry link in README.md prometheus#813
* [BUGFIX] Fix Docker run command and update documentation for cnf file handling prometheus#843
* [BUGFIX] info_schema_tables: do not collect the sys schema prometheus#879

Signed-off-by: Cristian Greco <[email protected]>
cristiangreco added a commit to grafana/mysqld_exporter that referenced this pull request Nov 8, 2024
Changes:
* [CHANGE] Replace logging library go-kit/log with slog prometheus#875
* [FEATURE] Support for prometheus scrape timeout in probe endpoint prometheus#828
* [ENHANCEMENT] Support MySQL 8.4 replicas syntax prometheus#837
* [ENHANCEMENT] Fetch lock time and cpu time from performance schema prometheus#862
* [ENHANCEMENT] Add the instance struct to handle connections prometheus#859
* [ENHANCEMENT] Optimize code by using built-in constants in the standard lib prometheus#844
* [BUGFIX] Fix fetching tmpTables vs tmpDiskTables from performance_schema prometheus#853
* [BUGFIX] Skip SPACE_TYPE column for MariaDB >=10.5 prometheus#860
* [BUGFIX] Fixed parsing of timestamps with non-zero padded days prometheus#841
* [BUGFIX] Fix auto_increment metric collection errors caused by using collation in INFORMATION_SCHEMA searches prometheus#833
* [BUGFIX] Fix race condition in ReloadConfig prometheus#760
* [BUGFIX] Change processlist query to support ONLY_FULL_GROUP_BY sql_mode prometheus#684
* [BUGFIX] replication_applier_status_by_worker requires mysql 8.0 prometheus#683
* [BUGFIX] Update docker registry link in README.md prometheus#813
* [BUGFIX] Fix Docker run command and update documentation for cnf file handling prometheus#843
* [BUGFIX] info_schema_tables: do not collect the sys schema prometheus#879

Signed-off-by: Cristian Greco <[email protected]>
SuperQ pushed a commit that referenced this pull request Nov 8, 2024
Changes:
* [CHANGE] Replace logging library go-kit/log with slog #875
* [FEATURE] Support for prometheus scrape timeout in probe endpoint #828
* [ENHANCEMENT] Support MySQL 8.4 replicas syntax #837
* [ENHANCEMENT] Fetch lock time and cpu time from performance schema #862
* [ENHANCEMENT] Add the instance struct to handle connections #859
* [ENHANCEMENT] Optimize code by using built-in constants in the standard lib #844
* [BUGFIX] Fix fetching tmpTables vs tmpDiskTables from performance_schema #853
* [BUGFIX] Skip SPACE_TYPE column for MariaDB >=10.5 #860
* [BUGFIX] Fixed parsing of timestamps with non-zero padded days #841
* [BUGFIX] Fix auto_increment metric collection errors caused by using collation in INFORMATION_SCHEMA searches #833
* [BUGFIX] Fix race condition in ReloadConfig #760
* [BUGFIX] Change processlist query to support ONLY_FULL_GROUP_BY sql_mode #684
* [BUGFIX] replication_applier_status_by_worker requires mysql 8.0 #683
* [BUGFIX] Update docker registry link in README.md #813
* [BUGFIX] Fix Docker run command and update documentation for cnf file handling #843
* [BUGFIX] info_schema_tables: do not collect the sys schema #879

Signed-off-by: Cristian Greco <[email protected]>
SuperQ added a commit that referenced this pull request Nov 8, 2024
* [CHANGE] Replace logging library go-kit/log with slog #875
* [FEATURE] Support for prometheus scrape timeout in probe endpoint #828
* [ENHANCEMENT] Support MySQL 8.4 replicas syntax #837
* [ENHANCEMENT] Fetch lock time and cpu time from performance schema #862
* [ENHANCEMENT] Add the instance struct to handle connections #859
* [ENHANCEMENT] Optimize code by using built-in constants in the standard lib #844
* [BUGFIX] Fix fetching tmpTables vs tmpDiskTables from performance_schema #853
* [BUGFIX] Skip SPACE_TYPE column for MariaDB >=10.5 #860
* [BUGFIX] Fixed parsing of timestamps with non-zero padded days #841
* [BUGFIX] Fix auto_increment metric collection errors caused by using collation in INFORMATION_SCHEMA searches #833
* [BUGFIX] Fix race condition in ReloadConfig #760
* [BUGFIX] Change processlist query to support ONLY_FULL_GROUP_BY sql_mode #684
* [BUGFIX] replication_applier_status_by_worker requires mysql 8.0 #683
* [BUGFIX] Update docker registry link in README.md #813
* [BUGFIX] Fix Docker run command and update documentation for cnf file handling #843
* [BUGFIX] info_schema_tables: do not collect the sys schema #879

Signed-off-by: SuperQ <[email protected]>
@SuperQ SuperQ mentioned this pull request Nov 8, 2024
SuperQ added a commit that referenced this pull request Nov 8, 2024
* [CHANGE] Replace logging library go-kit/log with slog #875
* [FEATURE] Support for prometheus scrape timeout in probe endpoint #828
* [ENHANCEMENT] Support MySQL 8.4 replicas syntax #837
* [ENHANCEMENT] Fetch lock time and cpu time from performance schema #862
* [ENHANCEMENT] Add the instance struct to handle connections #859
* [ENHANCEMENT] Optimize code by using built-in constants in the standard lib #844
* [BUGFIX] Fix fetching tmpTables vs tmpDiskTables from performance_schema #853
* [BUGFIX] Skip SPACE_TYPE column for MariaDB >=10.5 #860
* [BUGFIX] Fixed parsing of timestamps with non-zero padded days #841
* [BUGFIX] Fix auto_increment metric collection errors caused by using collation in INFORMATION_SCHEMA searches #833
* [BUGFIX] Fix race condition in ReloadConfig #760
* [BUGFIX] Change processlist query to support ONLY_FULL_GROUP_BY sql_mode #684
* [BUGFIX] replication_applier_status_by_worker requires mysql 8.0 #683
* [BUGFIX] Update docker registry link in README.md #813
* [BUGFIX] Fix Docker run command and update documentation for cnf file handling #843
* [BUGFIX] info_schema_tables: do not collect the sys schema #879

Signed-off-by: SuperQ <[email protected]>
@dbakit
Copy link

dbakit commented Feb 10, 2025

https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-28.html
----
As indicated in the release notes, SUM_LOCK_TIME and SUM_CPU_TIME are only supported starting from version 8.0.28. These fields do not exist in versions 5.7 and below or in versions prior to 8.0.28. It is recommended to differentiate the scrap SQL based on the mysql_version.
This PR case #905

@cristiangreco cristiangreco deleted the cristian/add-lock-cpu-time branch February 10, 2025 20:44
cristiangreco added a commit to cristiangreco/mysqld_exporter that referenced this pull request Feb 10, 2025
…8.0.28

Fix issue introduced in prometheus#862

Only query SUM_LOCK_TIME and SUM_CPU_TIME when using MySQL >= 8.0.28.
cristiangreco added a commit to cristiangreco/mysqld_exporter that referenced this pull request Feb 10, 2025
…8.0.28

Fix issue introduced in prometheus#862

Only query SUM_LOCK_TIME and SUM_CPU_TIME when using MySQL >= 8.0.28.

Signed-off-by: Cristian Greco <[email protected]>
cristiangreco added a commit to cristiangreco/mysqld_exporter that referenced this pull request Feb 10, 2025
…8.0.28

Fix issue introduced in prometheus#862

Only query SUM_LOCK_TIME and SUM_CPU_TIME when using MySQL >= 8.0.28.

Fixes prometheus#905

Signed-off-by: Cristian Greco <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants