-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
*: Add the metric about the SQL with TiFlash Success #23426
Conversation
Signed-off-by: jyz0309 <[email protected]>
Signed-off-by: jyz0309 <[email protected]>
Signed-off-by: jyz0309 <[email protected]>
Signed-off-by: jyz0309 <[email protected]>
Signed-off-by: jyz0309 <[email protected]>
Signed-off-by: jyz0309 <[email protected]>
store/tikv/metrics/metrics.go
Outdated
TiFlashExecuteErrorCounter = prometheus.NewCounter( | ||
prometheus.CounterOpts{ | ||
Namespace: namespace, | ||
Subsystem: subsystem, | ||
Name: "tiflash_execute_error_total", | ||
Help: "Counter of TiFlash execute errors.", | ||
}) | ||
|
||
TiFlashExecuteSuccCounter = prometheus.NewCounter( | ||
prometheus.CounterOpts{ | ||
Namespace: namespace, | ||
Subsystem: subsystem, | ||
Name: "tiflash_execute_total", | ||
Help: "Counter of TiFlash execute successfully.", | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TiFlashExecuteErrorCounter = prometheus.NewCounter( | |
prometheus.CounterOpts{ | |
Namespace: namespace, | |
Subsystem: subsystem, | |
Name: "tiflash_execute_error_total", | |
Help: "Counter of TiFlash execute errors.", | |
}) | |
TiFlashExecuteSuccCounter = prometheus.NewCounter( | |
prometheus.CounterOpts{ | |
Namespace: namespace, | |
Subsystem: subsystem, | |
Name: "tiflash_execute_total", | |
Help: "Counter of TiFlash execute successfully.", | |
}) | |
TiFlashExecuteErrorCounter = prometheus.NewCounterVec( | |
prometheus.CounterOpts{ | |
Namespace: namespace, | |
Subsystem: subsystem, | |
Name: "tiflash_requests_total", | |
Help: "Counter of TiFlash requests.", | |
}, []string{LblResult}) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use 1 metric counter is much easer to calculate the Number of “Failure query” / total queries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, wait, The original demand is calculate the ** failure query**, not the failure tiflash cop request
.
so the metric maybe rename to following is better.
TiFlashQueryTotalCounter = prometheus.NewCounterVec(
prometheus.CounterOpts{
Namespace: "tidb",
Subsystem: "server",
Name: "tiflash_query_total",
Help: "Counter of TiFlash queries.",
}, []string{LblResult})
And the metrics should put in metrics/server.go
file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect whether the "total queries" are counted correctly if we use an existing metrics. For example, does it count DDL? Does it count prepared statements? etc. I think @jyz0309 should take a look about this first before making this change, make sure that "total queries" to be replaced is exactly the same as the counter we added here.
Signed-off-by: jyz0309 <[email protected]>
Signed-off-by: jyz0309 <[email protected]>
Signed-off-by: jyz0309 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@crazycs520: Please use If you have approved this PR, please ignore this reply. This reply is being used as a temporary reply during the migration of the new bot and will be removed on April 1. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by writing |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: b7a6b33
|
Signed-off-by: ti-srebot <[email protected]>
cherry pick to release-4.0 in PR #23517 |
Signed-off-by: ti-srebot <[email protected]>
cherry pick to release-5.0 in PR #23956 |
Signed-off-by: jyz0309 [email protected]
What problem does this PR solve?
Problem Summary:
This pull request is for the TiDB SLO,to add the metric about the SQL with TiFlash
What is changed and how it works?
What's Changed:
Add two metrics for coprocessor send to promethus.
TiFlashExecuteCounter: The total num of SQL request send TiFlash.
TiFlashExecuteErrorCounter: The total num of SQL request error send TiFlash.
Related changes
Check List
Tests
Side effects
Release note