From 1dabae273bc4b577586938d4d54cc591ff97fdd4 Mon Sep 17 00:00:00 2001 From: tycho garen Date: Mon, 13 Jun 2022 10:47:25 -0400 Subject: [PATCH] cleanup peer manager --- internal/p2p/peermanager.go | 7 +-- internal/p2p/peermanager_scoring_test.go | 2 +- internal/p2p/peermanager_test.go | 63 ++++++++++-------------- 3 files changed, 29 insertions(+), 43 deletions(-) diff --git a/internal/p2p/peermanager.go b/internal/p2p/peermanager.go index f2e7f3f54..b88837dc1 100644 --- a/internal/p2p/peermanager.go +++ b/internal/p2p/peermanager.go @@ -44,7 +44,6 @@ type PeerScore int16 const ( PeerScorePersistent PeerScore = math.MaxInt16 // persistent peers MaxPeerScoreNotPersistent PeerScore = PeerScorePersistent - 1 - DefaultMutablePeerScore = 256 ) // PeerUpdate is a peer update event sent via PeerUpdates. @@ -433,10 +432,6 @@ func (m *PeerManager) Add(address NodeAddress) (bool, error) { return false, nil } - // set the peer's mutable score to something non-zero so that - // peer's we've never seen aren't very low at start. - peer.MutableScore = DefaultMutablePeerScore - // else add the new address peer.AddressInfo[address] = &peerAddressInfo{Address: address} if err := m.store.Set(peer); err != nil { @@ -1060,6 +1055,8 @@ 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]: diff --git a/internal/p2p/peermanager_scoring_test.go b/internal/p2p/peermanager_scoring_test.go index 02aa3a32e..5171db287 100644 --- a/internal/p2p/peermanager_scoring_test.go +++ b/internal/p2p/peermanager_scoring_test.go @@ -31,7 +31,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++ { diff --git a/internal/p2p/peermanager_test.go b/internal/p2p/peermanager_test.go index 3db40a26c..37ed2e865 100644 --- a/internal/p2p/peermanager_test.go +++ b/internal/p2p/peermanager_test.go @@ -504,11 +504,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: p2p.PeerScore(0 + p2p.DefaultMutablePeerScore), - b.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(2 + p2p.DefaultMutablePeerScore), - d.NodeID: p2p.PeerScore(3 + p2p.DefaultMutablePeerScore), - e.NodeID: p2p.PeerScore(0 + p2p.DefaultMutablePeerScore), + 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, @@ -561,10 +561,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(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 @@ -585,10 +583,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: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - }, + PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: p2p.PeerScore(1), c.NodeID: 1}, MaxConnected: 1, MaxConnectedUpgrade: 2, }) @@ -746,8 +741,8 @@ func TestPeerManager_DialFailed_UnreservePeer(t *testing.T) { peerManager, err := p2p.NewPeerManager(selfID, dbm.NewMemDB(), p2p.PeerManagerOptions{ PeerScores: map[types.NodeID]p2p.PeerScore{ - b.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), + b.NodeID: p2p.PeerScore(1), + c.NodeID: p2p.PeerScore(2), }, MaxConnected: 1, MaxConnectedUpgrade: 2, @@ -864,10 +859,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: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - d.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - }, + PeerScores: map[types.NodeID]p2p.PeerScore{c.NodeID: p2p.PeerScore(1), d.NodeID: 1}, }) require.NoError(t, err) @@ -917,10 +909,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: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - }, + PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: p2p.PeerScore(1), c.NodeID: 1}, }) require.NoError(t, err) @@ -964,10 +953,10 @@ func TestPeerManager_Dialed_UpgradeEvenLower(t *testing.T) { MaxConnected: 2, MaxConnectedUpgrade: 1, PeerScores: map[types.NodeID]p2p.PeerScore{ - a.NodeID: p2p.PeerScore(3 + p2p.DefaultMutablePeerScore), - b.NodeID: p2p.PeerScore(2 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(10 + p2p.DefaultMutablePeerScore), - d.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), + a.NodeID: p2p.PeerScore(3), + b.NodeID: p2p.PeerScore(2), + c.NodeID: p2p.PeerScore(10), + d.NodeID: p2p.PeerScore(1), }, }) require.NoError(t, err) @@ -1017,9 +1006,9 @@ func TestPeerManager_Dialed_UpgradeNoEvict(t *testing.T) { MaxConnected: 2, MaxConnectedUpgrade: 1, PeerScores: map[types.NodeID]p2p.PeerScore{ - a.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - b.NodeID: p2p.PeerScore(2 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(3 + p2p.DefaultMutablePeerScore), + a.NodeID: p2p.PeerScore(1), + b.NodeID: p2p.PeerScore(2), + c.NodeID: p2p.PeerScore(3), }, }) require.NoError(t, err) @@ -1138,8 +1127,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: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - d.NodeID: p2p.PeerScore(2 + p2p.DefaultMutablePeerScore), + c.NodeID: p2p.PeerScore(1), + d.NodeID: p2p.PeerScore(2), }, MaxConnected: 1, MaxConnectedUpgrade: 1, @@ -1183,8 +1172,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: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), + b.NodeID: p2p.PeerScore(1), + c.NodeID: p2p.PeerScore(1), }, MaxConnected: 1, MaxConnectedUpgrade: 2, @@ -1226,8 +1215,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: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), - c.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), + b.NodeID: p2p.PeerScore(1), + c.NodeID: p2p.PeerScore(1), }, MaxConnected: 1, MaxConnectedUpgrade: 2, @@ -1388,7 +1377,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: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore)}, + PeerScores: map[types.NodeID]p2p.PeerScore{b.NodeID: p2p.PeerScore(1)}, }) require.NoError(t, err) @@ -1427,7 +1416,7 @@ func TestPeerManager_EvictNext_WakeOnUpgradeAccepted(t *testing.T) { MaxConnected: 1, MaxConnectedUpgrade: 1, PeerScores: map[types.NodeID]p2p.PeerScore{ - b.NodeID: p2p.PeerScore(1 + p2p.DefaultMutablePeerScore), + b.NodeID: p2p.PeerScore(1), }, }) require.NoError(t, err)