From 0c9429884731af6d1247c992e15320f99a23349f Mon Sep 17 00:00:00 2001 From: Victor Farazdagi Date: Thu, 13 Aug 2020 12:36:50 +0300 Subject: [PATCH] Peer scorer: fix weighted sorting (#6981) * peer scorer: fix weighted sorting * Merge refs/heads/master into minor-fixes-to-peer-scorer * Merge refs/heads/master into minor-fixes-to-peer-scorer --- .../p2p/peers/score_block_providers.go | 13 ++++----- .../p2p/peers/score_block_providers_test.go | 20 +++++++------- beacon-chain/p2p/peers/scorer_manager_test.go | 27 ++++++++++--------- 3 files changed, 32 insertions(+), 28 deletions(-) diff --git a/beacon-chain/p2p/peers/score_block_providers.go b/beacon-chain/p2p/peers/score_block_providers.go index 8ef6dd2d0b97..3129a101ed32 100644 --- a/beacon-chain/p2p/peers/score_block_providers.go +++ b/beacon-chain/p2p/peers/score_block_providers.go @@ -15,20 +15,20 @@ import ( const ( // DefaultBlockProviderProcessedBatchWeight is a default reward weight of a processed batch of blocks. - DefaultBlockProviderProcessedBatchWeight = float64(0.05) + DefaultBlockProviderProcessedBatchWeight = float64(0.1) // DefaultBlockProviderProcessedBlocksCap defines default value for processed blocks cap. // e.g. 20 * 64 := 20 batches of size 64 (with 0.05 per batch reward, 20 batches result in score of 1.0). - DefaultBlockProviderProcessedBlocksCap = uint64(20 * 64) + DefaultBlockProviderProcessedBlocksCap = uint64(10 * 64) // DefaultBlockProviderDecayInterval defines how often the decaying routine is called. DefaultBlockProviderDecayInterval = 30 * time.Second // DefaultBlockProviderDecay defines default blocks that are to be subtracted from stats on each // decay interval. Effectively, this param provides minimum expected performance for a peer to remain // high scorer. - DefaultBlockProviderDecay = uint64(5 * 64) + DefaultBlockProviderDecay = uint64(1 * 64) // DefaultBlockProviderStalePeerRefreshInterval defines default interval at which peers should be given // opportunity to provide blocks (their score gets boosted, up until they are selected for // fetching). - DefaultBlockProviderStalePeerRefreshInterval = 1 * time.Minute + DefaultBlockProviderStalePeerRefreshInterval = 5 * time.Minute ) // BlockProviderScorer represents block provider scoring service. @@ -209,14 +209,15 @@ func (s *BlockProviderScorer) WeightSorted( nextPID := func(weights map[peer.ID]float64) peer.ID { totalWeight := 0 for _, w := range weights { - totalWeight += int(w) + // Factor by 100, to allow weights in (0; 1) range. + totalWeight += int(w * 100) } if totalWeight <= 0 { return "" } rnd := r.Intn(totalWeight) for pid, w := range weights { - rnd -= int(w) + rnd -= int(w * 100) if rnd < 0 { return pid } diff --git a/beacon-chain/p2p/peers/score_block_providers_test.go b/beacon-chain/p2p/peers/score_block_providers_test.go index 63fdaefd1f58..32b48acafa90 100644 --- a/beacon-chain/p2p/peers/score_block_providers_test.go +++ b/beacon-chain/p2p/peers/score_block_providers_test.go @@ -51,8 +51,9 @@ func TestPeerScorer_BlockProvider_Score(t *testing.T) { { name: "boost score of stale peer", update: func(scorer *peers.BlockProviderScorer) { + batchWeight := scorer.Params().ProcessedBatchWeight scorer.IncrementProcessedBlocks("peer1", batchSize*3) - assert.Equal(t, 0.05*3, scorer.Score("peer1"), "Unexpected score") + assert.Equal(t, roundScore(batchWeight*3), scorer.Score("peer1"), "Unexpected score") scorer.Touch("peer1", roughtime.Now().Add(-1*scorer.Params().StalePeerRefreshInterval)) }, check: func(scorer *peers.BlockProviderScorer) { @@ -85,23 +86,26 @@ func TestPeerScorer_BlockProvider_Score(t *testing.T) { scorer.IncrementProcessedBlocks("peer1", batchSize) }, check: func(scorer *peers.BlockProviderScorer) { - assert.Equal(t, roundScore(0.05), scorer.Score("peer1"), "Unexpected score") + batchWeight := scorer.Params().ProcessedBatchWeight + assert.Equal(t, roundScore(batchWeight), scorer.Score("peer1"), "Unexpected score") }, }, { name: "multiple batches", update: func(scorer *peers.BlockProviderScorer) { - scorer.IncrementProcessedBlocks("peer1", batchSize*13) + scorer.IncrementProcessedBlocks("peer1", batchSize*7) }, check: func(scorer *peers.BlockProviderScorer) { - assert.Equal(t, roundScore(0.05*13), scorer.Score("peer1"), "Unexpected score") + batchWeight := scorer.Params().ProcessedBatchWeight + assert.Equal(t, roundScore(batchWeight*7), scorer.Score("peer1"), "Unexpected score") }, }, { name: "maximum score cap", update: func(scorer *peers.BlockProviderScorer) { + batchWeight := scorer.Params().ProcessedBatchWeight scorer.IncrementProcessedBlocks("peer1", batchSize*2) - assert.Equal(t, roundScore(0.05*2), scorer.Score("peer1"), "Unexpected score") + assert.Equal(t, roundScore(batchWeight*2), scorer.Score("peer1"), "Unexpected score") scorer.IncrementProcessedBlocks("peer1", scorer.Params().ProcessedBlocksCap) }, check: func(scorer *peers.BlockProviderScorer) { @@ -116,9 +120,7 @@ func TestPeerScorer_BlockProvider_Score(t *testing.T) { peerStatuses := peers.NewStatus(ctx, &peers.StatusConfig{ PeerLimit: 30, ScorerParams: &peers.PeerScorerConfig{ - BlockProviderScorerConfig: &peers.BlockProviderScorerConfig{ - ProcessedBatchWeight: 0.05, - }, + BlockProviderScorerConfig: &peers.BlockProviderScorerConfig{}, }, }) scorer := peerStatuses.Scorers().BlockProviderScorer() @@ -150,7 +152,7 @@ func TestPeerScorer_BlockProvider_WeightSorted(t *testing.T) { peerStatuses := peers.NewStatus(ctx, &peers.StatusConfig{ ScorerParams: &peers.PeerScorerConfig{ BlockProviderScorerConfig: &peers.BlockProviderScorerConfig{ - ProcessedBatchWeight: 1, + ProcessedBatchWeight: 0.01, }, }, }) diff --git a/beacon-chain/p2p/peers/scorer_manager_test.go b/beacon-chain/p2p/peers/scorer_manager_test.go index cc830ecf97ba..43fc78e76529 100644 --- a/beacon-chain/p2p/peers/scorer_manager_test.go +++ b/beacon-chain/p2p/peers/scorer_manager_test.go @@ -109,8 +109,7 @@ func TestPeerScorer_PeerScorerManager_Score(t *testing.T) { Threshold: 5, }, BlockProviderScorerConfig: &peers.BlockProviderScorerConfig{ - ProcessedBatchWeight: 0.05, - Decay: 64, + Decay: 64, }, }, }) @@ -156,6 +155,7 @@ func TestPeerScorer_PeerScorerManager_Score(t *testing.T) { s, pids := setupScorer() s1 := s.BlockProviderScorer() zeroScore := s.BlockProviderScorer().MaxScore() + batchWeight := s1.Params().ProcessedBatchWeight // Partial batch. s1.IncrementProcessedBlocks("peer1", batchSize/4) @@ -163,26 +163,26 @@ func TestPeerScorer_PeerScorerManager_Score(t *testing.T) { // Single batch. s1.IncrementProcessedBlocks("peer1", batchSize) - assert.DeepEqual(t, pack(s, 0.05, zeroScore, zeroScore), peerScores(s, pids), "Unexpected scores") + assert.DeepEqual(t, pack(s, batchWeight, zeroScore, zeroScore), peerScores(s, pids), "Unexpected scores") // Multiple batches. s1.IncrementProcessedBlocks("peer2", batchSize*4) - assert.DeepEqual(t, pack(s, 0.05, 0.05*4, zeroScore), peerScores(s, pids), "Unexpected scores") + assert.DeepEqual(t, pack(s, batchWeight, batchWeight*4, zeroScore), peerScores(s, pids), "Unexpected scores") // Partial batch. s1.IncrementProcessedBlocks("peer3", batchSize/2) - assert.DeepEqual(t, pack(s, 0.05, 0.05*4, 0), peerScores(s, pids), "Unexpected scores") + assert.DeepEqual(t, pack(s, batchWeight, batchWeight*4, 0), peerScores(s, pids), "Unexpected scores") // See effect of decaying. assert.Equal(t, batchSize+batchSize/4, s1.ProcessedBlocks("peer1")) assert.Equal(t, batchSize*4, s1.ProcessedBlocks("peer2")) assert.Equal(t, batchSize/2, s1.ProcessedBlocks("peer3")) - assert.DeepEqual(t, pack(s, 0.05, 0.05*4, 0), peerScores(s, pids), "Unexpected scores") + assert.DeepEqual(t, pack(s, batchWeight, batchWeight*4, 0), peerScores(s, pids), "Unexpected scores") s1.Decay() assert.Equal(t, batchSize/4, s1.ProcessedBlocks("peer1")) assert.Equal(t, batchSize*3, s1.ProcessedBlocks("peer2")) assert.Equal(t, uint64(0), s1.ProcessedBlocks("peer3")) - assert.DeepEqual(t, pack(s, 0, 0.05*3, 0), peerScores(s, pids), "Unexpected scores") + assert.DeepEqual(t, pack(s, 0, batchWeight*3, 0), peerScores(s, pids), "Unexpected scores") }) t.Run("overall score", func(t *testing.T) { @@ -190,20 +190,21 @@ func TestPeerScorer_PeerScorerManager_Score(t *testing.T) { s, _ := setupScorer() s1 := s.BlockProviderScorer() s2 := s.BadResponsesScorer() + batchWeight := s1.Params().ProcessedBatchWeight - s1.IncrementProcessedBlocks("peer1", batchSize*10) - assert.Equal(t, roundScore(0.05*10), s1.Score("peer1")) + s1.IncrementProcessedBlocks("peer1", batchSize*5) + assert.Equal(t, roundScore(batchWeight*5), s1.Score("peer1")) // Now, adjust score by introducing penalty for bad responses. s2.Increment("peer1") s2.Increment("peer1") assert.Equal(t, -0.4, s2.Score("peer1"), "Unexpected bad responses score") - assert.Equal(t, roundScore(0.05*10), s1.Score("peer1"), "Unexpected block provider score") - assert.Equal(t, roundScore(0.05*10-0.4), s.Score("peer1"), "Unexpected overall score") + assert.Equal(t, roundScore(batchWeight*5), s1.Score("peer1"), "Unexpected block provider score") + assert.Equal(t, roundScore(batchWeight*5-0.4), s.Score("peer1"), "Unexpected overall score") // If peer continues to misbehave, score becomes negative. s2.Increment("peer1") assert.Equal(t, -0.6, s2.Score("peer1"), "Unexpected bad responses score") - assert.Equal(t, roundScore(0.05*10), s1.Score("peer1"), "Unexpected block provider score") - assert.Equal(t, -0.1, s.Score("peer1"), "Unexpected overall score") + assert.Equal(t, roundScore(batchWeight*5), s1.Score("peer1"), "Unexpected block provider score") + assert.Equal(t, roundScore(batchWeight*5-0.6), s.Score("peer1"), "Unexpected overall score") }) }