Skip to content
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

fix master_run_id bug #1778

Merged
merged 1 commit into from
Jul 20, 2023
Merged

Conversation

luky116
Copy link
Collaborator

@luky116 luky116 commented Jul 20, 2023

fix: #1777

@@ -536,7 +536,7 @@ int PikaConf::Load() {
if (slaveof_ != "") {
std::string master_run_id;
GetConfStr("master-run-id", &master_run_id);
if (master_run_id_.length() == configRunIDSize) {
if (master_run_id.length() == configRunIDSize) {
master_run_id_ = master_run_id;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code patch you provided seems to contain a small change. It looks like there is an update in the if condition comparison for the length of master_run_id. Instead of master_run_id_.length(), it has been changed to master_run_id.length().

This change seems fine and shouldn't introduce any bugs or issues, assuming master_run_id_ is a string variable declared elsewhere in the code. By using master_run_id.length(), the code ensures that the length of master_run_id matches configRunIDSize before assigning its value to master_run_id_.

Overall, without additional context or code, it's difficult to provide further suggestions or improvements. However, based on this snippet alone, the change appears to be reasonable and does not raise any immediate concerns.

@@ -180,6 +180,7 @@ void PikaServer::Start() {
if ((master_ip == "127.0.0.1" || master_ip == host_) && master_port == port_) {
LOG(FATAL) << "you will slaveof yourself as the config file, please check";
} else {
g_pika_server->set_master_run_id(g_pika_conf->master_run_id());
SetMaster(master_ip, master_port);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the code patch you provided, here is a brief review:

  1. The addition of g_pika_server->set_master_run_id(g_pika_conf->master_run_id()); sets the master run ID in the PikaServer object. Assuming this functionality is intended and implemented correctly, there doesn't appear to be any issues or bugs related to this specific line.

  2. It's important to note that analyzing the code patch alone may not provide a complete understanding of potential bugs or improvements. Contextual information about the surrounding code and the overall system architecture would be helpful for a more thorough review.

  3. It's also worth mentioning that performing a code review requires more than just looking at individual lines. It involves considering the design, structure, error handling, performance optimizations, security concerns, and adherence to best practices. Without access to the complete codebase and further details, it's challenging to provide a comprehensive review.

To ensure a thorough code review, consider sharing the relevant portions of the code surrounding this patch, as well as any specific concerns or areas where you would like improvement suggestions.

@AlexStocks AlexStocks merged commit 4158969 into OpenAtomFoundation:unstable Jul 20, 2023
9 checks passed
bigdaronlee163 pushed a commit to bigdaronlee163/pika that referenced this pull request Jun 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

进程启动时从配置文件加载master-run-id失败,导致从节点需要重新同步历史数据。
3 participants