From 7e7a2535c6e5cca1f11f0b535d66b2931d6eca7a Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 17 Jun 2022 08:26:03 -0400 Subject: [PATCH] p2p: peer store and dialing changes (#8737) (#8784) (cherry picked from commit 9e5b13725d8b1061354eb99acbd2c3c1cefe18c9) Co-authored-by: Sam Kleinman --- CHANGELOG_PENDING.md | 4 + config/config.go | 8 + config/toml.go | 4 + internal/p2p/metrics.gen.go | 50 +++- internal/p2p/metrics.go | 20 +- internal/p2p/p2ptest/network.go | 1 + internal/p2p/peermanager.go | 348 ++++++++++++++++++++--- internal/p2p/peermanager_scoring_test.go | 172 ++++++++++- internal/p2p/peermanager_test.go | 74 +++-- internal/p2p/router.go | 31 +- node/node.go | 2 +- node/seed.go | 2 +- node/setup.go | 13 +- proto/tendermint/p2p/types.pb.go | 121 +++++--- proto/tendermint/p2p/types.proto | 1 + 15 files changed, 708 insertions(+), 143 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index c3e191e7e..3deec9b7c 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -37,6 +37,10 @@ Special thanks to external contributors on this release: - [p2p] \#7035 Remove legacy P2P routing implementation and associated configuration options. (@tychoish) - [p2p] \#7265 Peer manager reduces peer score for each failed dial attempts for peers that have not successfully dialed. (@tychoish) - [p2p] [\#7594](https://github.com/tendermint/tendermint/pull/7594) always advertise self, to enable mutual address discovery. (@altergui) + - [p2p] \#8737 Introduce "inactive" peer label to avoid re-dialing incompatible peers. (@tychoish) + - [p2p] \#8737 Increase frequency of dialing attempts to reduce latency for peer acquisition. (@tychoish) + - [p2p] \#8737 Improvements to peer scoring and sorting to gossip a greater variety of peers during PEX. (@tychoish) + - [p2p] \#8737 Track incoming and outgoing peers separately to ensure more peer slots open for incoming connections. (@tychoish) - Go API diff --git a/config/config.go b/config/config.go index 43b3fc10f..400142309 100644 --- a/config/config.go +++ b/config/config.go @@ -627,6 +627,10 @@ type P2PConfig struct { //nolint: maligned // outbound). MaxConnections uint16 `mapstructure:"max-connections"` + // MaxOutgoingConnections defines the maximum number of connected peers (inbound and + // outbound). + MaxOutgoingConnections uint16 `mapstructure:"max-outgoing-connections"` + // MaxIncomingConnectionAttempts rate limits the number of incoming connection // attempts per IP address. MaxIncomingConnectionAttempts uint `mapstructure:"max-incoming-connection-attempts"` @@ -674,6 +678,7 @@ func DefaultP2PConfig() *P2PConfig { ExternalAddress: "", UPNP: false, MaxConnections: 64, + MaxOutgoingConnections: 32, MaxIncomingConnectionAttempts: 100, FlushThrottleTimeout: 100 * time.Millisecond, // The MTU (Maximum Transmission Unit) for Ethernet is 1500 bytes. @@ -708,6 +713,9 @@ func (cfg *P2PConfig) ValidateBasic() error { if cfg.RecvRate < 0 { return errors.New("recv-rate can't be negative") } + if cfg.MaxOutgoingConnections > cfg.MaxConnections { + return errors.New("max-outgoing-connections cannot be larger than max-connections") + } return nil } diff --git a/config/toml.go b/config/toml.go index 4db4f4e65..a9698096a 100644 --- a/config/toml.go +++ b/config/toml.go @@ -309,6 +309,10 @@ upnp = {{ .P2P.UPNP }} # Maximum number of connections (inbound and outbound). max-connections = {{ .P2P.MaxConnections }} +# Maximum number of connections reserved for outgoing +# connections. Must be less than max-connections +max-outgoing-connections = {{ .P2P.MaxOutgoingConnections }} + # Rate limits the number of incoming connection attempts per IP address. max-incoming-connection-attempts = {{ .P2P.MaxIncomingConnectionAttempts }} diff --git a/internal/p2p/metrics.gen.go b/internal/p2p/metrics.gen.go index cbfba29d9..cb215f2b6 100644 --- a/internal/p2p/metrics.gen.go +++ b/internal/p2p/metrics.gen.go @@ -14,11 +14,23 @@ func PrometheusMetrics(namespace string, labelsAndValues ...string) *Metrics { labels = append(labels, labelsAndValues[i]) } return &Metrics{ - Peers: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + PeersConnected: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ Namespace: namespace, Subsystem: MetricsSubsystem, - Name: "peers", - Help: "Number of peers.", + Name: "peers_connected", + Help: "Number of peers connected.", + }, labels).With(labelsAndValues...), + PeersStored: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Namespace: namespace, + Subsystem: MetricsSubsystem, + Name: "peers_stored", + Help: "Nomber of peers in the peer store database.", + }, labels).With(labelsAndValues...), + PeersInactivated: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Namespace: namespace, + Subsystem: MetricsSubsystem, + Name: "peers_inactivated", + Help: "Number of inactive peers stored.", }, labels).With(labelsAndValues...), PeerReceiveBytesTotal: prometheus.NewCounterFrom(stdprometheus.CounterOpts{ Namespace: namespace, @@ -38,6 +50,30 @@ func PrometheusMetrics(namespace string, labelsAndValues ...string) *Metrics { Name: "peer_pending_send_bytes", Help: "Number of bytes pending being sent to a given peer.", }, append(labels, "peer_id")).With(labelsAndValues...), + PeersConnectedSuccess: prometheus.NewCounterFrom(stdprometheus.CounterOpts{ + Namespace: namespace, + Subsystem: MetricsSubsystem, + Name: "peers_connected_success", + Help: "Number of successful connection attempts", + }, labels).With(labelsAndValues...), + PeersConnectedFailure: prometheus.NewCounterFrom(stdprometheus.CounterOpts{ + Namespace: namespace, + Subsystem: MetricsSubsystem, + Name: "peers_connected_failure", + Help: "Number of failed connection attempts", + }, labels).With(labelsAndValues...), + PeersConnectedIncoming: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Namespace: namespace, + Subsystem: MetricsSubsystem, + Name: "peers_connected_incoming", + Help: "Number of peers connected as a result of dialing the peer.", + }, labels).With(labelsAndValues...), + PeersConnectedOutgoing: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Namespace: namespace, + Subsystem: MetricsSubsystem, + Name: "peers_connected_outgoing", + Help: "Number of peers connected as a result of the peer dialing this node.", + }, labels).With(labelsAndValues...), RouterPeerQueueRecv: prometheus.NewHistogramFrom(stdprometheus.HistogramOpts{ Namespace: namespace, Subsystem: MetricsSubsystem, @@ -73,10 +109,16 @@ func PrometheusMetrics(namespace string, labelsAndValues ...string) *Metrics { func NopMetrics() *Metrics { return &Metrics{ - Peers: discard.NewGauge(), + PeersConnected: discard.NewGauge(), + PeersStored: discard.NewGauge(), + PeersInactivated: discard.NewGauge(), PeerReceiveBytesTotal: discard.NewCounter(), PeerSendBytesTotal: discard.NewCounter(), PeerPendingSendBytes: discard.NewGauge(), + PeersConnectedSuccess: discard.NewCounter(), + PeersConnectedFailure: discard.NewCounter(), + PeersConnectedIncoming: discard.NewGauge(), + PeersConnectedOutgoing: discard.NewGauge(), RouterPeerQueueRecv: discard.NewHistogram(), RouterPeerQueueSend: discard.NewHistogram(), RouterChannelQueueSend: discard.NewHistogram(), diff --git a/internal/p2p/metrics.go b/internal/p2p/metrics.go index b45f128e5..a88aaa3f2 100644 --- a/internal/p2p/metrics.go +++ b/internal/p2p/metrics.go @@ -26,8 +26,12 @@ var ( // Metrics contains metrics exposed by this package. type Metrics struct { - // Number of peers. - Peers metrics.Gauge + // Number of peers connected. + PeersConnected metrics.Gauge + // Nomber of peers in the peer store database. + PeersStored metrics.Gauge + // Number of inactive peers stored. + PeersInactivated metrics.Gauge // Number of bytes per channel received from a given peer. PeerReceiveBytesTotal metrics.Counter `metrics_labels:"peer_id, chID, message_type"` // Number of bytes per channel sent to a given peer. @@ -35,6 +39,18 @@ type Metrics struct { // Number of bytes pending being sent to a given peer. PeerPendingSendBytes metrics.Gauge `metrics_labels:"peer_id"` + // Number of successful connection attempts + PeersConnectedSuccess metrics.Counter + // Number of failed connection attempts + PeersConnectedFailure metrics.Counter + + // Number of peers connected as a result of dialing the + // peer. + PeersConnectedIncoming metrics.Gauge + // Number of peers connected as a result of the peer dialing + // this node. + PeersConnectedOutgoing metrics.Gauge + // RouterPeerQueueRecv defines the time taken to read off of a peer's queue // before sending on the connection. //metrics:The time taken to read off of a peer's queue before sending on the connection. diff --git a/internal/p2p/p2ptest/network.go b/internal/p2p/p2ptest/network.go index 85df029d8..5c1b0a218 100644 --- a/internal/p2p/p2ptest/network.go +++ b/internal/p2p/p2ptest/network.go @@ -257,6 +257,7 @@ func (n *Network) MakeNode(ctx context.Context, t *testing.T, opts NodeOptions) RetryTimeJitter: time.Millisecond, MaxPeers: opts.MaxPeers, MaxConnected: opts.MaxConnected, + Metrics: p2p.NopMetrics(), }) require.NoError(t, err) diff --git a/internal/p2p/peermanager.go b/internal/p2p/peermanager.go index 7391de4ea..ef4011093 100644 --- a/internal/p2p/peermanager.go +++ b/internal/p2p/peermanager.go @@ -38,11 +38,18 @@ const ( PeerStatusBad PeerStatus = "bad" // peer observed as bad ) -// PeerScore is a numeric score assigned to a peer (higher is better). -type PeerScore uint8 +type peerConnectionDirection int const ( - PeerScorePersistent PeerScore = math.MaxUint8 // persistent peers + peerConnectionIncoming peerConnectionDirection = iota + 1 + peerConnectionOutgoing +) + +// PeerScore is a numeric score assigned to a peer (higher is better). +type PeerScore int16 + +const ( + PeerScorePersistent PeerScore = math.MaxInt16 // persistent peers MaxPeerScoreNotPersistent PeerScore = PeerScorePersistent - 1 ) @@ -101,6 +108,13 @@ type PeerManagerOptions struct { // outbound). 0 means no limit. MaxConnected uint16 + // MaxOutgoingConnections specifies how many outgoing + // connections a node will maintain. It must be lower than MaxConnected. If it is + // 0, then all connections can be outgoing. Once this limit is + // reached, the node will not dial peers, allowing the + // remaining peer connections to be used by incoming connections. + MaxOutgoingConnections uint16 + // MaxConnectedUpgrade is the maximum number of additional connections to // use for probing any better-scored peers to upgrade to when all connection // slots are full. 0 disables peer upgrading. @@ -145,6 +159,9 @@ type PeerManagerOptions struct { // persistentPeers provides fast PersistentPeers lookups. It is built // by optimize(). persistentPeers map[types.NodeID]bool + + // Peer Metrics + Metrics *Metrics } // Validate validates the options. @@ -193,6 +210,10 @@ func (o *PeerManagerOptions) Validate() error { } } + if o.MaxOutgoingConnections > 0 && o.MaxConnected < o.MaxOutgoingConnections { + return errors.New("cannot set MaxOutgoingConnections to a value larger than MaxConnected") + } + return nil } @@ -261,19 +282,20 @@ func (o *PeerManagerOptions) optimize() { type PeerManager struct { selfID types.NodeID options PeerManagerOptions + metrics *Metrics rand *rand.Rand dialWaker *tmsync.Waker // wakes up DialNext() on relevant peer changes evictWaker *tmsync.Waker // wakes up EvictNext() on relevant peer changes mtx sync.Mutex store *peerStore - subscriptions map[*PeerUpdates]*PeerUpdates // keyed by struct identity (address) - dialing map[types.NodeID]bool // peers being dialed (DialNext → Dialed/DialFail) - upgrading map[types.NodeID]types.NodeID // peers claimed for upgrade (DialNext → Dialed/DialFail) - connected map[types.NodeID]bool // connected peers (Dialed/Accepted → Disconnected) - ready map[types.NodeID]bool // ready peers (Ready → Disconnected) - evict map[types.NodeID]bool // peers scheduled for eviction (Connected → EvictNext) - evicting map[types.NodeID]bool // peers being evicted (EvictNext → Disconnected) + subscriptions map[*PeerUpdates]*PeerUpdates // keyed by struct identity (address) + dialing map[types.NodeID]bool // peers being dialed (DialNext → Dialed/DialFail) + upgrading map[types.NodeID]types.NodeID // peers claimed for upgrade (DialNext → Dialed/DialFail) + connected map[types.NodeID]peerConnectionDirection // connected peers (Dialed/Accepted → Disconnected) + ready map[types.NodeID]bool // ready peers (Ready → Disconnected) + evict map[types.NodeID]bool // peers scheduled for eviction (Connected → EvictNext) + evicting map[types.NodeID]bool // peers being evicted (EvictNext → Disconnected) } // NewPeerManager creates a new peer manager. @@ -298,16 +320,22 @@ func NewPeerManager(selfID types.NodeID, peerDB dbm.DB, options PeerManagerOptio rand: rand.New(rand.NewSource(time.Now().UnixNano())), // nolint:gosec dialWaker: tmsync.NewWaker(), evictWaker: tmsync.NewWaker(), + metrics: NopMetrics(), store: store, dialing: map[types.NodeID]bool{}, upgrading: map[types.NodeID]types.NodeID{}, - connected: map[types.NodeID]bool{}, + connected: map[types.NodeID]peerConnectionDirection{}, ready: map[types.NodeID]bool{}, evict: map[types.NodeID]bool{}, evicting: map[types.NodeID]bool{}, subscriptions: map[*PeerUpdates]*PeerUpdates{}, } + + if options.Metrics != nil { + peerManager.metrics = options.Metrics + } + if err = peerManager.configurePeers(); err != nil { return nil, err } @@ -368,20 +396,45 @@ func (m *PeerManager) prunePeers() error { ranked := m.store.Ranked() for i := len(ranked) - 1; i >= 0; i-- { peerID := ranked[i].ID + switch { case m.store.Size() <= int(m.options.MaxPeers): return nil case m.dialing[peerID]: - case m.connected[peerID]: + case m.isConnected(peerID): default: if err := m.store.Delete(peerID); err != nil { return err } + m.metrics.PeersStored.Add(-1) } } return nil } +func (m *PeerManager) isConnected(peerID types.NodeID) bool { + _, ok := m.connected[peerID] + return ok +} + +type connectionStats struct { + incoming uint16 + outgoing uint16 +} + +func (m *PeerManager) getConnectedInfo() connectionStats { + out := connectionStats{} + for _, direction := range m.connected { + switch direction { + case peerConnectionIncoming: + out.incoming++ + case peerConnectionOutgoing: + out.outgoing++ + } + } + return out +} + // Add adds a peer to the manager, given as an address. If the peer already // exists, the address is added to it if it isn't already present. This will push // low scoring peers out of the address book if it exceeds the maximum size. @@ -405,12 +458,17 @@ func (m *PeerManager) Add(address NodeAddress) (bool, error) { if ok { return false, nil } + if peer.Inactive { + return false, nil + } // else add the new address peer.AddressInfo[address] = &peerAddressInfo{Address: address} if err := m.store.Set(peer); err != nil { return false, err } + + m.metrics.PeersStored.Add(1) if err := m.prunePeers(); err != nil { return true, err } @@ -464,13 +522,17 @@ func (m *PeerManager) TryDialNext() (NodeAddress, error) { // We allow dialing MaxConnected+MaxConnectedUpgrade peers. Including // MaxConnectedUpgrade allows us to probe additional peers that have a // higher score than any other peers, and if successful evict it. - if m.options.MaxConnected > 0 && len(m.connected)+len(m.dialing) >= - int(m.options.MaxConnected)+int(m.options.MaxConnectedUpgrade) { + if m.options.MaxConnected > 0 && len(m.connected)+len(m.dialing) >= int(m.options.MaxConnected)+int(m.options.MaxConnectedUpgrade) { + return NodeAddress{}, nil + } + + cinfo := m.getConnectedInfo() + if m.options.MaxOutgoingConnections > 0 && cinfo.outgoing >= m.options.MaxOutgoingConnections { return NodeAddress{}, nil } for _, peer := range m.store.Ranked() { - if m.dialing[peer.ID] || m.connected[peer.ID] { + if m.dialing[peer.ID] || m.isConnected(peer.ID) { continue } @@ -503,11 +565,10 @@ func (m *PeerManager) TryDialNext() (NodeAddress, error) { // DialFailed reports a failed dial attempt. This will make the peer available // for dialing again when appropriate (possibly after a retry timeout). -// -// FIXME: This should probably delete or mark bad addresses/peers after some time. func (m *PeerManager) DialFailed(ctx context.Context, address NodeAddress) error { m.mtx.Lock() defer m.mtx.Unlock() + m.metrics.PeersConnectedFailure.Add(1) delete(m.dialing, address.NodeID) for from, to := range m.upgrading { @@ -527,6 +588,7 @@ func (m *PeerManager) DialFailed(ctx context.Context, address NodeAddress) error addressInfo.LastDialFailure = time.Now().UTC() addressInfo.DialFailures++ + if err := m.store.Set(peer); err != nil { return err } @@ -560,6 +622,8 @@ func (m *PeerManager) Dialed(address NodeAddress) error { m.mtx.Lock() defer m.mtx.Unlock() + m.metrics.PeersConnectedSuccess.Add(1) + delete(m.dialing, address.NodeID) var upgradeFromPeer types.NodeID @@ -574,12 +638,11 @@ func (m *PeerManager) Dialed(address NodeAddress) error { if address.NodeID == m.selfID { return fmt.Errorf("rejecting connection to self (%v)", address.NodeID) } - if m.connected[address.NodeID] { + if m.isConnected(address.NodeID) { return fmt.Errorf("peer %v is already connected", address.NodeID) } if m.options.MaxConnected > 0 && len(m.connected) >= int(m.options.MaxConnected) { - if upgradeFromPeer == "" || len(m.connected) >= - int(m.options.MaxConnected)+int(m.options.MaxConnectedUpgrade) { + if upgradeFromPeer == "" || len(m.connected) >= int(m.options.MaxConnected)+int(m.options.MaxConnectedUpgrade) { return fmt.Errorf("already connected to maximum number of peers") } } @@ -589,6 +652,11 @@ func (m *PeerManager) Dialed(address NodeAddress) error { return fmt.Errorf("peer %q was removed while dialing", address.NodeID) } now := time.Now().UTC() + if peer.Inactive { + m.metrics.PeersInactivated.Add(-1) + } + peer.Inactive = false + peer.LastConnected = now if addressInfo, ok := peer.AddressInfo[address]; ok { addressInfo.DialFailures = 0 @@ -611,7 +679,9 @@ func (m *PeerManager) Dialed(address NodeAddress) error { } m.evict[upgradeFromPeer] = true } - m.connected[peer.ID] = true + + m.metrics.PeersConnectedOutgoing.Add(1) + m.connected[peer.ID] = peerConnectionOutgoing m.evictWaker.Wake() return nil @@ -641,11 +711,10 @@ func (m *PeerManager) Accepted(peerID types.NodeID) error { if peerID == m.selfID { return fmt.Errorf("rejecting connection from self (%v)", peerID) } - if m.connected[peerID] { + if m.isConnected(peerID) { return fmt.Errorf("peer %q is already connected", peerID) } - if m.options.MaxConnected > 0 && - len(m.connected) >= int(m.options.MaxConnected)+int(m.options.MaxConnectedUpgrade) { + if m.options.MaxConnected > 0 && len(m.connected) >= int(m.options.MaxConnected)+int(m.options.MaxConnectedUpgrade) { return fmt.Errorf("already connected to maximum number of peers") } @@ -670,12 +739,17 @@ func (m *PeerManager) Accepted(peerID types.NodeID) error { } } + if peer.Inactive { + m.metrics.PeersInactivated.Add(-1) + } + peer.Inactive = false peer.LastConnected = time.Now().UTC() if err := m.store.Set(peer); err != nil { return err } - m.connected[peerID] = true + m.metrics.PeersConnectedIncoming.Add(1) + m.connected[peerID] = peerConnectionIncoming if upgradeFromPeer != "" { m.evict[upgradeFromPeer] = true } @@ -694,7 +768,7 @@ func (m *PeerManager) Ready(ctx context.Context, peerID types.NodeID, channels C m.mtx.Lock() defer m.mtx.Unlock() - if m.connected[peerID] { + if m.isConnected(peerID) { m.ready[peerID] = true m.broadcast(ctx, PeerUpdate{ NodeID: peerID, @@ -730,7 +804,7 @@ func (m *PeerManager) TryEvictNext() (types.NodeID, error) { // random one. for peerID := range m.evict { delete(m.evict, peerID) - if m.connected[peerID] && !m.evicting[peerID] { + if m.isConnected(peerID) && !m.evicting[peerID] { m.evicting[peerID] = true return peerID, nil } @@ -747,7 +821,7 @@ func (m *PeerManager) TryEvictNext() (types.NodeID, error) { ranked := m.store.Ranked() for i := len(ranked) - 1; i >= 0; i-- { peer := ranked[i] - if m.connected[peer.ID] && !m.evicting[peer.ID] { + if m.isConnected(peer.ID) && !m.evicting[peer.ID] { m.evicting[peer.ID] = true return peer.ID, nil } @@ -762,6 +836,13 @@ func (m *PeerManager) Disconnected(ctx context.Context, peerID types.NodeID) { m.mtx.Lock() defer m.mtx.Unlock() + switch m.connected[peerID] { + case peerConnectionIncoming: + m.metrics.PeersConnectedIncoming.Add(-1) + case peerConnectionOutgoing: + m.metrics.PeersConnectedOutgoing.Add(-1) + } + ready := m.ready[peerID] delete(m.connected, peerID) @@ -792,17 +873,34 @@ func (m *PeerManager) Errored(peerID types.NodeID, err error) { m.mtx.Lock() defer m.mtx.Unlock() - if m.connected[peerID] { + if m.isConnected(peerID) { m.evict[peerID] = true } m.evictWaker.Wake() } +// Inactivate marks a peer as inactive which means we won't attempt to +// dial this peer again. A peer can be reactivated by successfully +// dialing and connecting to the node. +func (m *PeerManager) Inactivate(peerID types.NodeID) error { + m.mtx.Lock() + defer m.mtx.Unlock() + + peer, ok := m.store.peers[peerID] + if !ok { + return nil + } + + peer.Inactive = true + m.metrics.PeersInactivated.Add(1) + return m.store.Set(*peer) +} + // Advertise returns a list of peer addresses to advertise to a peer. // -// FIXME: This is fairly naïve and only returns the addresses of the -// highest-ranked peers. +// It sorts all peers in the peer store, and assembles a list of peers +// that is most likely to include the highest priority of peers. func (m *PeerManager) Advertise(peerID types.NodeID, limit uint16) []NodeAddress { m.mtx.Lock() defer m.mtx.Unlock() @@ -815,19 +913,92 @@ func (m *PeerManager) Advertise(peerID types.NodeID, limit uint16) []NodeAddress addresses = append(addresses, m.options.SelfAddress) } - for _, peer := range m.store.Ranked() { + var numAddresses int + var totalScore int + ranked := m.store.Ranked() + seenAddresses := map[NodeAddress]struct{}{} + scores := map[types.NodeID]int{} + + // get the total number of possible addresses + for _, peer := range ranked { if peer.ID == peerID { continue } + score := int(peer.Score()) - for nodeAddr, addressInfo := range peer.AddressInfo { - if len(addresses) >= int(limit) { - return addresses + totalScore += score + scores[peer.ID] = score + for addr := range peer.AddressInfo { + if _, ok := m.options.PrivatePeers[addr.NodeID]; !ok { + numAddresses++ + } + } + } + + var attempts uint16 + var addedLastIteration bool + + // if the number of addresses is less than the number of peers + // to advertise, adjust the limit downwards + if numAddresses < int(limit) { + limit = uint16(numAddresses) + } + + // collect addresses until we have the number requested + // (limit), or we've added all known addresses, or we've tried + // at least 256 times and the last time we iterated over + // remaining addresses we added no new candidates. + for len(addresses) < int(limit) && (attempts < (limit*2) || !addedLastIteration) { + attempts++ + addedLastIteration = false + + for idx, peer := range ranked { + if peer.ID == peerID { + continue } - // only add non-private NodeIDs - if _, ok := m.options.PrivatePeers[nodeAddr.NodeID]; !ok { - addresses = append(addresses, addressInfo.Address) + if len(addresses) >= int(limit) { + break + } + + for nodeAddr, addressInfo := range peer.AddressInfo { + if len(addresses) >= int(limit) { + break + } + + // only look at each address once, by + // tracking a set of addresses seen + if _, ok := seenAddresses[addressInfo.Address]; ok { + continue + } + + // only add non-private NodeIDs + if _, ok := m.options.PrivatePeers[nodeAddr.NodeID]; !ok { + // add the peer if the total number of ranked addresses is + // will fit within the limit, or otherwise adding + // addresses based on a coin flip. + + // the coinflip is based on the score, commonly, but + // 10% of the time we'll randomly insert a "loosing" + // peer. + + // nolint:gosec // G404: Use of weak random number generator + if numAddresses <= int(limit) || rand.Intn(totalScore+1) <= scores[peer.ID]+1 || rand.Intn((idx+1)*10) <= idx+1 { + addresses = append(addresses, addressInfo.Address) + addedLastIteration = true + seenAddresses[addressInfo.Address] = struct{}{} + } + } else { + seenAddresses[addressInfo.Address] = struct{}{} + // if the number of addresses + // is the same as the limit, + // we should remove private + // addresses from the limit so + // we can still return early. + if numAddresses == int(limit) { + limit-- + } + } } } } @@ -901,8 +1072,14 @@ func (m *PeerManager) processPeerEvent(ctx context.Context, pu PeerUpdate) { switch pu.Status { case PeerStatusBad: + if m.store.peers[pu.NodeID].MutableScore == math.MinInt16 { + return + } m.store.peers[pu.NodeID].MutableScore-- case PeerStatusGood: + if m.store.peers[pu.NodeID].MutableScore == math.MaxInt16 { + return + } m.store.peers[pu.NodeID].MutableScore++ } } @@ -993,9 +1170,11 @@ func (m *PeerManager) findUpgradeCandidate(id types.NodeID, score PeerScore) typ for i := len(ranked) - 1; i >= 0; i-- { candidate := ranked[i] switch { + case candidate.ID == id: + continue case candidate.Score() >= score: return "" // no further peers can be scored lower, due to sorting - case !m.connected[candidate.ID]: + case !m.isConnected(candidate.ID): case m.evict[candidate.ID]: case m.evicting[candidate.ID]: case m.upgrading[candidate.ID] != "": @@ -1175,9 +1354,48 @@ func (s *peerStore) Ranked() []*peerInfo { s.ranked = append(s.ranked, peer) } sort.Slice(s.ranked, func(i, j int) bool { - // FIXME: If necessary, consider precomputing scores before sorting, - // to reduce the number of Score() calls. return s.ranked[i].Score() > s.ranked[j].Score() + // TODO: reevaluate more wholistic sorting, perhaps as follows: + + // // sort inactive peers after active peers + // if s.ranked[i].Inactive && !s.ranked[j].Inactive { + // return false + // } else if !s.ranked[i].Inactive && s.ranked[j].Inactive { + // return true + // } + + // iLastDialed, iLastDialSuccess := s.ranked[i].LastDialed() + // jLastDialed, jLastDialSuccess := s.ranked[j].LastDialed() + + // // sort peers who our most recent dialing attempt was + // // successful ahead of peers with recent dialing + // // failures + // switch { + // case iLastDialSuccess && jLastDialSuccess: + // // if both peers were (are?) successfully + // // connected, convey their score, but give the + // // one we dialed successfully most recently a bonus + + // iScore := s.ranked[i].Score() + // jScore := s.ranked[j].Score() + // if jLastDialed.Before(iLastDialed) { + // jScore++ + // } else { + // iScore++ + // } + + // return iScore > jScore + // case iLastDialSuccess: + // return true + // case jLastDialSuccess: + // return false + // default: + // // if both peers were not successful in their + // // most recent dialing attempt, fall back to + // // peer score. + + // return s.ranked[i].Score() > s.ranked[j].Score() + // } }) return s.ranked } @@ -1195,11 +1413,11 @@ type peerInfo struct { // These fields are ephemeral, i.e. not persisted to the database. Persistent bool - Seed bool Height int64 FixedScore PeerScore // mainly for tests MutableScore int64 // updated by router + Inactive bool } // peerInfoFromProto converts a Protobuf PeerInfo message to a peerInfo, @@ -1208,6 +1426,7 @@ func peerInfoFromProto(msg *p2pproto.PeerInfo) (*peerInfo, error) { p := &peerInfo{ ID: types.NodeID(msg.ID), AddressInfo: map[NodeAddress]*peerAddressInfo{}, + Inactive: msg.Inactive, } if msg.LastConnected != nil { p.LastConnected = *msg.LastConnected @@ -1231,6 +1450,7 @@ func (p *peerInfo) ToProto() *p2pproto.PeerInfo { msg := &p2pproto.PeerInfo{ ID: string(p.ID), LastConnected: &p.LastConnected, + Inactive: p.Inactive, } for _, addressInfo := range p.AddressInfo { msg.AddressInfo = append(msg.AddressInfo, addressInfo.ToProto()) @@ -1254,6 +1474,46 @@ func (p *peerInfo) Copy() peerInfo { return c } +// LastDialed returns when the peer was last dialed, and if that dial +// attempt was successful. If the peer was never dialed the time stamp +// is zero time. +func (p *peerInfo) LastDialed() (time.Time, bool) { + var ( + last time.Time + success bool + ) + last = last.Add(-1) // so it's after the epoch + + for _, addr := range p.AddressInfo { + if addr.LastDialFailure.Equal(addr.LastDialSuccess) { + if addr.LastDialFailure.IsZero() { + continue + } + if last.After(addr.LastDialSuccess) { + continue + } + success = true + last = addr.LastDialSuccess + } + if addr.LastDialFailure.After(last) { + success = false + last = addr.LastDialFailure + } + if addr.LastDialSuccess.After(last) || last.Equal(addr.LastDialSuccess) { + success = true + last = addr.LastDialSuccess + } + } + + // if we never modified last, then we should return it to the + // zero value + if last.Add(1).IsZero() { + return time.Time{}, success + } + + return last, success +} + // Score calculates a score for the peer. Higher-scored peers will be // preferred over lower scores. func (p *peerInfo) Score() PeerScore { @@ -1275,10 +1535,6 @@ func (p *peerInfo) Score() PeerScore { score -= int64(addr.DialFailures) } - if score <= 0 { - return 0 - } - return PeerScore(score) } diff --git a/internal/p2p/peermanager_scoring_test.go b/internal/p2p/peermanager_scoring_test.go index a45df0b72..b454da151 100644 --- a/internal/p2p/peermanager_scoring_test.go +++ b/internal/p2p/peermanager_scoring_test.go @@ -34,7 +34,7 @@ func TestPeerScoring(t *testing.T) { t.Run("Synchronous", func(t *testing.T) { // update the manager and make sure it's correct - require.EqualValues(t, 0, peerManager.Scores()[id]) + require.Zero(t, peerManager.Scores()[id]) // add a bunch of good status updates and watch things increase. for i := 1; i < 10; i++ { @@ -97,3 +97,173 @@ func TestPeerScoring(t *testing.T) { } }) } + +func makeMockPeerStore(t *testing.T, peers ...peerInfo) *peerStore { + t.Helper() + s, err := newPeerStore(dbm.NewMemDB()) + if err != nil { + t.Fatal(err) + } + for idx := range peers { + if err := s.Set(peers[idx]); err != nil { + t.Fatal(err) + } + } + return s +} + +func TestPeerRanking(t *testing.T) { + t.Run("InactiveSecond", func(t *testing.T) { + t.Skip("inactive status is not currently factored into peer rank.") + + store := makeMockPeerStore(t, + peerInfo{ID: "second", Inactive: true}, + peerInfo{ID: "first", Inactive: false}, + ) + + ranked := store.Ranked() + if len(ranked) != 2 { + t.Fatal("missing peer in ranked output") + } + if ranked[0].ID != "first" { + t.Error("inactive peer is first") + } + if ranked[1].ID != "second" { + t.Error("active peer is second") + } + }) + t.Run("ScoreOrder", func(t *testing.T) { + for _, test := range []struct { + Name string + First int64 + Second int64 + }{ + { + Name: "Mirror", + First: 100, + Second: -100, + }, + { + Name: "VeryLow", + First: 0, + Second: -100, + }, + { + Name: "High", + First: 300, + Second: 256, + }, + } { + t.Run(test.Name, func(t *testing.T) { + store := makeMockPeerStore(t, + peerInfo{ + ID: "second", + MutableScore: test.Second, + }, + peerInfo{ + ID: "first", + MutableScore: test.First, + }) + + ranked := store.Ranked() + if len(ranked) != 2 { + t.Fatal("missing peer in ranked output") + } + if ranked[0].ID != "first" { + t.Error("higher peer is first") + } + if ranked[1].ID != "second" { + t.Error("higher peer is second") + } + }) + } + }) +} + +func TestLastDialed(t *testing.T) { + t.Run("Zero", func(t *testing.T) { + p := &peerInfo{} + ts, ok := p.LastDialed() + if !ts.IsZero() { + t.Error("timestamp should be zero:", ts) + } + if ok { + t.Error("peer reported success, despite none") + } + }) + t.Run("NeverDialed", func(t *testing.T) { + p := &peerInfo{ + AddressInfo: map[NodeAddress]*peerAddressInfo{ + {NodeID: "kip"}: {}, + {NodeID: "merlin"}: {}, + }, + } + ts, ok := p.LastDialed() + if !ts.IsZero() { + t.Error("timestamp should be zero:", ts) + } + if ok { + t.Error("peer reported success, despite none") + } + }) + t.Run("Ordered", func(t *testing.T) { + base := time.Now() + for _, test := range []struct { + Name string + SuccessTime time.Time + FailTime time.Time + ExpectedSuccess bool + }{ + { + Name: "Zero", + }, + { + Name: "Success", + SuccessTime: base.Add(time.Hour), + FailTime: base, + ExpectedSuccess: true, + }, + { + Name: "Equal", + SuccessTime: base, + FailTime: base, + ExpectedSuccess: true, + }, + { + Name: "Failure", + SuccessTime: base, + FailTime: base.Add(time.Hour), + ExpectedSuccess: false, + }, + } { + t.Run(test.Name, func(t *testing.T) { + p := &peerInfo{ + AddressInfo: map[NodeAddress]*peerAddressInfo{ + {NodeID: "kip"}: {LastDialSuccess: test.SuccessTime}, + {NodeID: "merlin"}: {LastDialFailure: test.FailTime}, + }, + } + ts, ok := p.LastDialed() + if test.ExpectedSuccess && !ts.Equal(test.SuccessTime) { + if !ts.Equal(test.FailTime) { + t.Fatal("got unexpected timestamp:", ts) + } + + t.Error("last dialed time reported incorrect value:", ts) + } + if !test.ExpectedSuccess && !ts.Equal(test.FailTime) { + if !ts.Equal(test.SuccessTime) { + t.Fatal("got unexpected timestamp:", ts) + } + + t.Error("last dialed time reported incorrect value:", ts) + } + if test.ExpectedSuccess != ok { + t.Error("test reported incorrect outcome for last dialed type") + } + }) + } + + }) + +} diff --git a/internal/p2p/peermanager_test.go b/internal/p2p/peermanager_test.go index 47e8462a4..5e9c3a8a4 100644 --- a/internal/p2p/peermanager_test.go +++ b/internal/p2p/peermanager_test.go @@ -524,11 +524,11 @@ func TestPeerManager_TryDialNext_MaxConnectedUpgrade(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ PeerScores: map[types.NodeID]p2p.PeerScore{ - a.NodeID: 0, - b.NodeID: 1, - c.NodeID: 2, - d.NodeID: 3, - e.NodeID: 0, + a.NodeID: p2p.PeerScore(0), + b.NodeID: p2p.PeerScore(1), + c.NodeID: p2p.PeerScore(2), + d.NodeID: p2p.PeerScore(3), + e.NodeID: p2p.PeerScore(0), }, PersistentPeers: []types.NodeID{c.NodeID, d.NodeID}, MaxConnected: 2, @@ -581,10 +581,8 @@ func TestPeerManager_TryDialNext_MaxConnectedUpgrade(t *testing.T) { // Now, if we disconnect a, we should be allowed to dial d because we have a // free upgrade slot. + require.Error(t, peerManager.Dialed(d)) peerManager.Disconnected(ctx, a.NodeID) - dial, err = peerManager.TryDialNext() - require.NoError(t, err) - require.Equal(t, d, dial) require.NoError(t, peerManager.Dialed(d)) // However, if we disconnect b (such that only c and d are connected), we @@ -605,7 +603,7 @@ func TestPeerManager_TryDialNext_UpgradeReservesPeer(t *testing.T) { c := p2p.NodeAddress{Protocol: "memory", NodeID: types.NodeID(strings.Repeat("c", 40))} peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ - PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: 1, c.NodeID: 1}, + PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: p2p.PeerScore(1), c.NodeID: 1}, MaxConnected: 1, MaxConnectedUpgrade: 2, }) @@ -771,7 +769,10 @@ func TestPeerManager_DialFailed_UnreservePeer(t *testing.T) { c := p2p.NodeAddress{Protocol: "memory", NodeID: types.NodeID(strings.Repeat("c", 40))} peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ - PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: 1, c.NodeID: 1}, + PeerScores: map[types.NodeID]p2p.PeerScore{ + b.NodeID: p2p.PeerScore(1), + c.NodeID: p2p.PeerScore(2), + }, MaxConnected: 1, MaxConnectedUpgrade: 2, }) @@ -887,7 +888,7 @@ func TestPeerManager_Dialed_MaxConnectedUpgrade(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ MaxConnected: 2, MaxConnectedUpgrade: 1, - PeerScores: map[types.NodeID]p2p.PeerScore{c.NodeID: 1, d.NodeID: 1}, + PeerScores: map[types.NodeID]p2p.PeerScore{c.NodeID: p2p.PeerScore(1), d.NodeID: 1}, }) require.NoError(t, err) @@ -937,7 +938,7 @@ func TestPeerManager_Dialed_Upgrade(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ MaxConnected: 1, MaxConnectedUpgrade: 2, - PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: 1, c.NodeID: 1}, + PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: p2p.PeerScore(1), c.NodeID: 1}, }) require.NoError(t, err) @@ -984,10 +985,10 @@ func TestPeerManager_Dialed_UpgradeEvenLower(t *testing.T) { MaxConnected: 2, MaxConnectedUpgrade: 1, PeerScores: map[types.NodeID]p2p.PeerScore{ - a.NodeID: 3, - b.NodeID: 2, - c.NodeID: 10, - d.NodeID: 1, + a.NodeID: p2p.PeerScore(3), + b.NodeID: p2p.PeerScore(2), + c.NodeID: p2p.PeerScore(10), + d.NodeID: p2p.PeerScore(1), }, }) require.NoError(t, err) @@ -1040,9 +1041,9 @@ func TestPeerManager_Dialed_UpgradeNoEvict(t *testing.T) { MaxConnected: 2, MaxConnectedUpgrade: 1, PeerScores: map[types.NodeID]p2p.PeerScore{ - a.NodeID: 1, - b.NodeID: 2, - c.NodeID: 3, + a.NodeID: p2p.PeerScore(1), + b.NodeID: p2p.PeerScore(2), + c.NodeID: p2p.PeerScore(3), }, }) require.NoError(t, err) @@ -1161,8 +1162,8 @@ func TestPeerManager_Accepted_MaxConnectedUpgrade(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ PeerScores: map[types.NodeID]p2p.PeerScore{ - c.NodeID: 1, - d.NodeID: 2, + c.NodeID: p2p.PeerScore(1), + d.NodeID: p2p.PeerScore(2), }, MaxConnected: 1, MaxConnectedUpgrade: 1, @@ -1209,8 +1210,8 @@ func TestPeerManager_Accepted_Upgrade(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ PeerScores: map[types.NodeID]p2p.PeerScore{ - b.NodeID: 1, - c.NodeID: 1, + b.NodeID: p2p.PeerScore(1), + c.NodeID: p2p.PeerScore(1), }, MaxConnected: 1, MaxConnectedUpgrade: 2, @@ -1252,8 +1253,8 @@ func TestPeerManager_Accepted_UpgradeDialing(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ PeerScores: map[types.NodeID]p2p.PeerScore{ - b.NodeID: 1, - c.NodeID: 1, + b.NodeID: p2p.PeerScore(1), + c.NodeID: p2p.PeerScore(1), }, MaxConnected: 1, MaxConnectedUpgrade: 2, @@ -1428,7 +1429,7 @@ func TestPeerManager_EvictNext_WakeOnUpgradeDialed(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ MaxConnected: 1, MaxConnectedUpgrade: 1, - PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: 1}, + PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: p2p.PeerScore(1)}, }) require.NoError(t, err) @@ -1469,7 +1470,9 @@ func TestPeerManager_EvictNext_WakeOnUpgradeAccepted(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ MaxConnected: 1, MaxConnectedUpgrade: 1, - PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: 1}, + PeerScores: map[types.NodeID]p2p.PeerScore{ + b.NodeID: p2p.PeerScore(1), + }, }) require.NoError(t, err) @@ -1833,6 +1836,7 @@ func TestPeerManager_Advertise(t *testing.T) { require.NoError(t, err) require.True(t, added) + require.Len(t, peerManager.Advertise(dID, 100), 6) // d should get all addresses. require.ElementsMatch(t, []p2p.NodeAddress{ aTCP, aMem, bTCP, bMem, cTCP, cMem, @@ -1846,10 +1850,18 @@ func TestPeerManager_Advertise(t *testing.T) { // Asking for 0 addresses should return, well, 0. require.Empty(t, peerManager.Advertise(aID, 0)) - // Asking for 2 addresses should get the highest-rated ones, i.e. a. - require.ElementsMatch(t, []p2p.NodeAddress{ - aTCP, aMem, - }, peerManager.Advertise(dID, 2)) + // Asking for 2 addresses should get two addresses + // the content of the list when there are two + addrs := peerManager.Advertise(dID, 2) + require.Len(t, addrs, 2) + for _, addr := range addrs { + if dID == addr.NodeID { + t.Fatal("never advertise self") + } + if cID == addr.NodeID { + t.Fatal("should not have returned the lowest ranked peer") + } + } } func TestPeerManager_Advertise_Self(t *testing.T) { diff --git a/internal/p2p/router.go b/internal/p2p/router.go index 511fa0fb9..d7236d472 100644 --- a/internal/p2p/router.go +++ b/internal/p2p/router.go @@ -443,9 +443,12 @@ func (r *Router) filterPeersID(ctx context.Context, id types.NodeID) error { func (r *Router) dialSleep(ctx context.Context) { if r.options.DialSleep == nil { + // the connTracker (on the other side) only rate + // limits peers for dialing more than once every 10ms, + // so these numbers are safe. const ( - maxDialerInterval = 3000 - minDialerInterval = 250 + maxDialerInterval = 500 // ms + minDialerInterval = 100 // ms ) // nolint:gosec // G404: Use of weak random number generator @@ -611,7 +614,7 @@ func (r *Router) connectPeer(ctx context.Context, address NodeAddress) { case errors.Is(err, context.Canceled): return case err != nil: - r.logger.Error("failed to dial peer", "peer", address, "err", err) + r.logger.Debug("failed to dial peer", "peer", address, "err", err) if err = r.peerManager.DialFailed(ctx, address); err != nil { r.logger.Error("failed to report dial failure", "peer", address, "err", err) } @@ -633,8 +636,7 @@ func (r *Router) connectPeer(ctx context.Context, address NodeAddress) { } if err := r.runWithPeerMutex(func() error { return r.peerManager.Dialed(address) }); err != nil { - r.logger.Error("failed to dial peer", - "op", "outgoing/dialing", "peer", address.NodeID, "err", err) + r.logger.Error("failed to dial peer", "op", "outgoing/dialing", "peer", address.NodeID, "err", err) conn.Close() return } @@ -692,12 +694,13 @@ func (r *Router) dialPeer(ctx context.Context, address NodeAddress) (Connection, // Internet can't and needs a different public address. conn, err := r.transport.Dial(dialCtx, endpoint) if err != nil { - r.logger.Error("failed to dial endpoint", "peer", address.NodeID, "endpoint", endpoint, "err", err) + r.logger.Debug("failed to dial endpoint", "peer", address.NodeID, "endpoint", endpoint, "err", err) } else { r.logger.Debug("dialed peer", "peer", address.NodeID, "endpoint", endpoint) return conn, nil } } + return nil, errors.New("all endpoints failed") } @@ -724,14 +727,6 @@ func (r *Router) handshakePeer( return peerInfo, fmt.Errorf("invalid handshake NodeInfo: %w", err) } - if peerInfo.Network != nodeInfo.Network { - if err := r.peerManager.store.Delete(peerInfo.NodeID); err != nil { - return peerInfo, fmt.Errorf("problem removing peer from store from incorrect network [%s]: %w", peerInfo.Network, err) - } - - return peerInfo, fmt.Errorf("connected to peer from wrong network, %q, removed from peer store", peerInfo.Network) - } - if types.NodeIDFromPubKey(peerKey) != peerInfo.NodeID { return peerInfo, fmt.Errorf("peer's public key did not match its node ID %q (expected %q)", peerInfo.NodeID, types.NodeIDFromPubKey(peerKey)) @@ -742,6 +737,10 @@ func (r *Router) handshakePeer( } if err := nodeInfo.CompatibleWith(peerInfo); err != nil { + if err := r.peerManager.Inactivate(peerInfo.NodeID); err != nil { + return peerInfo, fmt.Errorf("problem inactivating peer %q: %w", peerInfo.ID(), err) + } + return peerInfo, ErrRejected{ err: err, id: peerInfo.ID(), @@ -761,7 +760,7 @@ func (r *Router) runWithPeerMutex(fn func() error) error { // channels. It will close the given connection and send queue when done, or if // they are closed elsewhere it will cause this method to shut down and return. func (r *Router) routePeer(ctx context.Context, peerID types.NodeID, conn Connection, channels ChannelIDSet) { - r.metrics.Peers.Add(1) + r.metrics.PeersConnected.Add(1) r.peerManager.Ready(ctx, peerID, channels) sendQueue := r.getOrMakeQueue(peerID, channels) @@ -774,7 +773,7 @@ func (r *Router) routePeer(ctx context.Context, peerID types.NodeID, conn Connec sendQueue.close() r.peerManager.Disconnected(ctx, peerID) - r.metrics.Peers.Add(-1) + r.metrics.PeersConnected.Add(-1) }() r.logger.Info("peer connected", "peer", peerID, "endpoint", conn) diff --git a/node/node.go b/node/node.go index 1bda1f0f7..77773044b 100644 --- a/node/node.go +++ b/node/node.go @@ -203,7 +203,7 @@ func makeNode( } } - peerManager, peerCloser, err := createPeerManager(cfg, dbProvider, nodeKey.ID) + peerManager, peerCloser, err := createPeerManager(cfg, dbProvider, nodeKey.ID, nodeMetrics.p2p) closers = append(closers, peerCloser) if err != nil { return nil, combineCloseError( diff --git a/node/seed.go b/node/seed.go index a0b71e411..92d55230f 100644 --- a/node/seed.go +++ b/node/seed.go @@ -67,7 +67,7 @@ func makeSeedNode( // Setup Transport and Switch. p2pMetrics := p2p.PrometheusMetrics(cfg.Instrumentation.Namespace, "chain_id", genDoc.ChainID) - peerManager, closer, err := createPeerManager(cfg, dbProvider, nodeKey.ID) + peerManager, closer, err := createPeerManager(cfg, dbProvider, nodeKey.ID, p2pMetrics) if err != nil { return nil, combineCloseError( fmt.Errorf("failed to create peer manager: %w", err), diff --git a/node/setup.go b/node/setup.go index 51a048249..345489f8d 100644 --- a/node/setup.go +++ b/node/setup.go @@ -202,6 +202,7 @@ func createPeerManager( cfg *config.Config, dbProvider config.DBProvider, nodeID types.NodeID, + metrics *p2p.Metrics, ) (*p2p.PeerManager, closer, error) { selfAddr, err := p2p.ParseNodeAddress(nodeID.AddressString(cfg.P2P.ExternalAddress)) @@ -223,18 +224,28 @@ func createPeerManager( maxConns = 64 } + var maxOutgoingConns uint16 + switch { + case cfg.P2P.MaxOutgoingConnections > 0: + maxOutgoingConns = cfg.P2P.MaxOutgoingConnections + default: + maxOutgoingConns = maxConns / 2 + } + maxUpgradeConns := uint16(4) options := p2p.PeerManagerOptions{ SelfAddress: selfAddr, MaxConnected: maxConns, + MaxOutgoingConnections: maxOutgoingConns, MaxConnectedUpgrade: maxUpgradeConns, - MaxPeers: maxUpgradeConns + 2*maxConns, + MaxPeers: maxUpgradeConns + 4*maxConns, MinRetryTime: 250 * time.Millisecond, MaxRetryTime: 30 * time.Minute, MaxRetryTimePersistent: 5 * time.Minute, RetryTimeJitter: 5 * time.Second, PrivatePeers: privatePeerIDs, + Metrics: metrics, } peers := []p2p.NodeAddress{} diff --git a/proto/tendermint/p2p/types.pb.go b/proto/tendermint/p2p/types.pb.go index bffa6884f..7965b668b 100644 --- a/proto/tendermint/p2p/types.pb.go +++ b/proto/tendermint/p2p/types.pb.go @@ -243,6 +243,7 @@ type PeerInfo struct { ID string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` AddressInfo []*PeerAddressInfo `protobuf:"bytes,2,rep,name=address_info,json=addressInfo,proto3" json:"address_info,omitempty"` LastConnected *time.Time `protobuf:"bytes,3,opt,name=last_connected,json=lastConnected,proto3,stdtime" json:"last_connected,omitempty"` + Inactive bool `protobuf:"varint,4,opt,name=inactive,proto3" json:"inactive,omitempty"` } func (m *PeerInfo) Reset() { *m = PeerInfo{} } @@ -299,6 +300,13 @@ func (m *PeerInfo) GetLastConnected() *time.Time { return nil } +func (m *PeerInfo) GetInactive() bool { + if m != nil { + return m.Inactive + } + return false +} + type PeerAddressInfo struct { Address string `protobuf:"bytes,1,opt,name=address,proto3" json:"address,omitempty"` LastDialSuccess *time.Time `protobuf:"bytes,2,opt,name=last_dial_success,json=lastDialSuccess,proto3,stdtime" json:"last_dial_success,omitempty"` @@ -378,46 +386,46 @@ func init() { func init() { proto.RegisterFile("tendermint/p2p/types.proto", fileDescriptor_c8a29e659aeca578) } var fileDescriptor_c8a29e659aeca578 = []byte{ - // 610 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x8c, 0x54, 0xcd, 0x4e, 0x1b, 0x3d, - 0x14, 0xcd, 0x24, 0x21, 0x09, 0x37, 0x84, 0xf0, 0x59, 0xe8, 0xd3, 0x10, 0xa9, 0x19, 0x14, 0x36, - 0xac, 0x26, 0x52, 0xaa, 0x2e, 0xba, 0x64, 0x40, 0xad, 0x22, 0x55, 0x25, 0x9a, 0xa2, 0x2e, 0xda, - 0xc5, 0x68, 0x32, 0x76, 0x82, 0xc5, 0xc4, 0xb6, 0x3c, 0x4e, 0x4b, 0xdf, 0x82, 0x37, 0xe9, 0x63, - 0x94, 0x25, 0xcb, 0xae, 0xd2, 0x6a, 0xd8, 0xf6, 0x21, 0x2a, 0xdb, 0x33, 0x40, 0xa2, 0x2e, 0xd8, - 0xf9, 0xdc, 0xe3, 0x73, 0xee, 0x8f, 0xad, 0x0b, 0x3d, 0x45, 0x18, 0x26, 0x72, 0x41, 0x99, 0x1a, - 0x8a, 0x91, 0x18, 0xaa, 0x6f, 0x82, 0x64, 0xbe, 0x90, 0x5c, 0x71, 0xb4, 0xfb, 0xc8, 0xf9, 0x62, - 0x24, 0x7a, 0xfb, 0x73, 0x3e, 0xe7, 0x86, 0x1a, 0xea, 0x93, 0xbd, 0xd5, 0xf3, 0xe6, 0x9c, 0xcf, - 0x53, 0x32, 0x34, 0x68, 0xba, 0x9c, 0x0d, 0x15, 0x5d, 0x90, 0x4c, 0xc5, 0x0b, 0x61, 0x2f, 0x0c, - 0x2e, 0xa0, 0x3b, 0xd1, 0x87, 0x84, 0xa7, 0x1f, 0x89, 0xcc, 0x28, 0x67, 0xe8, 0x00, 0x6a, 0x62, - 0x24, 0x5c, 0xe7, 0xd0, 0x39, 0xae, 0x07, 0xcd, 0x7c, 0xe5, 0xd5, 0x26, 0xa3, 0x49, 0xa8, 0x63, - 0x68, 0x1f, 0xb6, 0xa6, 0x29, 0x4f, 0xae, 0xdc, 0xaa, 0x26, 0x43, 0x0b, 0xd0, 0x1e, 0xd4, 0x62, - 0x21, 0xdc, 0x9a, 0x89, 0xe9, 0xe3, 0xe0, 0x47, 0x15, 0x5a, 0xef, 0x39, 0x26, 0x63, 0x36, 0xe3, - 0x68, 0x02, 0x7b, 0xa2, 0x48, 0x11, 0x7d, 0xb1, 0x39, 0x8c, 0x79, 0x7b, 0xe4, 0xf9, 0xeb, 0x4d, - 0xf8, 0x1b, 0xa5, 0x04, 0xf5, 0xdb, 0x95, 0x57, 0x09, 0xbb, 0x62, 0xa3, 0xc2, 0x23, 0x68, 0x32, - 0x8e, 0x49, 0x44, 0xb1, 0x29, 0x64, 0x3b, 0x80, 0x7c, 0xe5, 0x35, 0x4c, 0xc2, 0xb3, 0xb0, 0xa1, - 0xa9, 0x31, 0x46, 0x1e, 0xb4, 0x53, 0x9a, 0x29, 0xc2, 0xa2, 0x18, 0x63, 0x69, 0xaa, 0xdb, 0x0e, - 0xc1, 0x86, 0x4e, 0x30, 0x96, 0xc8, 0x85, 0x26, 0x23, 0xea, 0x2b, 0x97, 0x57, 0x6e, 0xdd, 0x90, - 0x25, 0xd4, 0x4c, 0x59, 0xe8, 0x96, 0x65, 0x0a, 0x88, 0x7a, 0xd0, 0x4a, 0x2e, 0x63, 0xc6, 0x48, - 0x9a, 0xb9, 0x8d, 0x43, 0xe7, 0x78, 0x27, 0x7c, 0xc0, 0x5a, 0xb5, 0xe0, 0x8c, 0x5e, 0x11, 0xe9, - 0x36, 0xad, 0xaa, 0x80, 0xe8, 0x35, 0x6c, 0x71, 0x75, 0x49, 0xa4, 0xdb, 0x32, 0x6d, 0xbf, 0xd8, - 0x6c, 0xbb, 0x1c, 0xd5, 0xb9, 0xbe, 0x54, 0x34, 0x6d, 0x15, 0x83, 0xcf, 0xd0, 0x59, 0x63, 0xd1, - 0x01, 0xb4, 0xd4, 0x75, 0x44, 0x19, 0x26, 0xd7, 0x66, 0x8a, 0xdb, 0x61, 0x53, 0x5d, 0x8f, 0x35, - 0x44, 0x43, 0x68, 0x4b, 0x91, 0x98, 0x76, 0x49, 0x96, 0x15, 0xa3, 0xd9, 0xcd, 0x57, 0x1e, 0x84, - 0x93, 0xd3, 0x13, 0x1b, 0x0d, 0x41, 0x8a, 0xa4, 0x38, 0x0f, 0xbe, 0x3b, 0xd0, 0x9a, 0x10, 0x22, - 0xcd, 0x33, 0xfd, 0x0f, 0x55, 0x8a, 0xad, 0x65, 0xd0, 0xc8, 0x57, 0x5e, 0x75, 0x7c, 0x16, 0x56, - 0x29, 0x46, 0x01, 0xec, 0x14, 0x8e, 0x11, 0x65, 0x33, 0xee, 0x56, 0x0f, 0x6b, 0xff, 0x7c, 0x3a, - 0x42, 0x64, 0xe1, 0xab, 0xed, 0xc2, 0x76, 0xfc, 0x08, 0xd0, 0x5b, 0xd8, 0x4d, 0xe3, 0x4c, 0x45, - 0x09, 0x67, 0x8c, 0x24, 0x8a, 0x60, 0xf3, 0x1c, 0xed, 0x51, 0xcf, 0xb7, 0xff, 0xd3, 0x2f, 0xff, - 0xa7, 0x7f, 0x51, 0xfe, 0xcf, 0xa0, 0x7e, 0xf3, 0xcb, 0x73, 0xc2, 0x8e, 0xd6, 0x9d, 0x96, 0xb2, - 0xc1, 0x1f, 0x07, 0xba, 0x1b, 0x99, 0xf4, 0xdc, 0xcb, 0x96, 0x8b, 0x81, 0x14, 0x10, 0xbd, 0x83, - 0xff, 0x4c, 0x5a, 0x4c, 0xe3, 0x34, 0xca, 0x96, 0x49, 0x52, 0x8e, 0xe5, 0x39, 0x99, 0xbb, 0x5a, - 0x7a, 0x46, 0xe3, 0xf4, 0x83, 0x15, 0xae, 0xbb, 0xcd, 0x62, 0x9a, 0x2e, 0x25, 0x79, 0x76, 0x1f, - 0x0f, 0x6e, 0x6f, 0xac, 0x10, 0x1d, 0x41, 0xe7, 0xa9, 0x51, 0x66, 0xfe, 0x60, 0x27, 0xdc, 0xc1, - 0x8f, 0x77, 0xb2, 0xe0, 0xfc, 0x36, 0xef, 0x3b, 0x77, 0x79, 0xdf, 0xf9, 0x9d, 0xf7, 0x9d, 0x9b, - 0xfb, 0x7e, 0xe5, 0xee, 0xbe, 0x5f, 0xf9, 0x79, 0xdf, 0xaf, 0x7c, 0x7a, 0x35, 0xa7, 0xea, 0x72, - 0x39, 0xf5, 0x13, 0xbe, 0x18, 0x3e, 0xd9, 0x12, 0x4f, 0x17, 0x86, 0xd9, 0x05, 0xeb, 0x1b, 0x64, - 0xda, 0x30, 0xd1, 0x97, 0x7f, 0x03, 0x00, 0x00, 0xff, 0xff, 0x0b, 0xe9, 0x56, 0xd3, 0x5a, 0x04, - 0x00, 0x00, + // 621 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x8c, 0x54, 0x41, 0x4f, 0xdb, 0x30, + 0x14, 0x6e, 0xda, 0xd2, 0x96, 0x57, 0x4a, 0x99, 0x85, 0xa6, 0x50, 0x69, 0x0d, 0x2a, 0x17, 0x4e, + 0x89, 0xd4, 0x69, 0x87, 0x1d, 0x09, 0x68, 0x53, 0xa5, 0x69, 0x54, 0x1e, 0xda, 0x61, 0x3b, 0x44, + 0x69, 0xec, 0x16, 0x8b, 0xd4, 0xb6, 0x12, 0x97, 0xb1, 0x7f, 0xc1, 0xbf, 0x1a, 0xd2, 0x2e, 0x1c, + 0x77, 0xea, 0xa6, 0x70, 0xdd, 0x8f, 0x98, 0xec, 0x24, 0xd0, 0x56, 0x3b, 0x70, 0xf3, 0xf7, 0x9e, + 0xbf, 0xcf, 0xdf, 0x7b, 0xcf, 0x7a, 0xd0, 0x53, 0x94, 0x13, 0x9a, 0xcc, 0x19, 0x57, 0x9e, 0x1c, + 0x4a, 0x4f, 0x7d, 0x97, 0x34, 0x75, 0x65, 0x22, 0x94, 0x40, 0xbb, 0x4f, 0x39, 0x57, 0x0e, 0x65, + 0x6f, 0x7f, 0x26, 0x66, 0xc2, 0xa4, 0x3c, 0x7d, 0xca, 0x6f, 0xf5, 0x9c, 0x99, 0x10, 0xb3, 0x98, + 0x7a, 0x06, 0x4d, 0x16, 0x53, 0x4f, 0xb1, 0x39, 0x4d, 0x55, 0x38, 0x97, 0xf9, 0x85, 0xc1, 0x05, + 0x74, 0xc7, 0xfa, 0x10, 0x89, 0xf8, 0x33, 0x4d, 0x52, 0x26, 0x38, 0x3a, 0x80, 0x9a, 0x1c, 0x4a, + 0xdb, 0x3a, 0xb4, 0x8e, 0xeb, 0x7e, 0x33, 0x5b, 0x3a, 0xb5, 0xf1, 0x70, 0x8c, 0x75, 0x0c, 0xed, + 0xc3, 0xd6, 0x24, 0x16, 0xd1, 0x95, 0x5d, 0xd5, 0x49, 0x9c, 0x03, 0xb4, 0x07, 0xb5, 0x50, 0x4a, + 0xbb, 0x66, 0x62, 0xfa, 0x38, 0xf8, 0x51, 0x85, 0xd6, 0x47, 0x41, 0xe8, 0x88, 0x4f, 0x05, 0x1a, + 0xc3, 0x9e, 0x2c, 0x9e, 0x08, 0xae, 0xf3, 0x37, 0x8c, 0x78, 0x7b, 0xe8, 0xb8, 0xeb, 0x45, 0xb8, + 0x1b, 0x56, 0xfc, 0xfa, 0xdd, 0xd2, 0xa9, 0xe0, 0xae, 0xdc, 0x70, 0x78, 0x04, 0x4d, 0x2e, 0x08, + 0x0d, 0x18, 0x31, 0x46, 0xb6, 0x7d, 0xc8, 0x96, 0x4e, 0xc3, 0x3c, 0x78, 0x86, 0x1b, 0x3a, 0x35, + 0x22, 0xc8, 0x81, 0x76, 0xcc, 0x52, 0x45, 0x79, 0x10, 0x12, 0x92, 0x18, 0x77, 0xdb, 0x18, 0xf2, + 0xd0, 0x09, 0x21, 0x09, 0xb2, 0xa1, 0xc9, 0xa9, 0xfa, 0x26, 0x92, 0x2b, 0xbb, 0x6e, 0x92, 0x25, + 0xd4, 0x99, 0xd2, 0xe8, 0x56, 0x9e, 0x29, 0x20, 0xea, 0x41, 0x2b, 0xba, 0x0c, 0x39, 0xa7, 0x71, + 0x6a, 0x37, 0x0e, 0xad, 0xe3, 0x1d, 0xfc, 0x88, 0x35, 0x6b, 0x2e, 0x38, 0xbb, 0xa2, 0x89, 0xdd, + 0xcc, 0x59, 0x05, 0x44, 0x6f, 0x61, 0x4b, 0xa8, 0x4b, 0x9a, 0xd8, 0x2d, 0x53, 0xf6, 0xab, 0xcd, + 0xb2, 0xcb, 0x56, 0x9d, 0xeb, 0x4b, 0x45, 0xd1, 0x39, 0x63, 0xf0, 0x15, 0x3a, 0x6b, 0x59, 0x74, + 0x00, 0x2d, 0x75, 0x13, 0x30, 0x4e, 0xe8, 0x8d, 0xe9, 0xe2, 0x36, 0x6e, 0xaa, 0x9b, 0x91, 0x86, + 0xc8, 0x83, 0x76, 0x22, 0x23, 0x53, 0x2e, 0x4d, 0xd3, 0xa2, 0x35, 0xbb, 0xd9, 0xd2, 0x01, 0x3c, + 0x3e, 0x3d, 0xc9, 0xa3, 0x18, 0x12, 0x19, 0x15, 0xe7, 0xc1, 0x4f, 0x0b, 0x5a, 0x63, 0x4a, 0x13, + 0x33, 0xa6, 0x97, 0x50, 0x65, 0x24, 0x97, 0xf4, 0x1b, 0xd9, 0xd2, 0xa9, 0x8e, 0xce, 0x70, 0x95, + 0x11, 0xe4, 0xc3, 0x4e, 0xa1, 0x18, 0x30, 0x3e, 0x15, 0x76, 0xf5, 0xb0, 0xf6, 0xdf, 0xd1, 0x51, + 0x9a, 0x14, 0xba, 0x5a, 0x0e, 0xb7, 0xc3, 0x27, 0x80, 0xde, 0xc3, 0x6e, 0x1c, 0xa6, 0x2a, 0x88, + 0x04, 0xe7, 0x34, 0x52, 0x94, 0x98, 0x71, 0xb4, 0x87, 0x3d, 0x37, 0xff, 0x9f, 0x6e, 0xf9, 0x3f, + 0xdd, 0x8b, 0xf2, 0x7f, 0xfa, 0xf5, 0xdb, 0xdf, 0x8e, 0x85, 0x3b, 0x9a, 0x77, 0x5a, 0xd2, 0x74, + 0xff, 0x19, 0x0f, 0x23, 0xc5, 0xae, 0xa9, 0x19, 0x5a, 0x0b, 0x3f, 0xe2, 0xc1, 0x5f, 0x0b, 0xba, + 0x1b, 0x2e, 0xf4, 0x4c, 0xca, 0x76, 0x14, 0xcd, 0x2a, 0x20, 0xfa, 0x00, 0x2f, 0x8c, 0x25, 0xc2, + 0xc2, 0x38, 0x48, 0x17, 0x51, 0x54, 0xb6, 0xec, 0x39, 0xae, 0xba, 0x9a, 0x7a, 0xc6, 0xc2, 0xf8, + 0x53, 0x4e, 0x5c, 0x57, 0x9b, 0x86, 0x2c, 0x5e, 0x24, 0xf4, 0xd9, 0x35, 0x3e, 0xaa, 0xbd, 0xcb, + 0x89, 0xe8, 0x08, 0x3a, 0xab, 0x42, 0xa9, 0x29, 0xb5, 0x83, 0x77, 0xc8, 0xd3, 0x9d, 0xd4, 0x3f, + 0xbf, 0xcb, 0xfa, 0xd6, 0x7d, 0xd6, 0xb7, 0xfe, 0x64, 0x7d, 0xeb, 0xf6, 0xa1, 0x5f, 0xb9, 0x7f, + 0xe8, 0x57, 0x7e, 0x3d, 0xf4, 0x2b, 0x5f, 0xde, 0xcc, 0x98, 0xba, 0x5c, 0x4c, 0xdc, 0x48, 0xcc, + 0xbd, 0x95, 0x0d, 0xb2, 0xba, 0x4c, 0xcc, 0x9e, 0x58, 0xdf, 0x2e, 0x93, 0x86, 0x89, 0xbe, 0xfe, + 0x17, 0x00, 0x00, 0xff, 0xff, 0x42, 0xcb, 0x37, 0x26, 0x76, 0x04, 0x00, 0x00, } func (m *ProtocolVersion) Marshal() (dAtA []byte, err error) { @@ -600,6 +608,16 @@ func (m *PeerInfo) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l + if m.Inactive { + i-- + if m.Inactive { + dAtA[i] = 1 + } else { + dAtA[i] = 0 + } + i-- + dAtA[i] = 0x20 + } if m.LastConnected != nil { n3, err3 := github_com_gogo_protobuf_types.StdTimeMarshalTo(*m.LastConnected, dAtA[i-github_com_gogo_protobuf_types.SizeOfStdTime(*m.LastConnected):]) if err3 != nil { @@ -792,6 +810,9 @@ func (m *PeerInfo) Size() (n int) { l = github_com_gogo_protobuf_types.SizeOfStdTime(*m.LastConnected) n += 1 + l + sovTypes(uint64(l)) } + if m.Inactive { + n += 2 + } return n } @@ -1487,6 +1508,26 @@ func (m *PeerInfo) Unmarshal(dAtA []byte) error { return err } iNdEx = postIndex + case 4: + if wireType != 0 { + return fmt.Errorf("proto: wrong wireType = %d for field Inactive", wireType) + } + var v int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTypes + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + v |= int(b&0x7F) << shift + if b < 0x80 { + break + } + } + m.Inactive = bool(v != 0) default: iNdEx = preIndex skippy, err := skipTypes(dAtA[iNdEx:]) diff --git a/proto/tendermint/p2p/types.proto b/proto/tendermint/p2p/types.proto index faccd59d2..a2ed6c90f 100644 --- a/proto/tendermint/p2p/types.proto +++ b/proto/tendermint/p2p/types.proto @@ -32,6 +32,7 @@ message PeerInfo { string id = 1 [(gogoproto.customname) = "ID"]; repeated PeerAddressInfo address_info = 2; google.protobuf.Timestamp last_connected = 3 [(gogoproto.stdtime) = true]; + bool inactive = 4; } message PeerAddressInfo {