Merge pull request #451 from bboreham/448_reduce_conns

Avoid many pointless connection attempts

Fixes #448.

router/connection.go

@@ -13,9 +13,10 @@ import (
 type Connection interface {
 	Local() *Peer
 	Remote() *Peer
-	BreakTie(Connection) ConnectionTieBreak
 	RemoteTCPAddr() string
+	Outbound() bool
 	Established() bool
+	BreakTie(Connection) ConnectionTieBreak
 	Shutdown(error)
 	Log(args ...interface{})
 }
@@ -32,6 +33,7 @@ type RemoteConnection struct {
 	local         *Peer
 	remote        *Peer
 	remoteTCPAddr string
+	outbound      bool
 	established   bool
 }
 
@@ -66,36 +68,23 @@ type ConnectionInteraction struct {
 	payload interface{}
 }
 
-func NewRemoteConnection(from, to *Peer, tcpAddr string, established bool) *RemoteConnection {
+func NewRemoteConnection(from, to *Peer, tcpAddr string, outbound bool, established bool) *RemoteConnection {
 	return &RemoteConnection{
 		local:         from,
 		remote:        to,
 		remoteTCPAddr: tcpAddr,
-		established:   established}
+		outbound:      outbound,
+		established:   established,
+	}
 }
 
-func (conn *RemoteConnection) Local() *Peer {
-	return conn.local
-}
-
-func (conn *RemoteConnection) Remote() *Peer {
-	return conn.remote
-}
-
-func (conn *RemoteConnection) BreakTie(Connection) ConnectionTieBreak {
-	return TieBreakTied
-}
-
-func (conn *RemoteConnection) RemoteTCPAddr() string {
-	return conn.remoteTCPAddr
-}
-
-func (conn *RemoteConnection) Established() bool {
-	return conn.established
-}
-
-func (conn *RemoteConnection) Shutdown(error) {
-}
+func (conn *RemoteConnection) Local() *Peer                           { return conn.local }
+func (conn *RemoteConnection) Remote() *Peer                          { return conn.remote }
+func (conn *RemoteConnection) RemoteTCPAddr() string                  { return conn.remoteTCPAddr }
+func (conn *RemoteConnection) Outbound() bool                         { return conn.outbound }
+func (conn *RemoteConnection) Established() bool                      { return conn.established }
+func (conn *RemoteConnection) BreakTie(Connection) ConnectionTieBreak { return TieBreakTied }
+func (conn *RemoteConnection) Shutdown(error)                         {}
 
 func (conn *RemoteConnection) Log(args ...interface{}) {
 	log.Println(append(append([]interface{}{}, fmt.Sprintf("->[%s]:", conn.remote.Name)), args...)...)

router/connection_maker.go

@@ -144,8 +144,10 @@ func (cm *ConnectionMaker) checkStateAndAttemptConnections() time.Duration {
 				return
 			}
 			address := conn.RemoteTCPAddr()
-			// try both portnumber of connection and standard port
-			addTarget(address)
+			// try both portnumber of connection and standard port.  Don't use remote side of inbound connection.
+			if conn.Outbound() {
+				addTarget(address)
+			}
 			if host, _, err := net.SplitHostPort(address); err == nil {
 				addTarget(NormalisePeerAddr(host))
 			}

router/gossip_test.go

@@ -41,7 +41,7 @@ func (router *Router) AddTestChannelConnection(r *Router) {
 	r.Peers.FetchWithDefault(fromPeer)    // Has side-effect of incrementing refcount
 	router.Peers.FetchWithDefault(toPeer) //
 
-	conn := &mockChannelConnection{RemoteConnection{router.Ourself.Peer, toPeer, "", false}, r}
+	conn := &mockChannelConnection{RemoteConnection{router.Ourself.Peer, toPeer, "", false, true}, r}
 	router.Ourself.handleAddConnection(conn)
 	router.Ourself.handleConnectionEstablished(conn)
 }

router/local_peer.go

@@ -111,7 +111,7 @@ func (peer *LocalPeer) CreateConnection(peerAddr string, acceptNewPeer bool) err
 	if err != nil {
 		return err
 	}
-	connRemote := NewRemoteConnection(peer.Peer, nil, tcpConn.RemoteAddr().String(), false)
+	connRemote := NewRemoteConnection(peer.Peer, nil, tcpConn.RemoteAddr().String(), true, false)
 	connLocal := NewLocalConnection(connRemote, tcpConn, udpAddr, peer.Router)
 	connLocal.Start(acceptNewPeer)
 	return nil

router/mocks_test.go

@@ -27,7 +27,7 @@ func (peers *Peers) AddTestRemoteConnection(p1, p2 *Peer) {
 	toName := p2.Name
 	toPeer := NewPeer(toName, "", p2.UID, 0)
 	toPeer = peers.FetchWithDefault(toPeer)
-	peers.ourself.addConnection(&RemoteConnection{fromPeer, toPeer, "", false})
+	peers.ourself.addConnection(&RemoteConnection{fromPeer, toPeer, "", false, false})
 }
 
 func (peers *Peers) DeleteTestConnection(p *Peer) {
@@ -44,7 +44,7 @@ func (peers *Peers) DeleteTestConnection(p *Peer) {
 // from what is created by the real code.
 func newMockConnection(from, to *Peer) Connection {
 	type mockConnection struct{ RemoteConnection }
-	return &mockConnection{RemoteConnection{from, to, "", false}}
+	return &mockConnection{RemoteConnection{from, to, "", false, false}}
 }
 
 func checkEqualConns(t *testing.T, ourName PeerName, got, wanted map[PeerName]Connection) {

router/peers.go

@@ -199,7 +199,7 @@ func (peers *Peers) decodeUpdate(update []byte) (newPeers map[PeerName]*Peer, de
 	}
 
 	for _, connsBuf := range decodedConns {
-		decErr := connsIterator(connsBuf, func(remoteNameByte []byte, _ string, _ bool) {
+		decErr := connsIterator(connsBuf, func(remoteNameByte []byte, _ string, _, _ bool) {
 			remoteName := PeerNameFromBin(remoteNameByte)
 			if _, found := newPeers[remoteName]; found {
 				return
@@ -267,6 +267,7 @@ func (peer *Peer) encode(enc *gob.Encoder) {
 		checkFatal(connsEnc.Encode(conn.Remote().NameByte))
 		checkFatal(connsEnc.Encode(conn.RemoteTCPAddr()))
 		// DANGER holding rlock on peer, going to take rlock on conn
+		checkFatal(connsEnc.Encode(conn.Outbound()))
 		checkFatal(connsEnc.Encode(conn.Established()))
 	}
 	checkFatal(enc.Encode(connsBuf.Bytes()))
@@ -291,7 +292,7 @@ func decodePeerNoConns(dec *gob.Decoder) (nameByte []byte, nickName string, uid
 	return
 }
 
-func connsIterator(input []byte, fun func([]byte, string, bool)) error {
+func connsIterator(input []byte, fun func([]byte, string, bool, bool)) error {
 	buf := new(bytes.Buffer)
 	buf.Write(input)
 	dec := gob.NewDecoder(buf)
@@ -304,20 +305,24 @@ func connsIterator(input []byte, fun func([]byte, string, bool)) error {
 		if err := dec.Decode(&foundAt); err != nil {
 			return err
 		}
+		var outbound bool
+		if err := dec.Decode(&outbound); err != nil {
+			return err
+		}
 		var established bool
 		if err := dec.Decode(&established); err != nil {
 			return err
 		}
-		fun(nameByte, foundAt, established)
+		fun(nameByte, foundAt, outbound, established)
 	}
 }
 
 func readConnsMap(peer *Peer, buf []byte, table map[PeerName]*Peer) map[PeerName]Connection {
 	conns := make(map[PeerName]Connection)
-	if err := connsIterator(buf, func(nameByte []byte, remoteTCPAddr string, established bool) {
+	if err := connsIterator(buf, func(nameByte []byte, remoteTCPAddr string, outbound bool, established bool) {
 		name := PeerNameFromBin(nameByte)
 		remotePeer := table[name]
-		conn := NewRemoteConnection(peer, remotePeer, remoteTCPAddr, established)
+		conn := NewRemoteConnection(peer, remotePeer, remoteTCPAddr, outbound, established)
 		conns[name] = conn
 	}); err != io.EOF {
 		// this should never happen since we've already successfully

router/protocol.go

@@ -2,7 +2,7 @@ package router
 
 const (
 	Protocol        = "weave"
-	ProtocolVersion = 13
+	ProtocolVersion = 14
 )
 
 type ProtocolTag byte

router/router.go

@@ -192,7 +192,7 @@ func (router *Router) acceptTCP(tcpConn *net.TCPConn) {
 	// start.
 	remoteAddrStr := tcpConn.RemoteAddr().String()
 	log.Printf("->[%s] connection accepted\n", remoteAddrStr)
-	connRemote := NewRemoteConnection(router.Ourself.Peer, nil, remoteAddrStr, false)
+	connRemote := NewRemoteConnection(router.Ourself.Peer, nil, remoteAddrStr, false, false)
 	connLocal := NewLocalConnection(connRemote, tcpConn, nil, router)
 	connLocal.Start(true)
 }