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

Reset peer's log info once new connection is established #182

Merged
merged 3 commits into from
Feb 26, 2021

Conversation

greensky00
Copy link
Contributor

  • To support offline data change (including snapshot installation),
    peer's log info should be reset if the previous connection is gone.
    Otherwise, there will be a data mismatch and the leader will attempt
    to send logs based on incorrect info.

  • The reset peer info will be recovered during the first
    append_entries communication between the leader and the peer.

* To support offline data change (including snapshot installation),
peer's log info should be reset if the previous connection is gone.
Otherwise, there will be a data mismatch and the leader will attempt
to send logs based on incorrect info.

* The reset peer info will be recovered during the first
`append_entries` communication between the leader and the peer.
Copy link
Contributor

@sheepgrass sheepgrass left a comment

Choose a reason for hiding this comment

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

runtests.sh now passed. Please refer to other comments here.

Comment on lines 80 to 84
bool ok = pp->recreate_rpc(s_config, *ctx_);
if (ok) {
pp->set_free();
pp->set_manual_free();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems that every time recreate_rpc() call success, set_free() and set_manual_free() must be called. Better call them inside recreate_rpc() before return true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, thanks.

Comment on lines 896 to 900
bool ok = p.recreate_rpc(s_config, *ctx_);
if (ok) {
p.set_free();
p.set_manual_free();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

same as previous comment

@@ -209,10 +209,12 @@ void peer::recreate_rpc(ptr<srv_config>& config,
// A reconnection attempt should be treated as an activity,
// hence reset timer.
reset_active_timer();
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

better call set_free() and set_manual_free() here?

@greensky00 greensky00 merged commit ce6a6e8 into eBay:master Feb 26, 2021
greensky00 added a commit to greensky00/NuRaft that referenced this pull request Mar 14, 2021
* This happens due to recent protocol change:
  eBay#182
greensky00 added a commit that referenced this pull request Mar 14, 2021
* This happens due to recent protocol change:
  #182
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.

3 participants