Bug#121045 Server telemetry metrics backed by aggregated_stats_buffer always report 0 - #735
Open
hunter-kang wants to merge 1 commit into
Open
Bug#121045 Server telemetry metrics backed by aggregated_stats_buffer always report 0#735hunter-kang wants to merge 1 commit into
hunter-kang wants to merge 1 commit into
Conversation
… always report 0 Problem: ======== 25 server telemetry (OTEL) metrics under the mysql.stats and mysql.stats.handler meters always export 0, while the matching SHOW GLOBAL STATUS counter reports real activity. These metrics are read by the async metric callback (get_metric_aggregated_integer in sql/mysqld.cc) from the sharded aggregated_stats_buffer, aggregated across shards by aggregated_stats::get_single_total. The read side, the metric registration, the shard field, its reset, and its cross-shard aggregation all exist, and the producer-side shard write is present for the counters that work (e.g. questions, com_*, ha_commit). For these 25 it is missing: the producers write only the per-THD status_var and never the shard the reader sums, so the exported value is a constant 0. Affected producers: - THD::inc_status_*() for the Select_*, Sort_*, and Created_tmp_* family (sql/sql_class.cc) plus long_query_count (sql/log.cc) and max_execution_time_exceeded (sql/sql_class.cc). - handler::ha_statistic_increment() (sql/handler.cc) for the Handler_* read/write family, reached from every storage engine. The defect is not observable through SQL (performance_schema.setup_metrics lists metric names but has no value column), which is why it went unnoticed. Solution: ========= Populate the shard next to every existing per-THD store, matching the established idiom already used by the counters that work. For the by-name producers, add the matching global_aggregated_stats.get_shard(thread_id()).<field> increment next to the status_var store. handler::ha_statistic_increment() takes a ulonglong System_status_var::* member pointer, which cannot be reused for aggregated_stats_buffer (the two structs order their fields differently and the shard uses std::atomic_uint64_t). Extend the helper to also take the corresponding aggregated_stats_buffer member pointer and update the call sites to pass both. The shard store uses fetch_add with std::memory_order_relaxed: the value is only summed later by the reader and nothing synchronizes on it, and these counters sit on the per-row read path. Add an MTR regression test (perfschema.telemetry_metrics_shard_bug) that drives activity across the Select, Sort, Created_tmp, and Handler (read and write) families and asserts each covered metric is populated. It covers 19 of the 25 fixed counters; the other 6 (max_execution_time_ exceeded, mrr_init, select_range_check, select_full_range_join, sort_range, sort_merge_passes) are omitted because they cannot be driven nonzero deterministically and would make the test flaky. All 25 flow through the same two producer mechanisms (THD::inc_status_* and handler::ha_statistic_increment), both of which the 19 exercise. A -master.opt sets the component plugin-dir so the test component loads reliably. The test fails on the unfixed server (0) and passes once the shards are written. Note on later versions: this change is against 8.4 and fixes the 25 affected metrics present there. One further metric, count_hit_tmp_table_size, exists only in 9.x (added after 8.4) and is affected the same way; it is not touched by this change and will need a separate fix on the branch that introduces it. This contribution is under the OCA signed by Amazon and covering submissions to the MySQL project.
hunter-kang
marked this pull request as ready for review
August 27, 2026 08:58
marcalff
reviewed
Sep 1, 2026
marcalff
left a comment
Member
There was a problem hiding this comment.
Thanks for the fix.
The analysis in the bug report, and the fix in this PR, looks correct,
updating the global shards was indeed missing for some metrics.
The next steps for this PR are:
- deploy CI in github for branch 8.4 and 9.7 (only 26.7 has CI),
- and/or try the patch internally for now using the old infrastructure.
and then:
- merge to 8.4-LTS, 9.7-LTS and trunk, exact details not known at this point.
All this is pending completion of the migration to github.
Related, your github account shows as first contributor to this repository, so we should make sure when applying and merging the patch, that you get proper credits, and that github detects your account as a contributor from now on.
Again, pending completion of the migration to github.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this change do?
Fixes Bug#121045. 25 server telemetry (OTEL) metrics under
mysql.stats/mysql.stats.handleralways export 0: their per-row producers increment only the per-THDstatus_var, never the shardedaggregated_stats_bufferthat the metric callback (get_metric_aggregated_integer) reads. This adds the missing shard write next to each per-THD store, so the metrics report real activity.Why is it needed?
Without it, 25 metrics (the
Handler_*read/write family, plusSelect_*,Sort_*,Created_tmp_*,long_query_count,max_execution_time_exceeded) read a constant 0 under real load. Introduced in 8.2.0 by WL#15199 (commit 83926c7), which added the sharded buffer and its OTEL reader but wired the shard write only for transaction-level counters (e.g.ha_commit), not the per-row producers. It went unnoticed becauseSHOW STATUSsums the per-THD values through a different path, so the counters look correct there.How was it tested?
mysql-test/scripts/ci/mtr.shpasses locallyNew test
perfschema.telemetry_metrics_shard_bugdrives the Select / Sort / Created_tmp / Handler families and asserts each covered metric is populated. It fails on the unfixed server (metrics read 0) and passes with the fix. The fullperfschemasuite was run (618/624 pass, including this test); the handful of failures are pre-existing or parallel-execution-flaky (verified: they fail on the unpatched baseline or pass when run in isolation), none caused by this change.This change adds one
memory_order_relaxedatomic per handler call on the per-row path. sysbencholtp_read_onlyandoltp_write_only(baseline vs. patched) showed no measurable throughput or latency regression.Contributor checklist
scripts/ci/format.sh)AI assistance
Used Claude Opus 4.8 (via Claude Code) throughout this contribution: it generated the code change and the MTR test (iterated across revisions), ran the sysbench performance testing, researched the root cause and the commit that introduced the bug (WL#15199, 8.2.0), and drafted this PR description. All of it was human-reviewed and verified: I manually checked the code and its call paths, and manually confirmed the MTR test's before/after behavior (local builds + MTR).
Areas touched
sql(handler, sql_class, log) and the storage-engine handlers (innodb, archive, csv, federated, heap, myisam, myisammrg, ndb, perfschema, temptable);perfschemaMTR suite.Note on later versions
This fix targets 8.4 and covers the 25 affected counters present there. One further counter,
count_hit_tmp_table_size, exists only in 9.x (added after 8.4) and has the same defect; it is not touched by this change and will need to be handled separately.Copyright
This contribution is under the OCA signed by Amazon and covering submissions to the MySQL project.