Conversation
|
Warning Review limit reachedNext included review available in 55 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe documentation updates ChangesIndex usage documentation
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~3 minutes Change: Other Merge Risk: 🔵 Low · up to The section gives conflicting restart scopes, which can lead readers to misunderstand when an index is considered unused. Align the nearby wording before merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
[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 |
The sys.schema_unused_indexes view (see sys-schema/sys-schema-unused-indexes.md) is defined as SELECT ... FROM information_schema.cluster_tidb_index_usage ... GROUP BY ... HAVING sum(last_access_time) is null, i.e. it is derived from CLUSTER_TIDB_INDEX_USAGE (which aggregates usage across all TiDB instances), not from the single-instance TIDB_INDEX_USAGE. It also flags an index only once none of the cluster's instances have recorded access since their own last restart, not a single cluster-wide restart event. Found while reviewing the Japanese translation of this file, which faithfully mirrored the inaccurate EN wording. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3a21f13 to
492280b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Advanced
Run ID: d80c7505-ee78-4368-ac73-df6bd4bab750
📒 Files selected for processing (1)
best-practices/index-management-best-practices.md
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.
…indexes CLUSTER_TIDB_INDEX_USAGE aggregates across all TiDB instances, and an index counts as unused only when no instance has recorded access since that instance's own last restart, not since a single cluster-wide restart. Align the intro sentence and considerations heading with the per-instance wording already used elsewhere in this section. Found by CodeRabbit review on PR pingcap#23891. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
What is changed, added or deleted? (Required)
best-practices/index-management-best-practices.md's "How schema_unused_indexes works" section said the view is derived fromTIDB_INDEX_USAGEand filters out indexes with zero activity "since the last TiDB restart" (implying a single cluster-wide restart).The actual view definition (see
sys-schema/sys-schema-unused-indexes.md) is:This reads from
CLUSTER_TIDB_INDEX_USAGE(which aggregates usage across all TiDB instances), not from the single-instanceTIDB_INDEX_USAGE. It flags an index as unused only when none of the cluster's instances have recorded access since that instance's own last restart — not a single cluster-wide restart event.Found while reviewing the Japanese translation of this file (which faithfully mirrored the inaccurate EN wording) — flagged by a CodeRabbit review on the JA PR, verified against the real view definition before filing this fix.
Which TiDB version(s) do your changes apply to? (Required)
What is the related PR or file link(s)?
AI agent involvement
Do your changes match any of the following descriptions?
🤖 Generated with Claude Code
Summary by CodeRabbit