docs: add PD maintenance guides#10950
Conversation
Signed-off-by: Ryan Leung <rleungx@gmail.com>
ti-chi-bot
Bot
added
do-not-merge/needs-linked-issue
release-note-none
|
[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 |
ti-chi-bot
Bot
added
the
size/XXL
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a repository maintainer index and new subsystem maintenance guides under ChangesPD Maintenance Guides
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (10)
docs/maintenance-guides/repo-overview.md (2)
406-428: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGlossary is helpful but could define additional cross-cutting terms.
Consider adding: "Follower" (used in failure modes and cluster guides), "Epoch" (used in metadata contracts), and "Operator" (though defined, could be expanded). These appear frequently across guides.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/maintenance-guides/repo-overview.md` around lines 406 - 428, The Glossary section in repo-overview.md should be expanded with the missing cross-cutting terms mentioned in the review. Update the existing Glossary list near the current definitions for Operator and Placement rule to add clear entries for Follower and Epoch, and expand the Operator definition so it better matches its usage across the guides.
148-163: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStartup sequencing is accurate but less granular than server.md.
The 7-step sequence here correctly captures the high-level flow. The server.md breaks this into 9 specific anchors. Consider adding a note that server.md contains the detailed sequencing, or align the step count for consistency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/maintenance-guides/repo-overview.md` around lines 148 - 163, The startup sequence in the repo overview is correct but too coarse compared with server.md, so update the classic PD startup section to either note that server.md contains the more detailed 9-step sequencing or revise this list to match that granularity. Use the existing startup/shutdown overview text and the related server lifecycle wording to keep the step count and terminology consistent across both docs.docs/maintenance-guides/server.md (1)
72-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShutdown description omits
hotRegionStorageandmeteringWriter.The
Closecode snippet also closeshotRegionStorageand stopsmeteringWriter, which are not mentioned in the shutdown description. Consider adding them for completeness since they represent persistent state and I/O resources.Shutdown is centered on `Close`, which stops server loops first, then closes -owned managers, clients, embedded etcd member state, heartbeat streams, storage, -rate limiters, callbacks, and client connections. +owned managers, clients, embedded etcd member state, heartbeat streams, storage, +hot-region storage, metering writer, rate limiters, callbacks, and client connections.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/maintenance-guides/server.md` around lines 72 - 74, Update the shutdown description in the Close section to also mention hotRegionStorage and meteringWriter alongside the other owned resources. Keep the wording aligned with the Close shutdown flow so the documentation reflects that hotRegionStorage is closed and meteringWriter is stopped before cleanup finishes.docs/maintenance-guides/README.md (1)
131-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCurrent scope coverage question:
pkg/election/is not listed.The
member-election.mdguide coverspkg/election/(mentioned in its own file), butpkg/election/is absent from the Current Scope list. If this directory is covered by the member-election guide, consider adding it to the scope for completeness.Similarly,
pkg/schedule/placement/,pkg/schedule/labeler/, andpkg/schedule/affinity/are covered byplacement-policy.mdbut not explicitly listed. Consider whether the scope should list these or if the current package-level granularity is intentional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/maintenance-guides/README.md` around lines 131 - 151, The Current Scope list is missing directories that are already covered by the maintenance guides, including pkg/election/ from member-election.md and the pkg/schedule subdirectories covered by placement-policy.md. Update the scope section in README.md to either explicitly add these paths for completeness or clearly document that the list is intentionally package-level and covers those subdirectories implicitly, keeping the wording consistent with the existing scope entries.docs/maintenance-guides/placement-policy.md (2)
89-89: 📐 Maintainability & Code Quality | 🔵 TrivialFix hyphenation: "user-visible".
Change "user visible" to "user-visible" to follow standard hyphenation for compound modifiers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/maintenance-guides/placement-policy.md` at line 89, Fix the wording in the placement-policy documentation to use the standard compound modifier “user-visible” instead of “user visible”. Update the sentence in the policy metadata note so the hyphenation is consistent with the intended terminology and keeps the existing meaning unchanged.Source: Linters/SAST tools
96-96: 📐 Maintainability & Code Quality | 🔵 TrivialFormat directory path consistently.
Line 96 lists
pkg/schedule/placementas a directory while other entries are specific files. Append a trailing slash or list the specific entry file (e.g.,pkg/schedule/placement/rule_manager.go).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/maintenance-guides/placement-policy.md` at line 96, The directory reference in the placement-policy guide is inconsistent with the surrounding file-level entries. Update the `pkg/schedule/placement` entry in the markdown list to either use a trailing slash for the directory or replace it with a specific file path, matching the existing style used elsewhere in the document.docs/maintenance-guides/storage.md (1)
118-118: 📐 Maintainability & Code Quality | 🔵 TrivialFormat directory path consistently.
Line 118 lists
pkg/utils/keypathas a directory while all other entries are specific files. Append a trailing slash to indicate it's a directory, or list the specific entry file (e.g.,pkg/utils/keypath/keypath.go).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/maintenance-guides/storage.md` at line 118, The storage maintenance guide lists pkg/utils/keypath as a directory while the surrounding entries use concrete file paths, so update the referenced item in the guide to be consistent by either adding a trailing slash or replacing it with the specific keypath file name. Locate the list entry by the pkg/utils/keypath symbol in the maintenance guide and make the path format match the other items.docs/maintenance-guides/scheduling.md (1)
51-53: 📐 Maintainability & Code Quality | 🔵 TrivialUse constructor/function anchors for startup sequencing.
Lines 51-53 list type names (
schedulers.Controller,checker.Controller,operator.Controller) rather than initialization points. For consistency with the other anchors and to make this actionable for maintainers, use constructor names likeschedulers.NewControlleror initialization methods likeController.Start().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/maintenance-guides/scheduling.md` around lines 51 - 53, The startup sequencing anchors currently point to controller type names instead of executable initialization points, so update the references in the scheduling guide to use constructor or startup symbols such as schedulers.NewController, checker.NewController, operator.NewController, or Controller.Start() to match the other actionable anchors and help maintainers locate the actual startup flow.docs/maintenance-guides/keyspace-and-microservices.md (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: Title case "And" should be "and" for consistency with standard title case.
Current: "Keyspace And Microservices"
Suggested: "Keyspace and Microservices"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/maintenance-guides/keyspace-and-microservices.md` at line 1, The document title uses inconsistent title case in the Keyspace And Microservices heading. Update the top-level heading in the markdown so it follows standard title case conventions by changing the capitalized conjunction to the lowercase form, and keep the rest of the title unchanged.docs/maintenance-guides/api-and-client.md (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: Title case "And" should be "and" for consistency with standard title case.
Current: "API And Client"
Suggested: "API and Client"This matches typical title case conventions where coordinating conjunctions are lowercase unless they're the first or last word.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/maintenance-guides/api-and-client.md` at line 1, Update the document title in the API and client guide so it follows standard title case conventions by changing “API And Client” to “API and Client.” This is a simple text fix in the heading at the top of the guide, and the unique symbol to locate is the document title “API And Client.”
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/maintenance-guides/api-and-client.md`:
- Line 1: Update the document title in the API and client guide so it follows
standard title case conventions by changing “API And Client” to “API and
Client.” This is a simple text fix in the heading at the top of the guide, and
the unique symbol to locate is the document title “API And Client.”
In `@docs/maintenance-guides/keyspace-and-microservices.md`:
- Line 1: The document title uses inconsistent title case in the Keyspace And
Microservices heading. Update the top-level heading in the markdown so it
follows standard title case conventions by changing the capitalized conjunction
to the lowercase form, and keep the rest of the title unchanged.
In `@docs/maintenance-guides/placement-policy.md`:
- Line 89: Fix the wording in the placement-policy documentation to use the
standard compound modifier “user-visible” instead of “user visible”. Update the
sentence in the policy metadata note so the hyphenation is consistent with the
intended terminology and keeps the existing meaning unchanged.
- Line 96: The directory reference in the placement-policy guide is inconsistent
with the surrounding file-level entries. Update the `pkg/schedule/placement`
entry in the markdown list to either use a trailing slash for the directory or
replace it with a specific file path, matching the existing style used elsewhere
in the document.
In `@docs/maintenance-guides/README.md`:
- Around line 131-151: The Current Scope list is missing directories that are
already covered by the maintenance guides, including pkg/election/ from
member-election.md and the pkg/schedule subdirectories covered by
placement-policy.md. Update the scope section in README.md to either explicitly
add these paths for completeness or clearly document that the list is
intentionally package-level and covers those subdirectories implicitly, keeping
the wording consistent with the existing scope entries.
In `@docs/maintenance-guides/repo-overview.md`:
- Around line 406-428: The Glossary section in repo-overview.md should be
expanded with the missing cross-cutting terms mentioned in the review. Update
the existing Glossary list near the current definitions for Operator and
Placement rule to add clear entries for Follower and Epoch, and expand the
Operator definition so it better matches its usage across the guides.
- Around line 148-163: The startup sequence in the repo overview is correct but
too coarse compared with server.md, so update the classic PD startup section to
either note that server.md contains the more detailed 9-step sequencing or
revise this list to match that granularity. Use the existing startup/shutdown
overview text and the related server lifecycle wording to keep the step count
and terminology consistent across both docs.
In `@docs/maintenance-guides/scheduling.md`:
- Around line 51-53: The startup sequencing anchors currently point to
controller type names instead of executable initialization points, so update the
references in the scheduling guide to use constructor or startup symbols such as
schedulers.NewController, checker.NewController, operator.NewController, or
Controller.Start() to match the other actionable anchors and help maintainers
locate the actual startup flow.
In `@docs/maintenance-guides/server.md`:
- Around line 72-74: Update the shutdown description in the Close section to
also mention hotRegionStorage and meteringWriter alongside the other owned
resources. Keep the wording aligned with the Close shutdown flow so the
documentation reflects that hotRegionStorage is closed and meteringWriter is
stopped before cleanup finishes.
In `@docs/maintenance-guides/storage.md`:
- Line 118: The storage maintenance guide lists pkg/utils/keypath as a directory
while the surrounding entries use concrete file paths, so update the referenced
item in the guide to be consistent by either adding a trailing slash or
replacing it with the specific keypath file name. Locate the list entry by the
pkg/utils/keypath symbol in the maintenance guide and make the path format match
the other items.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 79f48df0-bbbd-4f29-bcce-b35a175d6042
📒 Files selected for processing (14)
docs/maintenance-guides/README.mddocs/maintenance-guides/api-and-client.mddocs/maintenance-guides/cluster.mddocs/maintenance-guides/config.mddocs/maintenance-guides/keyspace-and-microservices.mddocs/maintenance-guides/member-election.mddocs/maintenance-guides/placement-policy.mddocs/maintenance-guides/repo-overview.mddocs/maintenance-guides/resource-manager.mddocs/maintenance-guides/scheduling.mddocs/maintenance-guides/server.mddocs/maintenance-guides/statistics.mddocs/maintenance-guides/storage.mddocs/maintenance-guides/tso.md
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10950 +/- ##
==========================================
- Coverage 79.21% 79.19% -0.03%
==========================================
Files 541 541
Lines 75112 75114 +2
==========================================
- Hits 59500 59483 -17
- Misses 11411 11430 +19
Partials 4201 4201
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx
force-pushed
the
docs-maintenance-guides
branch
from
345744e to
4c9dc1d
Compare
|
@rleungx: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
rleungx
force-pushed
the
docs-maintenance-guides
branch
from
4c9dc1d to
cf9b307
Compare
|
[FORMAT CHECKER NOTIFICATION] Notice: To remove the 📖 For more info, you can check the "Linking issues" section in the CONTRIBUTING.md. |
okJiang
left a comment
okJiang
left a comment
There was a problem hiding this comment.
Should the docs for different components be placed in their respective code directories?
The root directory may only need to contain high-level architectural designs.
2 participants
What problem does this PR solve?
Issue Number:
This PR adds a maintainer-oriented guide set for the PD repository, following the structure of the TiKV maintenance guide work and adapting it to PD subsystem boundaries.
What is changed and how does it work?
Check List
Tests
Release note
Summary by CodeRabbit