fix: Resolve the inconsistency between member configuration and log index in snapshot meta #386
Conversation
…ndex in snapshot meta
|
Warning Rate limit exceeded@panlei-coder has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 5 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Walkthrough此次更改涉及多个文件，主要集中在外部项目配置的更新、功能的新增以及现有方法的优化。 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 as PR comments)
Additionally, you can add CodeRabbit Configuration 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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- cmake/braft.cmake (1 hunks)
- pikiwidb.conf (1 hunks)
- save_load.sh (1 hunks)
- src/praft/praft.cc (2 hunks)
- src/praft/praft.h (2 hunks)
- src/praft/psnapshot.cc (2 hunks)
Files skipped from review due to trivial changes (2)
- pikiwidb.conf
- save_load.sh
Additional comments not posted (8)
cmake/braft.cmake (2)
23-23: 更新外部项目的Git标签已将
braft项目的Git标签从v1.1.2-alpha2更改为get_configuration。请确保新标签指向正确的版本，并且该版本包含所有必要的功能和修复。
22-22: 更新外部项目的Git仓库已将
braft项目的Git仓库从https://github.com/pikiwidb/braft.git更改为https://github.com/panlei-coder/braft.git。请确保新仓库是可信的，并且包含所需的所有依赖项和功能。Verification successful
更新外部项目的Git仓库
已验证
braft项目的新Git仓库https://github.com/panlei-coder/braft.git。该仓库包含所有必要的文件和依赖项，结构完整。
- 仓库包含构建脚本、源代码、文档、示例和测试文件。
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new Git repository URL. # Test: Clone the repository to check if it's accessible and contains the necessary files. git clone https://github.com/panlei-coder/braft.gitLength of output: 139
Script:
#!/bin/bash # Description: Verify the contents of the cloned repository to ensure it includes necessary files and dependencies. # List the contents of the cloned repository ls -R braftLength of output: 3984
src/praft/psnapshot.cc (2)
13-14: 包含新头文件引入了
braft/configuration.h和braft/configuration_manager.h头文件。这些文件用于处理配置相关的操作。
84-106: 更新快照元数据的逻辑更新了快照的最后日志索引、最后日志任期以及配置和学习者信息。这些更改确保了快照元数据准确反映当前的配置状态，改进了系统的可靠性和一致性。
src/praft/praft.h (2)
17-17: 包含新头文件引入了
braft/configuration_manager.h头文件，用于管理配置。
144-145: 新增方法：GetConfigurationByIndex在
PRaft类中新增了GetConfigurationByIndex方法，该方法根据日志索引获取配置和学习者配置。这增强了类的功能，使其能够更动态地访问配置条目。src/praft/praft.cc (2)
676-678: 代码看起来不错！但请验证新方法的使用情况。代码更改已批准。
请确保新方法
set_last_applied_index_and_term已正确实现并使用。
262-270: 代码看起来不错！但请验证函数的使用情况。代码更改已批准。
请确保在调用此函数之前，
node_已正确初始化。
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- build.sh (1 hunks)
- pikiwidb.conf (1 hunks)
- src/pikiwidb.cc (2 hunks)
Files skipped from review due to trivial changes (2)
- build.sh
- pikiwidb.conf
Additional comments not posted (2)
src/pikiwidb.cc (2)
230-230: 更改警告信息以提高清晰度警告信息现在包含了当前限制值的占位符，这有助于用户了解PikiwiDB重新配置的具体限制。
285-285: 验证注释掉的InitLimit()调用的影响
InitLimit()调用被注释掉，表明在程序启动时存在导致“直接段错误”的问题。请确保此更改不会影响程序正确处理文件描述符的能力。Verification successful
验证注释掉的
InitLimit()调用的影响
InitLimit()调用被注释掉，表明在程序启动时存在导致“直接段错误”的问题。通过检查代码库，发现src/pstd/env.cc文件中有类似的逻辑来设置文件描述符限制。这表明可能存在替代机制或后备方法来设置这些限制，从而减轻了注释掉InitLimit()的影响。
src/pikiwidb.cc: 包含InitLimit()函数的定义和逻辑。src/pstd/env.cc: 包含类似的文件描述符限制设置逻辑。请确保这些替代机制能够正确处理文件描述符限制，以避免潜在问题。
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of commenting out `InitLimit()` on program startup behavior. # Test: Search for the `InitLimit` function usage. Expect: Only occurrences of the commented-out function call. rg --type cpp -A 5 $'InitLimit'Length of output: 609
Script:
#!/bin/bash # Description: Search for alternative methods or configurations related to setting resource limits, particularly for file descriptors. # Test: Search for usage of `rlimit` and `RLIMIT_NOFILE` across the codebase to identify any other mechanisms for setting file descriptor limits. rg --type cpp -A 5 'rlimit|RLIMIT_NOFILE'Length of output: 1545
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- cmake/braft.cmake (1 hunks)
Additional comments not posted (1)
cmake/braft.cmake (1)
23-23: 更改 Git 标签以使用更稳定的版本。将 Git 标签从
v1.1.2-alpha2更新为v1.1.2.1，这表明转向了一个更稳定或最终的版本。这有助于提高项目的稳定性和功能性。
| auto& new_meta = const_cast<braft::SnapshotMeta&>(snapshot_meta_memtable.meta()); | ||
| auto last_log_index = PSTORE.GetBackend(db_id)->GetStorage()->GetSmallestFlushedLogIndex(); | ||
| new_meta.set_last_included_index(last_log_index); | ||
| auto last_log_term = PRAFT.GetTerm(last_log_index); | ||
| new_meta.set_last_included_term(last_log_term); | ||
| braft::ConfigurationEntry conf_entry; |
src/praft/psnapshot.cc
Outdated
| braft::ConfigurationEntry learner_conf_entry; | ||
| PRAFT.GetConfigurationByIndex(last_log_index, &conf_entry, &learner_conf_entry); | ||
| new_meta.clear_peers(); | ||
| for (braft::Configuration::const_iterator iter = conf_entry.conf.begin(); iter != conf_entry.conf.end(); ++iter) { |
| // update snapshot last log index, last_log_term, conf of last log index and learners | ||
| auto& new_meta = const_cast<braft::SnapshotMeta&>(snapshot_meta_memtable.meta()); | ||
| auto last_log_index = PSTORE.GetBackend(db_id)->GetStorage()->GetSmallestFlushedLogIndex(); | ||
| new_meta.set_last_included_index(last_log_index); | ||
| auto last_log_term = PRAFT.GetTerm(last_log_index); | ||
| new_meta.set_last_included_term(last_log_term); | ||
| braft::ConfigurationEntry conf_entry; | ||
| braft::ConfigurationEntry learner_conf_entry; | ||
| PRAFT.GetConfigurationByIndex(last_log_index, &conf_entry, &learner_conf_entry); | ||
| new_meta.clear_peers(); | ||
| for (auto iter = conf_entry.conf.begin(); iter != conf_entry.conf.end(); ++iter) { | ||
| *new_meta.add_peers() = iter->to_string(); | ||
| } | ||
| new_meta.clear_old_peers(); | ||
| for (auto iter = conf_entry.old_conf.begin(); iter != conf_entry.old_conf.end(); ++iter) { | ||
| *new_meta.add_old_peers() = iter->to_string(); | ||
| } | ||
| new_meta.clear_learners(); | ||
| for (auto iter = learner_conf_entry.conf.begin(); iter != learner_conf_entry.conf.end(); ++iter) { | ||
| *new_meta.add_learners() = iter->to_string(); | ||
| } |
There was a problem hiding this comment.
优化快照元数据更新逻辑。
代码中使用了 const_cast 来修改 snapshot_meta_memtable.meta()，这可能导致未定义的行为，建议寻找替代方法来处理不可变对象的修改。
此外，更新快照元数据的逻辑看起来正确，但建议添加更多的错误处理和日志记录，以便在更新过程中捕获和诊断可能的问题。
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- etc/script/build.sh (1 hunks)
- etc/script/save_load.sh (1 hunks)
Files skipped from review due to trivial changes (2)
- etc/script/build.sh
- etc/script/save_load.sh
panlei-coder
force-pushed
the
fix_conf_of_snapshot_meta
branch
from
f7f3608 to
df23447
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- save_load.sh (1 hunks)
Additional comments not posted (1)
save_load.sh (1)
7-10: 使用变量替代硬编码路径通过引入变量来存储当前工作目录、项目主目录、二进制路径和配置文件路径，脚本变得更加适应不同环境，降低了因路径变更导致的错误风险。
save_load.sh
Outdated
| cd leader && ulimit -n 99999 && rm -fr * && ../bin/pikiwidb ../pikiwidb.conf --port 7777 & | ||
| cd follower1 && ulimit -n 99999 && rm -fr * && ../bin/pikiwidb ../pikiwidb.conf --port 8888 & | ||
|
|
||
| PWD=`pwd` |
There was a problem hiding this comment.
建议使用现代的命令替代方式
建议使用 $(...) 替代过时的反引号 `，以提高可读性和支持嵌套命令。
- PWD=`pwd`
+ PWD=$(pwd)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.
| PWD=`pwd` | |
| PWD=$(pwd) |
save_load.sh
Outdated
| cd leader && ulimit -n 99999 && rm -fr * && ${BIN} ${CONF} --port 7777 & | ||
| cd follower1 && ulimit -n 99999 && rm -fr * && ${BIN} ${CONF} --port 8888 & |
There was a problem hiding this comment.
增加错误处理
当前脚本在执行过程中缺乏错误处理，这可能导致生产环境中的静默失败。建议在每个关键命令后添加错误检查，以提高脚本的健壮性。
+ set -e
+ trap "echo 'An error occurred.'; exit 1" ERRCommittable 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.
| cd leader && ulimit -n 99999 && rm -fr * && ${BIN} ${CONF} --port 7777 & | |
| cd follower1 && ulimit -n 99999 && rm -fr * && ${BIN} ${CONF} --port 8888 & | |
| set -e | |
| trap "echo 'An error occurred.'; exit 1" ERR | |
| cd leader && ulimit -n 99999 && rm -fr * && ${BIN} ${CONF} --port 7777 & | |
| cd follower1 && ulimit -n 99999 && rm -fr * && ${BIN} ${CONF} --port 8888 & |
优先合并：arana-db/braft#8 braft
1、解决在真正生成快照时，snapshot meta 保存的成员配置信息和实际的快照数据不一致问题，可能导致新加入的节点跳过成员变更日志，出现配置信息不一致的问题。#385
2、修改 set_self_replay_point 接口为 set_last_applied_index_and_term。
Summary by CodeRabbit
Summary by CodeRabbit
新特性
PRaft类中新增了一个用于根据索引获取配置的公共方法GetConfigurationByIndex，提升了配置条目的动态访问能力。braft库的版本标签至v1.1.2.1，以确保使用更稳定的库版本。save_load.sh脚本进行了重构，使用变量替代硬编码路径，提升了可维护性和适应性。样式
pikiwidb.conf文件中的raft-port-offset和use-raft设置进行了微小的空格调整，确保配置一致性。修复