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.
This commit is contained in:
Roei Erez 2018-10-16 17:39:38 +03:00
parent 058b0e5767
commit 6c256aea30

@ -2124,13 +2124,13 @@ func (s *server) InboundPeerConnected(conn net.Conn) {
case nil: case nil:
// We already have a connection with the incoming peer. If the // We already have a connection with the incoming peer. If the
// connection we've already established should be kept, then // connection we've already established should be kept and is not of
// we'll close out this connection s.t there's only a single // the same type of the new connection (inbound), then we'll close out
// connection between us. // the new connection s.t there's only a single connection between us.
localPub := s.identityPriv.PubKey() localPub := s.identityPriv.PubKey()
if !shouldDropLocalConnection(localPub, nodePub) { if !connectedPeer.inbound && !shouldDropLocalConnection(localPub, nodePub) {
srvrLog.Warnf("Received inbound connection from "+ srvrLog.Warnf("Received inbound connection from peer %x, "+
"peer %x, but already connected, dropping conn", "but already have outbound connection, dropping conn",
nodePub.SerializeCompressed()) nodePub.SerializeCompressed())
conn.Close() conn.Close()
return return
@ -2230,14 +2230,14 @@ func (s *server) OutboundPeerConnected(connReq *connmgr.ConnReq, conn net.Conn)
s.peerConnected(conn, connReq, false) s.peerConnected(conn, connReq, false)
case nil: case nil:
// We already have a connection open with the target peer. // We already have a connection with the incoming peer. If the
// If our (this) connection should be dropped, then we'll do // connection we've already established should be kept and is not of
// so, in order to ensure we don't have any duplicate // the same type of the new connection (outbound), then we'll close out
// connections. // the new connection s.t there's only a single connection between us.
localPub := s.identityPriv.PubKey() localPub := s.identityPriv.PubKey()
if shouldDropLocalConnection(localPub, nodePub) { if connectedPeer.inbound && shouldDropLocalConnection(localPub, nodePub) {
srvrLog.Warnf("Established outbound connection to "+ srvrLog.Warnf("Established outbound connection to peer %x, "+
"peer %x, but already connected, dropping conn", "but already have inbound connection, dropping conn",
nodePub.SerializeCompressed()) nodePub.SerializeCompressed())
if connReq != nil { if connReq != nil {
s.connMgr.Remove(connReq.ID()) s.connMgr.Remove(connReq.ID())