This repository has been archived by the owner on Jun 25, 2021. It is now read-only.
feat/node: send Ack on consensused message #1715
Merged
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
maqi
changed the title
maqi
force-pushed
the
issue_1666_send_ack_on_msg_consensus
branch
from
cf4b2a4
to
dc8feb7
Compare
maqi
force-pushed
the
issue_1666_send_ack_on_msg_consensus
branch
from
dc8feb7
to
c299256
Compare
maqi
force-pushed
the
issue_1666_send_ack_on_msg_consensus
branch
3 times, most recently
from
4d608da
to
8f4bda5
Compare
maqi
changed the title
|
Looks good, a minor comment |
maqi
force-pushed
the
issue_1666_send_ack_on_msg_consensus
branch
from
8f4bda5
to
f000e7a
Compare
fizyk20
previously approved these changes
Jul 24, 2019
src/states/common/approved.rs
Outdated
| @@ -59,6 +59,7 @@ pub trait Approved: Relocated { | |||
| fn handle_neighbour_merge_event(&mut self) -> Result<(), RoutingError>; | |||
|
|
|||
| /// Handles an accumulated `SectionInfo` event. | |||
| /// If triggered during initial, an ack message shall not be sent out. | |||
There was a problem hiding this comment.
Could you rephrase the comment? I'm not sure what "triggered during initial" means.
maqi
force-pushed
the
issue_1666_send_ack_on_msg_consensus
branch
2 times, most recently
from
925c5f6
to
3e8a934
Compare
fizyk20
previously approved these changes
Jul 24, 2019
jeanphilippeD
previously approved these changes
Jul 25, 2019
maqi
force-pushed
the
issue_1666_send_ack_on_msg_consensus
branch
from
627738d
to
4ca73ef
Compare
jeanphilippeD
previously approved these changes
Jul 25, 2019
As the Ack will be a multi-cast message sent among section members, this could cause a 10-15% performance drop on the churn tests. As those tests invloves lost of membership change which will result in many section_info updates. Also, this commit contains a test requiring a setup of section splitting, which cost some time to complete normally.
maqi
force-pushed
the
issue_1666_send_ack_on_msg_consensus
branch
from
4ca73ef
to
dec5b3b
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Closes #1666