Merge pull request #1736 from weaveworks/remove-unknown-peer-error

remove UnknownPeerError for quicker convergence and efficiency. LGTM.

mesh/local_peer.go

@@ -157,7 +157,7 @@ func (peer *LocalPeer) handleAddConnection(conn Connection) error {
 	}
 
 	peer.router.Routes.Recalculate()
-	peer.broadcastPeerUpdate(conn.Remote())
+	peer.broadcastPeerUpdate()
 
 	return nil
 }
@@ -199,13 +199,13 @@ func (peer *LocalPeer) handleDeleteConnection(conn Connection) {
 
 // helpers
 
-func (peer *LocalPeer) broadcastPeerUpdate(peers ...*Peer) {
+func (peer *LocalPeer) broadcastPeerUpdate() {
 	// Some tests run without a router.  This should be fixed so
 	// that the relevant part of Router can be easily run in the
 	// context of a test, but that will involve significant
 	// reworking of tests.
 	if peer.router != nil {
-		peer.router.BroadcastTopologyUpdate(append(peers, peer.Peer))
+		peer.router.BroadcastTopologyUpdate([]*Peer{peer.Peer})
 	}
 }
 

mesh/peer.go

@@ -26,7 +26,12 @@ func randUint16() (r uint16) {
 type PeerUID uint64
 
 func randomPeerUID() PeerUID {
-	return PeerUID(randUint64())
+	for {
+		uid := randUint64()
+		if uid != 0 { // uid 0 is reserved for peer placeholder
+			return PeerUID(uid)
+		}
+	}
 }
 
 func ParsePeerUID(s string) (PeerUID, error) {
@@ -95,6 +100,10 @@ func NewPeer(name PeerName, nickName string, uid PeerUID, version uint64, shortI
 	})
 }
 
+func NewPeerPlaceholder(name PeerName) *Peer {
+	return NewPeerFromSummary(PeerSummary{NameByte: name.Bin()})
+}
+
 func NewPeerFrom(peer *Peer) *Peer {
 	return NewPeerFromSummary(peer.PeerSummary)
 }

mesh/peers.go

@@ -3,7 +3,6 @@ package mesh
 import (
 	"bytes"
 	"encoding/gob"
-	"fmt"
 	"io"
 	"math/rand"
 	"sync"
@@ -32,14 +31,6 @@ type ShortIDPeers struct {
 	others []*Peer
 }
 
-type UnknownPeerError struct {
-	Name PeerName
-}
-
-func (upe UnknownPeerError) Error() string {
-	return fmt.Sprint("Reference to unknown peer ", upe.Name)
-}
-
 type PeerNameSet map[PeerName]struct{}
 
 type ConnectionSummary struct {
@@ -334,9 +325,7 @@ func (peers *Peers) ApplyUpdate(update []byte) (PeerNameSet, PeerNameSet, error)
 		return nil, nil, err
 	}
 
-	// By this point, we know the update doesn't refer to any peers we
-	// have no knowledge of. We can now apply the update. Start by
-	// adding in any new peers.
+	// Add new peers
 	for name, newPeer := range newPeers {
 		peers.byName[name] = newPeer
 		peers.addByShortID(newPeer, &pending)
@@ -446,10 +435,8 @@ func (peers *Peers) decodeUpdate(update []byte) (newPeers map[PeerName]*Peer, de
 			if _, found := peers.byName[remoteName]; found {
 				continue
 			}
-			// Update refers to a peer which we have no knowledge
-			// of. Thus we can't apply the update. Abort.
-			err = UnknownPeerError{remoteName}
-			return
+			// Update refers to a peer which we have no knowledge of.
+			newPeers[remoteName] = NewPeerPlaceholder(remoteName)
 		}
 	}
 	return
@@ -484,6 +471,7 @@ func (peers *Peers) applyUpdate(decodedUpdate []*Peer, decodedConns [][]Connecti
 			}
 			peer.Version = newPeer.Version
 			peer.UID = newPeer.UID
+			peer.NickName = newPeer.NickName
 			peer.connections = makeConnsMap(peer, connSummaries, peers.byName)
 
 			if newPeer.ShortID != peer.ShortID || newPeer.HasShortID != peer.HasShortID {

mesh/router.go

@@ -172,14 +172,6 @@ func (router *Router) OnGossip(update []byte) (GossipData, error) {
 
 func (router *Router) applyTopologyUpdate(update []byte) (PeerNameSet, PeerNameSet, error) {
 	origUpdate, newUpdate, err := router.Peers.ApplyUpdate(update)
-	if _, ok := err.(UnknownPeerError); err != nil && ok {
-		// That update contained a reference to a peer which wasn't
-		// itself included in the update, and we didn't know about
-		// already. We ignore this; eventually we should receive an
-		// update containing a complete topology.
-		log.Println("Topology gossip:", err)
-		return nil, nil, nil
-	}
 	if err != nil {
 		return nil, nil, err
 	}