Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finish registering a new connection before sending routing update #9517

Merged
merged 3 commits into from
Sep 13, 2023

Conversation

saketh-are
Copy link
Collaborator

Updates to the routing table trigger a broadcast advising all peers of the latest information:

fn broadcast_distance_vector(&self, distance_vector: DistanceVector) {
let msg = Arc::new(PeerMessage::DistanceVector(distance_vector));
for conn in self.tier2.load().ready.values() {
conn.send_message(msg.clone());
}
}

Connection of a new peer is one type of event which results in a routing table update. The update should be broadcast after registering the new connection, so that the new peer is included in the broadcast.

@saketh-are
Copy link
Collaborator Author

This fix corrects an issue in which nodes don't converge correctly to the complete set of available routes. In the screenshot below, a node in a 3-node clique is aware of its direct connections to the other two nodes, but is not aware of the edge between its two peers.

image

After this change, the simple 3-node setup converges correctly.

@saketh-are saketh-are marked this pull request as ready for review September 12, 2023 20:04
@saketh-are saketh-are requested a review from a team as a code owner September 12, 2023 20:04
Copy link
Contributor

@wacban wacban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@saketh-are saketh-are added this pull request to the merge queue Sep 13, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 13, 2023
@saketh-are saketh-are added this pull request to the merge queue Sep 13, 2023
Merged via the queue into master with commit 06aebc3 Sep 13, 2023
14 checks passed
@saketh-are saketh-are deleted the fix-routing-peer-connected branch September 13, 2023 13:35
marcelo-gonzalez pushed a commit to marcelo-gonzalez/nearcore that referenced this pull request Oct 18, 2023
…ar#9517)

* register connection before updating routing

* make sure DistanceVectors are sent upon connection
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants