br: add metrics description for snapshot restore and PITR #18516
Conversation
Signed-off-by: Wenqi Mou <[email protected]>
ti-chi-bot
bot
added
first-time-contributor
grafana-tikv-dashboard.md
Outdated
| - Import Wait Duration: time spent on downloading task waiting in queue for execution. | ||
| - Import Read SST Duration: time spent on reading from external storage of a file and download it to TiKV. | ||
| - Import Rewrite SST Duration: time spent on rewriting SST based on rewrite rules. | ||
| - Import Ingest RPC Duration: time spent on sending ingest response in RPC call. |
There was a problem hiding this comment.
| - Import Ingest RPC Duration: time spent on sending ingest response in RPC call. | |
| - Import Ingest RPC Duration: time spent on handing ingest response in RPC call. |
It is server-side.
There was a problem hiding this comment.
I see, just read the code again, you are right, the timer is started before the handling logic, I thought it was the time of sending response from server back to client.
The name of RPC duration is confusing to me actually. Normally it should measure the entire GRPC call end to end.
// client side pseudo code
timer = Time();
server.doSomething().await; // GRPC call to server
record(timer);
in our use case it is only measuring part of it on the server side. we should audit all those metrics and rename it properly.
grafana-tikv-dashboard.md
Outdated
| - Blocked by Concurrency Time: time spent on waiting to get executed due to concurrency constraint. | ||
| - Apply Request Speed: speed of applying request to raft engine. | ||
| - Cached File in Memory: files cached by the applying requests of importer. | ||
| - Engine Requests Unfinished: number of pending requests to raft engine. |
There was a problem hiding this comment.
The "raft engine" should be "raft store", this "Engine" actually means the Engine trait (RaftKv class) in TiKV. "Raft Engine" specially means the storage of the raft state in TiKV.
grafana-tikv-dashboard.md
Outdated
| - Import Ingest SST Bytes: number of bytes ingested. | ||
| - Import Download SST Throughput: SST download throughput in bytes per second. | ||
| - Import Local Write keys: ??? | ||
| - Import Local Write bytes: ??? |
There was a problem hiding this comment.
yeah I guess that raise the problem of mixing unrelated metrics. we shouldn't expect to see lightning related metrics in the backup&restore dashboard
opened issue tikv/tikv#17369
Signed-off-by: Wenqi Mou <[email protected]>
|
@BornChanger: adding LGTM is restricted to approvers and reviewers in OWNERS files. In response to this: 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 kubernetes/test-infra repository. |
grafana-tikv-dashboard.md
Outdated
| - Import Ingest SST Duration: time spent on ingesting SST into RocksDB. | ||
| - Import Ingest SST Bytes: number of bytes ingested. | ||
| - Import Download SST Throughput: SST download throughput in bytes per second. | ||
| - TTL Expired: number of expired items after TTL in backup files. |
There was a problem hiding this comment.
we can also skip this item since it's raw_kv related, and it also needs to be filtered out from snapshot restore panel.
grafana-tikv-dashboard.md
Outdated
| @@ -484,6 +484,45 @@ This section provides a detailed description of these key metrics on the **TiKV- | |||
| - Initial Scanning Trigger Reason: The reason for triggering incremental scanning | |||
| - Region Checkpoint Key Putting: The number of checkpoint operations logged to the PD | |||
|
|
|||
| ### Snapshot restore | |||
There was a problem hiding this comment.
most are for import, why call it Snapshot restore
There was a problem hiding this comment.
The purpose of this PR is to add metrics description for restore, as a new member in the team I find it myself confused when looking at sst importer where it's actually used by restore. I feel like customer might have the same doubt when looking the metrics, so changing the name to be snapshot restore.
I have brought up to the team that we probably should have an abstraction of the restore on the TiKV path, where internally it still calls sst importer. It can be more explicit and meaningful to users this way, and inside of it we can rename or create new metrics to be decoupled with sst importer.
let me know if that makes sense!
Signed-off-by: Wenqi Mou <[email protected]>
|
/cc @Oreoxmt please help merge the pr |
|
@BornChanger: GitHub didn't allow me to request PR reviews from the following users: merge, the, pr, please. Note that only pingcap members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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 kubernetes/test-infra repository. |
Oreoxmt
added
type/enhancement
ti-chi-bot
bot
removed
the
missing-translation-status
Oreoxmt
added
missing-translation-status
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Oreoxmt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
In response to a cherrypick label: new pull request created to branch |
|
In response to a cherrypick label: new pull request created to branch |
|
In response to a cherrypick label: new pull request created to branch |
Signed-off-by: ti-chi-bot <[email protected]>
|
In response to a cherrypick label: new pull request created to branch |
|
In response to a cherrypick label: new pull request created to branch |
|
In response to a cherrypick label: new pull request created to branch |
|
In response to a cherrypick label: new pull request created to branch |
Oreoxmt
added
translation/done
First-time contributors' checklist
What is changed, added or deleted? (Required)
Add a bunch of descriptions for restore based on metrics in
TiKV#tikv_details.dashboard.pyWhich TiDB version(s) do your changes apply to? (Required)
Tips for choosing the affected version(s):
By default, CHOOSE MASTER ONLY so your changes will be applied to the next TiDB major or minor releases. If your PR involves a product feature behavior change or a compatibility change, CHOOSE THE AFFECTED RELEASE BRANCH(ES) AND MASTER.
For details, see tips for choosing the affected versions.
What is the related PR or file link(s)?
Do your changes match any of the following descriptions?