Add a gtid transaction counter based on the @@gtid_executed global variable - #965
Add a gtid transaction counter based on the @@gtid_executed global variable#965sjmudd wants to merge 1 commit into
Conversation
This PR provides a simple global transaction counter which can be used to track the transaction rate on a MySQL server. - The full GTID set is combined into a single transaction count - UUIDs are ignored - Any GTID gaps are taken into account - works on MySQL 5.6+ - works with MySQL 8.4+ tagged gtid format Signed-off-by: Simon J Mudd <[email protected]>
|
Hello, It is not very clear to me how long I should expect to wait for a response to my PR, whether positive or negative. If you could provide some sort of feedback here or in the README that would be most helpful. |
|
2 months have passed. Any chance of getting this PR accepted? If you need something more from me can you tell me what it is? |
ArthurSens
left a comment
There was a problem hiding this comment.
Hey, sorry for the super-delayed review. We are understaffed for exporter maintainers, but we're trying to catch up with our backlog 😓
I've left a few comments below. Also, we might want to rewrite the code to clarify that it's about GTID executions, not transactions, since people might get confused with regular database transactions. For example, SET gtid_purged is not a transaction at all, but counts as a GTID execution
| var ( | ||
| // Metric descriptors. | ||
| GtidTransactionCounterDesc = prometheus.NewDesc( | ||
| prometheus.BuildFQName(namespace, prometheusSubsystem, prometheusName), |
There was a problem hiding this comment.
| prometheus.BuildFQName(namespace, prometheusSubsystem, prometheusName), | |
| prometheus.BuildFQName(namespace, prometheusSubsystem,"executed_total"), |
Following Prometheus naming conventions, counters should be suffixed with _total. Besides that, gtid_executed seems more accurate than gtid_transactions 🤔
| tests := []struct { | ||
| name string | ||
| gtidSet string | ||
| expected float64 | ||
| }{ | ||
| {"empty_set", "", 0}, | ||
| {"single_uuid_and_range", `uuid1:1-1000`, 1000}, | ||
| {"multiple_uuid_single_range", `uuid1:1-1000, | ||
| uuid1:1001-2000`, 2000}, | ||
| {"single_uuid_with_ranges", `uuid1:1-1000,2001-4000`, 3000}, | ||
| } |
There was a problem hiding this comment.
Looking at docs
- GTID format, sets, intervals, and tags
- gtid_executed system-variable semantics
- GTID lifecycle, gaps, filtering, and resets
The test cases seems incorrect 🤔
I'd add something like
| tests := []struct { | |
| name string | |
| gtidSet string | |
| expected float64 | |
| }{ | |
| {"empty_set", "", 0}, | |
| {"single_uuid_and_range", `uuid1:1-1000`, 1000}, | |
| {"multiple_uuid_single_range", `uuid1:1-1000, | |
| uuid1:1001-2000`, 2000}, | |
| {"single_uuid_with_ranges", `uuid1:1-1000,2001-4000`, 3000}, | |
| } | |
| tests := []struct { | |
| name string | |
| gtidSet string | |
| expected float64 | |
| expectErr bool | |
| }{ | |
| {"empty_set", "", 0, false}, | |
| {"single_transaction", `3E11FA47-71CA-11E1-9E33-C80AA9429562:1`, 1, false}, | |
| {"single_uuid_and_range", `3E11FA47-71CA-11E1-9E33-C80AA9429562:1-1000`, 1000, false}, | |
| {"single_uuid_with_ranges", `3E11FA47-71CA-11E1-9E33-C80AA9429562:1-3:11:47-49`, 7, false}, | |
| {"multiple_uuids", `3E11FA47-71CA-11E1-9E33-C80AA9429562:1-3, | |
| 24BC7856-9C4A-11E1-9D41-80C16E4A511C:1-10`, 13, false}, | |
| {"multiple_uuids_single_line", `3E11FA47-71CA-11E1-9E33-C80AA9429562:1-3,24BC7856-9C4A-11E1-9D41-80C16E4A511C:1-10`, 13, false}, | |
| {"tagged_gtid_with_ranges", `3E11FA47-71CA-11E1-9E33-C80AA9429562:Domain_1:1-3:11:47-49`, 7, false}, | |
| {"multiple_tags_for_same_uuid", `3E11FA47-71CA-11E1-9E33-C80AA9429562:Domain_1:1-3:15-21, | |
| 3E11FA47-71CA-11E1-9E33-C80AA9429562:Domain_2:8-52`, 55, false}, | |
| {"mixed_tagged_and_untagged_sets", `3E11FA47-71CA-11E1-9E33-C80AA9429562:1-5, | |
| 24BC7856-9C4A-11E1-9D41-80C16E4A511C:Analytics_1:10-14`, 10, false}, | |
| {"missing_uuid", `1-10`, 0, true}, | |
| {"comma_between_intervals", `3E11FA47-71CA-11E1-9E33-C80AA9429562:1-3,11-20`, 0, true}, | |
| {"reversed_range", `3E11FA47-71CA-11E1-9E33-C80AA9429562:20-10`, 0, true}, | |
| {"zero_transaction_id", `3E11FA47-71CA-11E1-9E33-C80AA9429562:0`, 0, true}, | |
| {"missing_tagged_interval", `3E11FA47-71CA-11E1-9E33-C80AA9429562:Domain_1`, 0, true}, | |
| {"invalid_tag", `3E11FA47-71CA-11E1-9E33-C80AA9429562:123tag:1-10`, 0, true}, | |
| {"transaction_id_overflow", `3E11FA47-71CA-11E1-9E33-C80AA9429562:1-9223372036854775808`, 0, true}, | |
| }``` |
| github.com/prometheus/client_model v0.6.2 | ||
| github.com/prometheus/common v0.65.0 | ||
| github.com/prometheus/exporter-toolkit v0.13.2 | ||
| github.com/sjmudd/mysqlgtid v0.1.0 |
There was a problem hiding this comment.
This dependency seems to parse GTID incorrectly; is there a more accurate one? It's been a few years since the PR was opened, so maybe there are newer, more accurate versions?
|
|
||
| // Name of the Scraper. Should be unique. | ||
| func (ScrapeGtidExecuted) Name() string { | ||
| return "gtid_transactions" |
There was a problem hiding this comment.
| return "gtid_transactions" | |
| return "gtid_executions" |
|
|
||
| // Help describes the role of the Scraper. | ||
| func (ScrapeGtidExecuted) Help() string { | ||
| return "Number of GTID transactions" |
There was a problem hiding this comment.
| return "Number of GTID transactions" | |
| return "Number of GTID executions" |
This PR provides a simple global transaction counter which can be used to track the transaction rate on a MySQL server.