Make raft leader resume probing after snapshot crash #2707
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
srfrog
approved these changes
Oct 30, 2018
There was a problem hiding this comment.
Reviewed 6 of 6 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved
dna2github
pushed a commit
to dna2fork/dgraph
that referenced
this pull request
Jul 19, 2019
When a raft follower is considered to be falling behind, leader sends it a snapshot. The follower then opens a streaming connection to the leader, asking it to send the snapshot. If the follower crashes while receiving and applying the snapshot, it is left in a permanent limbo, receiving no more raft updates. This was because the leader pauses its probing until it hears back from the follower. This "persists" through follower crash and restart. This PR makes the snapshot streaming bi-directional. So, the follower can send an ACK back to the leader when it has successfully applied the snapshot. If the leader gets an error instead, it would mark the snapshot as a failure, and resume probing. The end effect is that, if the follower crashes while receiving a snapshot, leader resumes probing, so when the follower comes back up, it again requests a snapshot from the leader. Fixes dgraph-io#2698 . P.S. I wish etcd's raft lib had better documentation to warn us about this, via the function. I'll create another PR to improve their godocs. * Change the order in which Snapshot is retrieved and WAL is `SaveToStorage`. So, if a follower is unable to retrieve the snapshot, it won't store it in the WAL. This allows future probing to work correctly. * Some exploration to determine a good way to send ack of writes back to the leader. * Recovers correctly from a node crash during snapshot streaming.
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.
When a raft follower is considered to be falling behind, leader sends it a snapshot. The follower then opens a streaming connection to the leader, asking it to send the snapshot. If the follower crashes while receiving and applying the snapshot, it is left in a permanent limbo, receiving no more raft updates. This was because the leader pauses its probing until it hears back from the follower. This "persists" through follower crash and restart.
This PR makes the snapshot streaming bi-directional. So, the follower can send an ACK back to the leader when it has successfully applied the snapshot. If the leader gets an error instead, it would mark the snapshot as a failure, and resume probing.
The end effect is that, if the follower crashes while receiving a snapshot, leader resumes probing, so when the follower comes back up, it again requests a snapshot from the leader.
Fixes #2698 .
P.S. I wish etcd's raft lib had better documentation to warn us about this, via the function. I'll create another PR to improve their godocs.
This change is