feat: add InitLimit #346
Merged
feat: add InitLimit #346
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
流程概要在 变更
时序图sequenceDiagram
participant Main
participant InitLimit
Main->>InitLimit: 调用 InitLimit()
InitLimit->>System: 获取当前文件限制
System->>InitLimit: 返回文件限制
InitLimit->>System: 调整文件限制（如果需要）
System->>InitLimit: 确认新文件限制
InitLimit-->>Main: 完成调用
Main->>Logs: 初始化日志
与关联问题的评估
诗
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/pikiwidb.cc (2 hunks)
Additional comments not posted (1)
src/pikiwidb.cc (1)
282-282:InitLimit函数的调用位置适当，确保在服务器初始化之前设置文件描述符限制。
src/pikiwidb.cc
Outdated
Comment on lines
221
to
238
| static int InitLimit() { | ||
| rlimit limit; | ||
| rlim_t maxfiles = g_config.max_clients; | ||
| if (getrlimit(RLIMIT_NOFILE, &limit) == -1) { | ||
| WARN("getrlimit error: ", strerror(errno)); | ||
| } else if (limit.rlim_cur < maxfiles) { | ||
| rlim_t old_limit = limit.rlim_cur; | ||
| limit.rlim_cur = maxfiles; | ||
| limit.rlim_max = maxfiles; | ||
| if (setrlimit(RLIMIT_NOFILE, &limit) != -1) { | ||
| WARN( "your 'limit -n ' of ", old_limit, " is not enough for PikiwiDB to start. PikiwiDB have successfully reconfig it to " , limit.rlim_cur); | ||
| } else { | ||
| ERROR("your 'limit -n ' of " , old_limit, " is not enough for PikiwiDB to start. PikiwiDB can not reconfig it(", strerror(errno), "), do it by yourself"); | ||
| return -1; | ||
| } | ||
| } | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
调整文件描述符限制的逻辑看起来是正确的，但是在日志消息中使用逗号连接字符串可能会导致问题。建议使用格式化字符串。
- WARN("getrlimit error: ", strerror(errno));
+ WARN("getrlimit error: %s", strerror(errno));
- WARN( "your 'limit -n ' of ", old_limit, " is not enough for PikiwiDB to start. PikiwiDB have successfully reconfig it to " , limit.rlim_cur);
+ WARN("your 'limit -n ' of %ld is not enough for PikiwiDB to start. PikiwiDB have successfully reconfig it to %ld", old_limit, limit.rlim_cur);
- ERROR("your 'limit -n ' of " , old_limit, " is not enough for PikiwiDB to start. PikiwiDB can not reconfig it(", strerror(errno), "), do it by yourself");
+ ERROR("your 'limit -n ' of %ld is not enough for PikiwiDB to start. PikiwiDB can not reconfig it(%s), do it by yourself", old_limit, strerror(errno));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.
Suggested change
| static int InitLimit() { | |
| rlimit limit; | |
| rlim_t maxfiles = g_config.max_clients; | |
| if (getrlimit(RLIMIT_NOFILE, &limit) == -1) { | |
| WARN("getrlimit error: ", strerror(errno)); | |
| } else if (limit.rlim_cur < maxfiles) { | |
| rlim_t old_limit = limit.rlim_cur; | |
| limit.rlim_cur = maxfiles; | |
| limit.rlim_max = maxfiles; | |
| if (setrlimit(RLIMIT_NOFILE, &limit) != -1) { | |
| WARN( "your 'limit -n ' of ", old_limit, " is not enough for PikiwiDB to start. PikiwiDB have successfully reconfig it to " , limit.rlim_cur); | |
| } else { | |
| ERROR("your 'limit -n ' of " , old_limit, " is not enough for PikiwiDB to start. PikiwiDB can not reconfig it(", strerror(errno), "), do it by yourself"); | |
| return -1; | |
| } | |
| } | |
| return 0; | |
| } | |
| static int InitLimit() { | |
| rlimit limit; | |
| rlim_t maxfiles = g_config.max_clients; | |
| if (getrlimit(RLIMIT_NOFILE, &limit) == -1) { | |
| WARN("getrlimit error: %s", strerror(errno)); | |
| } else if (limit.rlim_cur < maxfiles) { | |
| rlim_t old_limit = limit.rlim_cur; | |
| limit.rlim_cur = maxfiles; | |
| limit.rlim_max = maxfiles; | |
| if (setrlimit(RLIMIT_NOFILE, &limit) != -1) { | |
| WARN("your 'limit -n ' of %ld is not enough for PikiwiDB to start. PikiwiDB have successfully reconfig it to %ld", old_limit, limit.rlim_cur); | |
| } else { | |
| ERROR("your 'limit -n ' of %ld is not enough for PikiwiDB to start. PikiwiDB can not reconfig it(%s), do it by yourself", old_limit, strerror(errno)); | |
| return -1; | |
| } | |
| } | |
| return 0; | |
| } |
There was a problem hiding this comment.
@luky116, 感谢您的修改！如果有任何其他问题或需要进一步的帮助，请随时告知。
Learnings added
Learnt from: luky116
PR: OpenAtomFoundation/pikiwidb#346
File: src/pikiwidb.cc:0-0
Timestamp: 2024-07-01T23:31:59.313Z
Learning: When suggesting changes to log messages, ensure to use formatted strings instead of concatenation for better readability and to avoid potential issues.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
added 2 commits
June 10, 2024 14:56
AlexStocks
reviewed
Jun 13, 2024
|
请及时处理 Review 的comment，并更新最新代码 |
Please update the latest code in time |
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/pikiwidb.cc (3 hunks)
Additional comments not posted (2)
src/pikiwidb.cc (2)
15-15: 包含<sys/resource.h>是必要的。此包含是为了处理系统资源限制所必需的。
288-288: 在main函数中调用InitLimit是合理的。在初始化日志之前调用
InitLimit函数，以确保 PikiwiDB 运行时具有所需的资源限制。
src/pikiwidb.cc
Outdated
Comment on lines
222
to
244
| static int InitLimit() { | ||
| rlimit limit; | ||
| rlim_t maxfiles = g_config.max_clients; | ||
| if (getrlimit(RLIMIT_NOFILE, &limit) == -1) { | ||
| WARN("getrlimit error: {}", strerror(errno)); | ||
| } else if (limit.rlim_cur < maxfiles) { | ||
| rlim_t old_limit = limit.rlim_cur; | ||
| limit.rlim_cur = maxfiles; | ||
| limit.rlim_max = maxfiles; | ||
| if (setrlimit(RLIMIT_NOFILE, &limit) != -1) { | ||
| WARN("your 'limit -n ' of {} is not enough for PikiwiDB to start. PikiwiDB have successfully reconfig it to ", | ||
| old_limit, limit.rlim_cur); | ||
| } else { | ||
| ERROR( | ||
| "your 'limit -n ' of {} is not enough for PikiwiDB to start. PikiwiDB can not reconfig it({}), do it by " | ||
| "yourself", | ||
| old_limit, strerror(errno)); | ||
|
|
||
| return -1; | ||
| } | ||
| } | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
改进日志消息的格式化。
当前使用字符串连接的方式记录日志，建议使用格式化字符串以提高可读性和性能。
- WARN("getrlimit error: ", strerror(errno));
+ WARN("getrlimit error: %s", strerror(errno));
- WARN( "your 'limit -n ' of ", old_limit, " is not enough for PikiwiDB to start. PikiwiDB have successfully reconfig it to " , limit.rlim_cur);
+ WARN("your 'limit -n ' of %ld is not enough for PikiwiDB to start. PikiwiDB have successfully reconfig it to %ld", old_limit, limit.rlim_cur);
- ERROR("your 'limit -n ' of " , old_limit, " is not enough for PikiwiDB to start. PikiwiDB can not reconfig it(", strerror(errno), "), do it by yourself");
+ ERROR("your 'limit -n ' of %ld is not enough for PikiwiDB to start. PikiwiDB can not reconfig it(%s), do it by yourself", old_limit, strerror(errno));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.
Suggested change
| static int InitLimit() { | |
| rlimit limit; | |
| rlim_t maxfiles = g_config.max_clients; | |
| if (getrlimit(RLIMIT_NOFILE, &limit) == -1) { | |
| WARN("getrlimit error: {}", strerror(errno)); | |
| } else if (limit.rlim_cur < maxfiles) { | |
| rlim_t old_limit = limit.rlim_cur; | |
| limit.rlim_cur = maxfiles; | |
| limit.rlim_max = maxfiles; | |
| if (setrlimit(RLIMIT_NOFILE, &limit) != -1) { | |
| WARN("your 'limit -n ' of {} is not enough for PikiwiDB to start. PikiwiDB have successfully reconfig it to ", | |
| old_limit, limit.rlim_cur); | |
| } else { | |
| ERROR( | |
| "your 'limit -n ' of {} is not enough for PikiwiDB to start. PikiwiDB can not reconfig it({}), do it by " | |
| "yourself", | |
| old_limit, strerror(errno)); | |
| return -1; | |
| } | |
| } | |
| return 0; | |
| } | |
| static int InitLimit() { | |
| rlimit limit; | |
| rlim_t maxfiles = g_config.max_clients; | |
| if (getrlimit(RLIMIT_NOFILE, &limit) == -1) { | |
| WARN("getrlimit error: %s", strerror(errno)); | |
| } else if (limit.rlim_cur < maxfiles) { | |
| rlim_t old_limit = limit.rlim_cur; | |
| limit.rlim_cur = maxfiles; | |
| limit.rlim_max = maxfiles; | |
| if (setrlimit(RLIMIT_NOFILE, &limit) != -1) { | |
| WARN("your 'limit -n ' of %ld is not enough for PikiwiDB to start. PikiwiDB have successfully reconfig it to %ld", old_limit, limit.rlim_cur); | |
| } else { | |
| ERROR("your 'limit -n ' of %ld is not enough for PikiwiDB to start. PikiwiDB can not reconfig it(%s), do it by yourself", old_limit, strerror(errno)); | |
| return -1; | |
| } | |
| } | |
| return 0; | |
| } |
嗯嗯，已处理 |
OK, processed |
AlexStocks
previously approved these changes
Jul 2, 2024
AlexStocks
previously approved these changes
Jul 2, 2024
AlexStocks
approved these changes
Jul 3, 2024
panlei-coder
approved these changes
Jul 5, 2024
chanfun-ren
pushed a commit
to chanfun-ren/pikiwidb
that referenced
this pull request
Jul 20, 2024
* add InitLimit
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix #315
Summary by CodeRabbit