tiup: clarify limits.conf behavior in no-sudo mode - #23120
Conversation
|
Hi @doc-claw-bot. 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. |
There was a problem hiding this comment.
Code Review
This pull request updates the documentation for TiDB in no-sudo mode to explain how to apply limits from /etc/security/limits.conf by restarting the per-user systemd instance. The review feedback suggests improving clarity and readability by using active voice, making the shell code block more copy-paste friendly by moving the conditional restart explanation into the text, and changing "might not take effect" to "will not take effect" for technical accuracy.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| 2. Use the `root` user to start the user service. If `user@${uid}.service` is already running after you update `/etc/security/limits.conf`, run `systemctl restart user@${uid}.service` to apply the new limits. | ||
|
|
||
| ```shell | ||
| $ uid=$(id -u tidb) # Get the ID of the tidb user | ||
| $ systemctl start user@${uid}.service | ||
| # If user@${uid}.service is already running after you update /etc/security/limits.conf, run: | ||
| $ systemctl restart user@${uid}.service | ||
| $ systemctl status user@${uid}.service |
There was a problem hiding this comment.
Including both start and restart commands with a conditional comment inside a single shell code block makes it cluttered and less copy-paste friendly (users copy-pasting the block will run both commands sequentially). It is cleaner to explain the restart scenario in the text and keep the code block focused on the standard start/status commands.
| 2. Use the `root` user to start the user service. If `user@${uid}.service` is already running after you update `/etc/security/limits.conf`, run `systemctl restart user@${uid}.service` to apply the new limits. | |
| ```shell | |
| $ uid=$(id -u tidb) # Get the ID of the tidb user | |
| $ systemctl start user@${uid}.service | |
| # If user@${uid}.service is already running after you update /etc/security/limits.conf, run: | |
| $ systemctl restart user@${uid}.service | |
| $ systemctl status user@${uid}.service | |
| 2. Use the `root` user to start the user service. If `user@${uid}.service` is already running and you have updated `/etc/security/limits.conf`, run `systemctl restart user@${uid}.service` instead of `systemctl start user@${uid}.service` to apply the new limits. | |
| ```shell | |
| $ uid=$(id -u tidb) # Get the ID of the tidb user | |
| $ systemctl start user@${uid}.service | |
| $ systemctl status user@${uid}.service |
References
- Ensure clarity, simplicity, and readability, and avoid unnecessary words and repetition. (link)
| EOF | ||
| ``` | ||
|
|
||
| Because TiDB services in no-sudo mode are managed by the per-user `systemd` instance (`user@<UID>.service`), the limits in `/etc/security/limits.conf` are applied when `user@<UID>.service` starts. If you change `/etc/security/limits.conf` after `user@<UID>.service` is already running, restart `user@<UID>.service` or reboot the target machine before you deploy or restart TiDB services. Otherwise, the new limits might not take effect for the TiDB services. |
There was a problem hiding this comment.
To improve clarity and accuracy, use active voice ("you must restart") and change "might not take effect" to "will not take effect" (since systemd will definitely not apply the new limits without a restart).
| Because TiDB services in no-sudo mode are managed by the per-user `systemd` instance (`user@<UID>.service`), the limits in `/etc/security/limits.conf` are applied when `user@<UID>.service` starts. If you change `/etc/security/limits.conf` after `user@<UID>.service` is already running, restart `user@<UID>.service` or reboot the target machine before you deploy or restart TiDB services. Otherwise, the new limits might not take effect for the TiDB services. | |
| Because TiDB services in no-sudo mode are managed by the per-user `systemd` instance (`user@<UID>.service`), the limits in `/etc/security/limits.conf` only take effect when `user@<UID>.service` starts. If you change `/etc/security/limits.conf` after `user@<UID>.service` is already running, you must restart `user@<UID>.service` or reboot the target machine before deploying or restarting TiDB services. Otherwise, the new limits will not take effect. |
References
- Write in second person ('you'), prefer present tense, and avoid unnecessary words and repetition. (link)
|
/ok-to-test |
There was a problem hiding this comment.
Pull request overview
This PR clarifies how file descriptor limits are applied in TiUP no-sudo mode, specifically how user@<UID>.service (the per-user systemd manager) affects the effective Max open files limit for TiDB services.
Changes:
- Adds an explanation of why
/etc/security/limits.confmight not fully take effect for the running user systemd manager. - Adds steps to restart
user@${uid}.service(when applicable) and verify the effectiveMax open filesvalue via/proc/<pid>/limits. - Documents a fallback using a system-level drop-in to set
LimitNOFILE=1000000foruser@${uid}.service, plus an operational warning about the restart impact.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| If you update `/etc/security/limits.conf` after `user@${uid}.service` is already running, restart `user@${uid}.service` before you deploy or restart TiDB services so that the user `systemd` instance reloads the updated resource limits. Restarting `user@${uid}.service` stops all systemd user services managed by that user. If the cluster is already running, stop it gracefully first or perform this operation during a maintenance window. | ||
|
|
||
| ```shell | ||
| $ uid=$(id -u tidb) # Get the ID of the tidb user | ||
| $ systemctl restart user@${uid}.service | ||
| ``` |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@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 |
|
Warning Review limit reached
Next review available in: 46 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe no-sudo mode guide now explains how to restart, inspect, and configure the user systemd service’s effective file descriptor limit before validating TiDB startup. ChangesNo-sudo systemd setup
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested reviewers: 🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1f2c60ae-d9c7-425a-9763-650e96060c85
📒 Files selected for processing (1)
tiup/tiup-cluster-no-sudo-mode.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tiup/tiup-cluster-no-sudo-mode.md (1)
109-109: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRun a concrete user-manager command.
systemctl --useris not a valid action; it only selects the user manager. Use a real command such assystemctl --user statusorsystemctl --user list-unitsso this step fails when the user service mode is not reachable.Source: MCP tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3630c65a-b268-4831-82e4-b7d4b388a789
📒 Files selected for processing (1)
tiup/tiup-cluster-no-sudo-mode.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
What is changed, added or deleted? (Required)
This PR updates the TiUP no-sudo documentation to clarify how resource limits take effect for the per-user
systemdmanager./etc/security/limits.confmight be applied through PAM whenuser@<UID>.servicestarts, but the effectiveMax open filesvalue still depends on the limits obtained by the user manager itself.Max open filesvalue of the runninguser@${uid}.service.user@${uid}.servicewithLimitNOFILE=1000000.user@${uid}.servicestops all user services managed by that user, so existing clusters should be stopped first or handled in a maintenance window.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 (in Chinese).
What is the related PR or file link(s)?
Do your changes match any of the following descriptions?
Summary by CodeRabbit
/etc/security/limits.confmay not raise effective open-file limits when systemd user instance limits apply.user@<UID>.serviceafter changes and verifyingMax open filesvia/proc/<MainPID>/limits.LimitNOFILE=1000000) when the hard limit is too low, followed bydaemon-reload, restart, and re-verification.systemctl --usershows the user service started successfully.