Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

remove UnknownPeerError for quicker convergence and efficiency #1736

Merged
merged 2 commits into from
Dec 4, 2015

Conversation

rade
Copy link
Member

@rade rade commented Nov 30, 2015

When encountering an unknown peer in received gossip, rather than ignoring the entire gossip we create a placeholder Peer. This has version 0 and uid 0, which means it will be replaced by any complete peer information subsequently received via gossip.

Not ignoring the gossip results in quicker convergence.

We also no longer need to broadcast about the remote peer when adding a connection, since we only did that in order to reduce the occurrence of UnknownPeerErrors. Not needing to do this is is more efficient, since the broadcast was strictly redundant.

@rade rade self-assigned this Nov 30, 2015
@rade rade force-pushed the remove-unknown-peer-error branch from 4cd78af to 29cb566 Compare November 30, 2015 16:03
when encountering an unknown peer in received gossip, rather than
ignoring the entire gossip we create a placeholder Peer. This has
version 0 and uid 0, which means it will be replaced by any complete
peer information subsequently received via gossip.
@rade rade force-pushed the remove-unknown-peer-error branch from 29cb566 to 7e15eab Compare November 30, 2015 16:17
@rade rade changed the title [WIP] Remove UnknownPeerError [WIP] Remove UnknownPeerError for quicker convergence and efficiency Dec 1, 2015
This is more efficient since broadcasting about other peers is
strictly redundant. Any change to the peers will be broadcast by
those peers anyway. And SendAllGossipDown takes care of informing new
peers about others on first connect, and periodic gossip take care of
missed broadcasts.

The only reason we did broadcast about the remote peer is to reduce
the liklihood of UnknownPeerError and the associated throwing away of
gossip. This was always a bit of kludge, and is no longer necessary
since UnknownPeerError is gone.

Now peers only ever broadcast information about themselves. Which,
unlike information about other peers, is always authorative and most
up-to-date. Which is nice.
@rade
Copy link
Member Author

rade commented Dec 1, 2015

There is one user-visible consequence of this change: occasionally, and usually briefly, weave status et al will show peers without a nickname.

@rade rade changed the title [WIP] Remove UnknownPeerError for quicker convergence and efficiency remove UnknownPeerError for quicker convergence and efficiency Dec 1, 2015
@rade rade removed their assignment Dec 1, 2015
@bboreham bboreham self-assigned this Dec 4, 2015
@bboreham bboreham force-pushed the remove-unknown-peer-error branch from 8c73249 to cbb2ad0 Compare December 4, 2015 10:59
bboreham added a commit that referenced this pull request Dec 4, 2015
remove UnknownPeerError for quicker convergence and efficiency. LGTM.
@bboreham bboreham merged commit ab1033c into master Dec 4, 2015
@awh awh added this to the 1.4.0 milestone Dec 4, 2015
@rade rade deleted the remove-unknown-peer-error branch December 31, 2015 15:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants