Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Repository: pingcap/docs/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @Copilot. Thanks for your PR. I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
Co-authored-by: lilin90 <30922556+lilin90@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
sary8
left a comment
There was a problem hiding this comment.
I filed #23921 and checked this against the TiKV dashboard source.
The eight renames match ResolvedTS() in tikv_details.dashboard.py (lines 8998-9257 at bdf406f). The two description changes carry over TiKV's own wording:
title="Max gap of follower safe-ts",
description="The gap between now() and the minimal (non-zero) safe ts for followers",title="Min Safe TS Follower Region",
description="The region id of the follower that has minimal safe ts",One thing I noticed: this patch only changes master, but the PR description also marks v9.0, v8.5, v8.1, v7.5, v7.1 and v6.5 as affected.
release-8.5, release-8.1, release-7.5 and release-7.1 still have Resolved-TS worker CPU here, while the TiKV dashboard on those branches already uses the newer names, including Max gap of follower safe-ts, so they seem to need the same update.
v9.0 and v6.5 are different. There is no release-9.0 branch in this repo, and TiKV release-6.5 has Resolved TS Worker CPU but neither Max gap of follower safe-ts nor Max gap of safe-ts, so I would verify that branch separately rather than backporting the same change.
Also, separately, this row has six panels the section does not mention at all: CheckLeader request region count, CheckLeader request size, Fail advance ts count, Initial scan backoff duration, Observe region status, Pending command size. I can open another issue for those if useful.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Several cross-document references still use renamed or removed dashboard labels.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Updates TiKV Grafana dashboard documentation to match current panel names and follower-specific safe-ts semantics.
Changes:
- Renames the Resolved TS section and seven panel labels.
- Corrects follower safe-ts descriptions.
- Leaves related cross-references requiring updates.
| File | Description |
|---|---|
grafana-tikv-dashboard.md |
Synchronizes Resolved TS labels and descriptions with TiKV. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - Min Safe TS Region: The ID of the Region whose safe-ts is the minimal | ||
| - Check Leader Duration: The distribution of time spent on processing leader requests. The duration is from sending requests to receiving responses in leader | ||
| - Max gap of resolved-ts in Region leaders: The maximum time difference between the resolved-ts of all active Regions in this TiKV and the current time, only for Region leaders | ||
| - Min Safe TS Follower Region: The ID of the follower Region whose safe-ts is the minimal |
| - Check Leader Duration: The distribution of time spent on processing leader requests. The duration is from sending requests to receiving responses in leader | ||
| - Max gap of resolved-ts in Region leaders: The maximum time difference between the resolved-ts of all active Regions in this TiKV and the current time, only for Region leaders | ||
| - Min Safe TS Follower Region: The ID of the follower Region whose safe-ts is the minimal | ||
| - Check leader duration: The distribution of time spent on processing leader requests. The duration is from sending requests to receiving responses in leader |

First-time contributors' checklist
What is changed, added, or deleted? (Required)
The
Resolved-TSsection ingrafana-tikv-dashboard.mdwas out of sync with current TiKV dashboard titles, including two safe-ts panels whose meanings are now follower-scoped. This update aligns labels and descriptions with the upstream dashboard source.Resolved TS section title and panel title alignment
Resolved-TS→Resolved TSResolved TS Worker CPUAdvance ts Worker CPUScan lock Worker CPUCheck leader durationMax gap of resolved-ts in region leadersMax gap of follower safe-tsMin Safe TS Follower RegionFollower safe-ts semantic correction
Max gap of follower safe-ts(minimum non-zero follower safe-ts gap to current time)Min Safe TS Follower Region(follower Region with minimum safe-ts)Doc snippet (updated section excerpt)
Which 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)?
AI agent involvement
Do your changes match any of the following descriptions?