=cluster,handshake #724 immediately initialize membership #778
Conversation
This file contains hidden or 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
ktoso
commented
Sep 29, 2020
| // always update the snapshot before emitting events | ||
| context.system.cluster.updateMembershipSnapshot(state.membership) | ||
| self.clusterEvents.publish(.membershipChange(change)) | ||
| } |
Member
Author
There was a problem hiding this comment.
the fix™
The sending to self to avoid having another spot where we have to write "updateMembershipSnapshot and publish event" was causing a window of opportunity for another message to be handled first -- and that message would then hit us when we had NO members at all in the membership, not even "us".
This would then cause the nonsensical handshake rejection because "there is no local member"
Yet there always is a local member! It's impossible to not know the local member, it is us.
ktoso
commented
Sep 29, 2020
| /// If, and only if, the current node is a leader it performs a set of tasks, such as moving nodes to `.up` etc. | ||
| func collectLeaderActions() -> [LeaderAction] { | ||
| guard self.membership.isLeader(self.localNode) else { | ||
| guard self.membership.isLeader(self.selfNode) else { |
ktoso
commented
Sep 29, 2020
| reason: "Node cannot be part of cluster, no member available.", | ||
| whenHandshakeReplySent: nil | ||
| ) | ||
| } |
Member
Author
There was a problem hiding this comment.
this rejection reason is nonsensical and was triggered in some of the tests sporadically; This is now made impossible for good.
"loop" through shell as this may cause a race condition between a node extenging a handshake to this node before the "loop through self adding the myself Member" has a chance to run; This manifested in tests by rejecting handshakes by "no local member" which is nonsense, there always is a local known member after all.
yim-lee
approved these changes
Sep 29, 2020
Co-authored-by: Yim Lee <yim_lee@apple.com>
Member
Author
|
Weird failure #779 :/ |
Member
Author
|
@swift-server-bot test this please |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
immediately initialize membership, without "loop" through shell as this may cause a race condition between a node
extenging a handshake to this node before the "loop through self adding
the myself Member" has a chance to run; This manifested in tests by
rejecting handshakes by "no local member" which is nonsense, there
always is a local known member after all.
resolves #724, an actual bug 😱