-
Notifications
You must be signed in to change notification settings - Fork 359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update systemd.md #1639
base: main
Are you sure you want to change the base?
Update systemd.md #1639
Conversation
Modify the bridge node guide (LimitNOFILE) according to celestiaorg#1633
WalkthroughThe Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- nodes/systemd.md (1 hunks)
Additional context used
LanguageTool
nodes/systemd.md
[grammar] ~135-~135: The verb form ‘value’ does not seem to be suitable in this context.
Context: ...ng file descriptor limits. Setting this value too high might affect system performanc...(THIS_VBP_DT)
[uncategorized] ~137-~137: Possible missing comma found.
Context: ...any-open-files If the file was created successfully you will be able to see its content: `...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
nodes/systemd.md
134-134: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
135-135: null
Bare URL used(MD034, no-bare-urls)
Additional comments not posted (1)
nodes/systemd.md (1)
129-129
: Approved: UpdateLimitNOFILE
to 65535.The change from
1400000
to65535
aligns with the reference provided in issue #1633. Ensure this value is appropriate for all node types and does not impact performance.
nodes/systemd.md
Outdated
|
||
[Install] | ||
WantedBy=multi-user.target | ||
EOF | ||
``` | ||
Note: Be cautious when increasing file descriptor limits. Setting this value too high might affect system performance. Ensure the value is appropriate for your system's capabilities. https://docs.celestia.org/nodes/celestia-node-troubleshooting#error-too-many-open-files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve clarity, grammar, and formatting of the note.
The note can be improved for better clarity and grammar. Additionally, the bare URL should be formatted properly.
- Note: Be cautious when increasing file descriptor limits. Setting this value too high might affect system performance. Ensure the value is appropriate for your system's capabilities. https://docs.celestia.org/nodes/celestia-node-troubleshooting#error-too-many-open-files
+ **Note:** Be cautious when increasing file descriptor limits. Setting this value too high might affect system performance. Ensure the value is appropriate for your system's capabilities. For more information, refer to the [Celestia Node Troubleshooting Guide](https://docs.celestia.org/nodes/celestia-node-troubleshooting#error-too-many-open-files).
[grammar] [MD034]
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Note: Be cautious when increasing file descriptor limits. Setting this value too high might affect system performance. Ensure the value is appropriate for your system's capabilities. https://docs.celestia.org/nodes/celestia-node-troubleshooting#error-too-many-open-files | |
Note: Be cautious when increasing file descriptor limits. Setting this value too high might affect system performance. Ensure the value is appropriate for your system's capabilities. For more information, refer to the [Celestia Node Troubleshooting Guide](https://docs.celestia.org/nodes/celestia-node-troubleshooting#error-too-many-open-files). |
Tools
LanguageTool
[grammar] ~135-~135: The verb form ‘value’ does not seem to be suitable in this context.
Context: ...ng file descriptor limits. Setting this value too high might affect system performanc...(THIS_VBP_DT)
Markdownlint
135-135: null
Bare URL used(MD034, no-bare-urls)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- nodes/systemd.md (1 hunks)
Additional context used
LanguageTool
nodes/systemd.md
[grammar] ~135-~135: The verb form ‘value’ does not seem to be suitable in this context.
Context: ...ng file descriptor limits. Setting this value too high might affect system performanc...(THIS_VBP_DT)
[uncategorized] ~137-~137: Possible missing comma found.
Context: ...-open-files). If the file was created successfully you will be able to see its content: `...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
nodes/systemd.md
135-135: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
134-134: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
Additional comments not posted (2)
nodes/systemd.md (2)
129-129
: Verify the appropriateness of the newLimitNOFILE
value.The
LimitNOFILE
value for the bridge node has been changed from1400000
to65535
. Ensure this value is suitable for the bridge node's requirements and does not affect its performance.
135-135
: Improve clarity, grammar, and formatting of the note.The note can be improved for better clarity and grammar. Additionally, the bare URL should be formatted properly.
[grammar] [MD034]- Note: You can set LimitNOFILE up to 1400000. Be cautious when increasing file descriptor limits. Setting this value too high might affect system performance. [Ensure the value is appropriate for your system's capabilities](https://docs.celestia.org/nodes/celestia-node-troubleshooting#error-too-many-open-files). + **Note:** You can set `LimitNOFILE` up to 1,400,000. Be cautious when increasing file descriptor limits. Setting this value too high might affect system performance. For more information, refer to the [Celestia Node Troubleshooting Guide](https://docs.celestia.org/nodes/celestia-node-troubleshooting#error-too-many-open-files).Tools
LanguageTool
[grammar] ~135-~135: The verb form ‘value’ does not seem to be suitable in this context.
Context: ...ng file descriptor limits. Setting this value too high might affect system performanc...(THIS_VBP_DT)
Markdownlint
135-135: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
cc @jbowen93 |
reverts #1407 |
@cryptomolot do you want to also change the other instances of 140000? |
Sure! Sorry for late response Will do |
nbd! thanks @cryptomolot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- nodes/systemd.md (1 hunks)
Additional context used
LanguageTool
nodes/systemd.md
[grammar] ~135-~135: The verb form ‘value’ does not seem to be suitable in this context.
Context: ...ng file descriptor limits. Setting this value too high might affect system performanc...(THIS_VBP_DT)
Additional comments not posted (2)
nodes/systemd.md (2)
Line range hint
25-25
: Approved: UpdatedLimitNOFILE
setting for celestia-appd service.The change from a higher limit to
65535
aligns with standard practices and enhances system safety and performance.Tools
LanguageTool
[grammar] ~135-~135: The verb form ‘value’ does not seem to be suitable in this context.
Context: ...ng file descriptor limits. Setting this value too high might affect system performanc...(THIS_VBP_DT)
129-129
: Approved: UpdatedLimitNOFILE
setting for celestia-bridge service.The reduction to
65535
is a prudent adjustment that should help in maintaining a balance between functionality and system resource limits.
|
||
[Install] | ||
WantedBy=multi-user.target | ||
EOF | ||
``` | ||
Note: You can set LimitNOFILE up to 1400000. Be cautious when increasing file descriptor limits. Setting this value too high might affect system performance. [Ensure the value is appropriate for your system's capabilities](https://docs.celestia.org/nodes/celestia-node-troubleshooting#error-too-many-open-files). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve clarity, grammar, and formatting of the note.
The note can be improved for better clarity and grammar. Additionally, the bare URL should be formatted properly.
Apply this diff to improve the note:
- Note: You can set LimitNOFILE up to 1400000. Be cautious when increasing file descriptor limits. Setting this value too high might affect system performance. [Ensure the value is appropriate for your system's capabilities](https://docs.celestia.org/nodes/celestia-node-troubleshooting#error-too-many-open-files).
+ **Note:** Be cautious when increasing file descriptor limits. Setting this value too high might affect system performance. Ensure the value is appropriate for your system's capabilities. For more information, refer to the [Celestia Node Troubleshooting Guide](https://docs.celestia.org/nodes/celestia-node-troubleshooting#error-too-many-open-files).
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Note: You can set LimitNOFILE up to 1400000. Be cautious when increasing file descriptor limits. Setting this value too high might affect system performance. [Ensure the value is appropriate for your system's capabilities](https://docs.celestia.org/nodes/celestia-node-troubleshooting#error-too-many-open-files). | |
**Note:** Be cautious when increasing file descriptor limits. Setting this value too high might affect system performance. Ensure the value is appropriate for your system's capabilities. For more information, refer to the [Celestia Node Troubleshooting Guide](https://docs.celestia.org/nodes/celestia-node-troubleshooting#error-too-many-open-files). |
Tools
LanguageTool
[grammar] ~135-~135: The verb form ‘value’ does not seem to be suitable in this context.
Context: ...ng file descriptor limits. Setting this value too high might affect system performanc...(THIS_VBP_DT)
cc @Wondertan and @renaynay for review here please 🙏 |
Modify the bridge node guide
Changing LimitNOFILE number according to #1633
Summary by CodeRabbit
LimitNOFILE
value in system configuration from1400000
to65535
to ensure compatibility and stability.