From 6c256aea3081c035183f7b331a60156029f15be3 Mon Sep 17 00:00:00 2001 From: Roei Erez Date: Tue, 16 Oct 2018 17:39:38 +0300 Subject: [PATCH] server: fix the algo to prefer connections when multiple exist This commit modified the condition to whether drop an existing connection to a peer when a new connection to this peer is established. The previous algorithm used public keys comparison for this decision which determines that between every two nodes only one of them will ever drop the connection in such cases. The problematic case is when a node disconnects and reconnects in a short interval which is the case of mobile devices. In such case it takes as much as the "timeout" configured value for the remote node to detect the "disconnection" (and try to reconnect if this connection is persistent). In the case this node is also the one that has the "smaller" public key the reconnect attempts of the other node will be rejected causing it impossible to fast reconnect. The solution is to only drop the connection if if we already have a connected peer that is of the opposite direction from the this new connection. By doing so the "initiator" will be enabled to replace the connection and recconnect immediately. --- server.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/server.go b/server.go index a55f2f60..4db5b6ad 100644 --- a/server.go +++ b/server.go @@ -2124,13 +2124,13 @@ func (s *server) InboundPeerConnected(conn net.Conn) { case nil: // We already have a connection with the incoming peer. If the - // connection we've already established should be kept, then - // we'll close out this connection s.t there's only a single - // connection between us. + // connection we've already established should be kept and is not of + // the same type of the new connection (inbound), then we'll close out + // the new connection s.t there's only a single connection between us. localPub := s.identityPriv.PubKey() - if !shouldDropLocalConnection(localPub, nodePub) { - srvrLog.Warnf("Received inbound connection from "+ - "peer %x, but already connected, dropping conn", + if !connectedPeer.inbound && !shouldDropLocalConnection(localPub, nodePub) { + srvrLog.Warnf("Received inbound connection from peer %x, "+ + "but already have outbound connection, dropping conn", nodePub.SerializeCompressed()) conn.Close() return @@ -2230,14 +2230,14 @@ func (s *server) OutboundPeerConnected(connReq *connmgr.ConnReq, conn net.Conn) s.peerConnected(conn, connReq, false) case nil: - // We already have a connection open with the target peer. - // If our (this) connection should be dropped, then we'll do - // so, in order to ensure we don't have any duplicate - // connections. + // We already have a connection with the incoming peer. If the + // connection we've already established should be kept and is not of + // the same type of the new connection (outbound), then we'll close out + // the new connection s.t there's only a single connection between us. localPub := s.identityPriv.PubKey() - if shouldDropLocalConnection(localPub, nodePub) { - srvrLog.Warnf("Established outbound connection to "+ - "peer %x, but already connected, dropping conn", + if connectedPeer.inbound && shouldDropLocalConnection(localPub, nodePub) { + srvrLog.Warnf("Established outbound connection to peer %x, "+ + "but already have inbound connection, dropping conn", nodePub.SerializeCompressed()) if connReq != nil { s.connMgr.Remove(connReq.ID())