Repository navigation
Conversation
Three nodes, partial netsplit: A sees B and C, B and C cannot see each other. A name is registered on B, then on C, so A holds the process on C. A custom resolver keeps the earlier registration, the one on B. When B connects to C, B can handle the discover of C before its own nodeup. C then handles the ack_sync of B while B is not yet in its nodes_map. The handler merged the remote data first: C kept the process on B and broadcast the unregistration of its own process, but only to A, because B was not in nodes_map yet. Then C built the snapshot for B, and the name was no longer in it. B never saw the conflict and never sent its process to A. A removed the process on C and lost the name, while B and C kept the process on B. With the default resolution, or any resolver that keeps the later registration, A already holds the winner and keeps it. For a node that is not in nodes_map yet, the ack_sync handler now monitors it, adds it to nodes_map and sends the local snapshot before merging its data. B gets the conflicting entry, resolves the conflict on its side and broadcasts its process to A with a new time. The broadcasts of the merge on C now reach B as well, after the snapshot. The change adds no message type and keeps the message format.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes #92.
When a node handles an
ack_syncfrom a node that is not in itsnodes_mapyet and the merge resolves a conflict in favour of the remote process, the peer gets a snapshot that no longer shows the conflict, and the resolution broadcast does not reach it. A third node that held the losing process is left without an entry for the name, and nothing repairs it. It takes a partial netsplit, a resolver that keeps the earlier registration, and the connecting node handling the peer'sdiscoverbefore its ownnodeup.Before: the unknown-node branch of the
ack_synchandler merged the remote data, then monitored the node, sent the local snapshot and added the node tonodes_map. After: it monitors the node, adds it tonodes_mapand sends the snapshot first, and merges after. The snapshot still holds the entries the resolution is about to remove, so the peer sees the conflict and resolves it on its side too, and the broadcasts of the merge reach the new node after the snapshot, through the same sender process. A peer that joins throughdiscoveralready gets its snapshot before any broadcast; theack_syncpath now has the same order.No message type is added and the message format is unchanged. The one difference on the wire is that the broadcasts of the merge have one more recipient, the new node.
Mixed clusters. What an unpatched node receives from a patched one, a snapshot followed by the merge's broadcasts, is what it receives today when the join goes through
discoverfirst. The bug stays possible while the node that takes the unknown-node branch is unpatched. There is no new failure mode for unpatched nodes.Of the four new cases in
syn_registry_SUITE,three_nodes_ack_sync_from_unknown_node_keeps_remoteis the order of the issue and fails on 3.4.2: the lookup for the name returnsundefinedwhere the kept process is expected. The other three pass before and after the change. Each half of the change has a case that fails without it:keeps_remotestill fails when the merge runs first even with the updatednodes_map, andthree_nodes_ack_sync_from_unknown_node_resolution_reaches_peerfails when the snapshot goes first but the merge still broadcasts with the oldnodes_map.syn_registry_SUITE(19 cases) passes on OTP 25.3.2.8, 27.3.4.11 and 29.0.6,syn_pg_SUITE(13 cases) too; dialyzer is clean on 27.3.4.11 and 29.0.6. The natural-order loops from the issue, 300 iterations with the resolver that keeps the earlier registration and 100 with one that keeps the later one, stay consistent with the change.