diff --git a/connmgr/connmanager.go b/connmgr/connmanager.go index 58259ee6..8e64ae2f 100644 --- a/connmgr/connmanager.go +++ b/connmgr/connmanager.go @@ -127,8 +127,9 @@ type Config struct { // behavior, while a pointer to zero disables inbound connections. MaxInbound *uint32 - // TargetOutbound is the number of outbound network connections to - // maintain. Defaults to 8. + // TargetOutbound is the number of automatic outbound network connections + // to maintain. Connections made through Connect are additional. Defaults + // to 8. TargetOutbound uint32 // RetryDuration is the duration to wait before retrying connection @@ -616,7 +617,7 @@ func (cm *ConnManager) Start() { } } - for i := atomic.LoadUint64(&cm.connReqCount); i < uint64(cm.cfg.TargetOutbound); i++ { + for i := uint32(0); i < cm.cfg.TargetOutbound; i++ { go cm.NewConnReq() } } diff --git a/connmgr/connmanager_test.go b/connmgr/connmanager_test.go index 49c5f843..2316ccf4 100644 --- a/connmgr/connmanager_test.go +++ b/connmgr/connmanager_test.go @@ -371,6 +371,114 @@ func TestTargetOutbound(t *testing.T) { cmgr.Stop() } +// TestTargetOutboundComposition verifies that explicit permanent connections +// are additional to the automatic outbound target regardless of whether they +// receive connection request IDs before or after the manager starts. +func TestTargetOutboundComposition(t *testing.T) { + const ( + targetOutbound = uint32(3) + permanentPeers = uint64(2) + ) + + tests := []struct { + name string + permanentBeforeStart bool + }{ + {name: "permanent before start", permanentBeforeStart: true}, + {name: "permanent after start"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + connected := make(chan *ConnReq, + int(targetOutbound)+int(permanentPeers)) + cmgr, err := New(&Config{ + TargetOutbound: targetOutbound, + Dial: mockDialer, + GetNewAddress: func() (net.Addr, error) { + return &net.TCPAddr{ + IP: net.ParseIP("127.0.0.1"), + Port: 18555, + }, nil + }, + OnConnection: func(c *ConnReq, _ net.Conn) { + connected <- c + }, + }) + if err != nil { + t.Fatalf("New error: %v", err) + } + + connectPermanent := func() { + for i := uint64(0); i < permanentPeers; i++ { + go cmgr.Connect(&ConnReq{ + Addr: &net.TCPAddr{ + IP: net.ParseIP("127.0.0.1"), + Port: 18556 + int(i), + }, + Permanent: true, + }) + } + } + + if test.permanentBeforeStart { + connectPermanent() + + deadline := time.After(time.Second) + for atomic.LoadUint64(&cmgr.connReqCount) < + permanentPeers { + + select { + case <-deadline: + t.Fatal("permanent requests did not receive IDs") + case <-time.After(time.Millisecond): + } + } + } + + cmgr.Start() + if !test.permanentBeforeStart { + connectPermanent() + } + + var ( + automaticCount int + permanentCount int + ) + for i := 0; i < int(targetOutbound)+int(permanentPeers); i++ { + select { + case connReq := <-connected: + if connReq.Permanent { + permanentCount++ + } else { + automaticCount++ + } + + case <-time.After(time.Second): + t.Fatal("timed out waiting for outbound connections") + } + } + + if automaticCount != int(targetOutbound) { + t.Fatalf("unexpected automatic count: got %d, want %d", + automaticCount, targetOutbound) + } + if permanentCount != int(permanentPeers) { + t.Fatalf("unexpected permanent count: got %d, want %d", + permanentCount, permanentPeers) + } + + select { + case connReq := <-connected: + t.Fatalf("unexpected extra connection: %v", connReq) + case <-time.After(10 * time.Millisecond): + } + + cmgr.Stop() + }) + } +} + // TestRetryPermanent tests that permanent connection requests are retried. // // We make a permanent connection request using Connect, disconnect it using diff --git a/server.go b/server.go index 678e3f82..f41f0e52 100644 --- a/server.go +++ b/server.go @@ -74,6 +74,24 @@ var ( // zeroHash is the zero value hash (all zeros). It is defined as a convenience. var zeroHash chainhash.Hash +// targetOutboundPeers returns the automatic outbound target for the configured +// peer mode after permanent peers reserve their portion of the total budget. +func targetOutboundPeers( + maxPeers, permanentPeers int, automaticOutbound bool, +) int { + + if !automaticOutbound || permanentPeers >= maxPeers { + return 0 + } + + available := maxPeers - permanentPeers + if available < defaultTargetOutbound { + return available + } + + return defaultTargetOutbound +} + // reservedOutboundPeers returns the outbound connection reservation for the // configured peer mode, capped at the total peer limit. func reservedOutboundPeers( @@ -3225,17 +3243,20 @@ func newServer(listenAddrs, agentBlacklist, agentWhitelist []string, } // Create a connection manager. - targetOutbound := defaultTargetOutbound - if cfg.MaxPeers < targetOutbound { - targetOutbound = cfg.MaxPeers - } permanentPeerCount := len(cfg.ConnectPeers) if permanentPeerCount == 0 { permanentPeerCount = len(cfg.AddPeers) } + automaticOutbound := newAddressFunc != nil + targetOutbound := targetOutboundPeers( + cfg.MaxPeers, permanentPeerCount, automaticOutbound, + ) + if targetOutbound == 0 { + newAddressFunc = nil + } reservedOutbound := reservedOutboundPeers( cfg.MaxPeers, targetOutbound, permanentPeerCount, - newAddressFunc != nil, + automaticOutbound, ) maxInbound := maxInboundPeers(cfg.MaxPeers, reservedOutbound) if maxInbound == 0 && len(listeners) > 0 { diff --git a/server_test.go b/server_test.go index 5639b595..3e7fb2c5 100644 --- a/server_test.go +++ b/server_test.go @@ -117,61 +117,66 @@ func TestInboundPeerReservation(t *testing.T) { tests := []struct { name string maxPeers int - targetOutbound int permanentPeers int automatic bool + wantTarget int wantReserved int wantInbound uint32 }{ { - name: "connect only", maxPeers: 8, targetOutbound: 8, + name: "connect only", maxPeers: 8, permanentPeers: 1, wantReserved: 1, wantInbound: 7, }, { name: "connect only capped", maxPeers: 8, - targetOutbound: 8, permanentPeers: 10, - wantReserved: 8, wantInbound: 0, + permanentPeers: 10, + wantReserved: 8, wantInbound: 0, }, { - name: "simnet without peers", maxPeers: 8, - targetOutbound: 8, wantReserved: 0, wantInbound: 8, + name: "simnet without peers", maxPeers: 8, wantReserved: 0, + wantInbound: 8, }, { - name: "simnet with peers", maxPeers: 8, - targetOutbound: 8, permanentPeers: 3, + name: "simnet with peers", maxPeers: 8, permanentPeers: 3, wantReserved: 3, wantInbound: 5, }, { name: "automatic without add peers", maxPeers: 125, - targetOutbound: 8, automatic: true, + automatic: true, wantTarget: 8, wantReserved: 8, wantInbound: 117, }, { name: "add peers below target", maxPeers: 125, - targetOutbound: 8, permanentPeers: 3, automatic: true, + permanentPeers: 3, automatic: true, wantTarget: 8, wantReserved: 11, wantInbound: 114, }, { name: "add peers above target", maxPeers: 10, - targetOutbound: 8, permanentPeers: 9, automatic: true, + permanentPeers: 9, automatic: true, wantTarget: 1, wantReserved: 10, wantInbound: 0, }, { name: "add peers at max peers", maxPeers: 10, - targetOutbound: 8, permanentPeers: 10, automatic: true, + permanentPeers: 10, automatic: true, wantReserved: 10, wantInbound: 0, }, { name: "add peers above max peers", maxPeers: 10, - targetOutbound: 8, permanentPeers: 12, automatic: true, + permanentPeers: 12, automatic: true, wantReserved: 10, wantInbound: 0, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { + targetOutbound := targetOutboundPeers( + test.maxPeers, test.permanentPeers, + test.automatic, + ) + require.Equal(t, test.wantTarget, targetOutbound) + reserved := reservedOutboundPeers( - test.maxPeers, test.targetOutbound, + test.maxPeers, targetOutbound, test.permanentPeers, test.automatic, ) require.Equal(t, test.wantReserved, reserved)