[FIXED] Clustering: channel first/last sequence may fall to zero #840
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
derekcollison
approved these changes
May 18, 2019
This could happen if the leader took a snapshot while messages were not yet expired, then a node is started without state and tries to restore from this snapshot. If the messages have expired by then, no message would be stored. If that node later did a snapshot itself, it would persist in it the first/last being zero. If no message are published and this node becomes leader, it would start storing messages at the wrong sequence and would also send the bad snapshot to other nodes. Resolves #833 Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
|
@derekcollison I have pushed forced because I did a rebase from master to take the raft shutdown changes. But I kept a separate commit to ease the review. I can squash prior to merge. Commit to review would be this one |
- bail out after a number of failed attempts to restore msgs - create snapshot on success if restore msgs indicates that first in channel has moved. Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
| // Also don't use startGoRoutine() here since it needs the | ||
| // server lock, which we already have. | ||
| s.mu.Lock() | ||
| s.raft.Snapshot().Error() |
There was a problem hiding this comment.
Can't do in place correct?
There was a problem hiding this comment.
meaning without go routine? if so no, since as I tried to explain, as part of the startup process we call NewRaft() (hashicorp raft constructor), which returns a Raft object that we assign to s.raft.Raft at the end of createRaftNode(). NewRaft() is the one invoking Restore() on startup (when there are local snapshots), so I can't call s.raft.Snapshot() until after the creation of the raft node has completed.
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.
This could happen if the leader took a snapshot while messages were
not yet expired, then a node is started without state and tries
to restore from this snapshot. If the messages have expired by then,
no message would be stored. If that node later did a snapshot itself,
it would persist in it the first/last being zero. If no message are
published and this node becomes leader, it would start storing
messages at the wrong sequence and would also send the bad snapshot
to other nodes.
Resolves #833
Signed-off-by: Ivan Kozlovic ivan@synadia.com