Skip to content

Commit d5593a4

Browse files
algonautshantalgonautshant
authored
network: peer selector expansion and PeersPhonebookArchivalNodes (#5385)
1 parent fd60def commit d5593a4

File tree

8 files changed

+242
-55
lines changed

8 files changed

+242
-55
lines changed

‎catchup/peerSelector.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,16 @@ const (
4747
peerRank3LowBlockTime = 601
4848
peerRank3HighBlockTime = 799
4949

50+
peerRankInitialFifthPriority = 800
51+
peerRank4LowBlockTime = 801
52+
peerRank4HighBlockTime = 999
53+
5054
// peerRankDownloadFailed is used for responses which could be temporary, such as missing files, or such that we don't
5155
// have clear resolution
52-
peerRankDownloadFailed = 900
56+
peerRankDownloadFailed = 10000
5357
// peerRankInvalidDownload is used for responses which are likely to be invalid - whether it's serving the wrong content
5458
// or attempting to serve malicious content
55-
peerRankInvalidDownload = 1000
59+
peerRankInvalidDownload = 12000
5660

5761
// once a block is downloaded, the download duration is clamped into the range of [lowBlockDownloadThreshold..highBlockDownloadThreshold] and
5862
// then mapped into the a ranking range.
@@ -383,8 +387,10 @@ func (ps *peerSelector) peerDownloadDurationToRank(psp *peerSelectorPeer, blockD
383387
return downloadDurationToRank(blockDownloadDuration, lowBlockDownloadThreshold, highBlockDownloadThreshold, peerRank1LowBlockTime, peerRank1HighBlockTime)
384388
case peerRankInitialThirdPriority:
385389
return downloadDurationToRank(blockDownloadDuration, lowBlockDownloadThreshold, highBlockDownloadThreshold, peerRank2LowBlockTime, peerRank2HighBlockTime)
386-
default: // i.e. peerRankInitialFourthPriority
390+
case peerRankInitialFourthPriority:
387391
return downloadDurationToRank(blockDownloadDuration, lowBlockDownloadThreshold, highBlockDownloadThreshold, peerRank3LowBlockTime, peerRank3HighBlockTime)
392+
default: // i.e. peerRankInitialFifthPriority
393+
return downloadDurationToRank(blockDownloadDuration, lowBlockDownloadThreshold, highBlockDownloadThreshold, peerRank4LowBlockTime, peerRank4HighBlockTime)
388394
}
389395
}
390396

@@ -520,8 +526,10 @@ func lowerBound(class peerClass) int {
520526
return peerRank1LowBlockTime
521527
case peerRankInitialThirdPriority:
522528
return peerRank2LowBlockTime
523-
default: // i.e. peerRankInitialFourthPriority
529+
case peerRankInitialFourthPriority:
524530
return peerRank3LowBlockTime
531+
default: // i.e. peerRankInitialFifthPriority
532+
return peerRank4LowBlockTime
525533
}
526534
}
527535

@@ -533,8 +541,10 @@ func upperBound(class peerClass) int {
533541
return peerRank1HighBlockTime
534542
case peerRankInitialThirdPriority:
535543
return peerRank2HighBlockTime
536-
default: // i.e. peerRankInitialFourthPriority
544+
case peerRankInitialFourthPriority:
537545
return peerRank3HighBlockTime
546+
default: // i.e. peerRankInitialFifthPriority
547+
return peerRank4HighBlockTime
538548
}
539549
}
540550

‎catchup/peerSelector_test.go

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -381,12 +381,12 @@ func TestPeersDownloadFailed(t *testing.T) {
381381
if len(peerSelector.pools) == 2 {
382382
b1orb2 := peerAddress(peerSelector.pools[0].peers[1].peer) == "b1" || peerAddress(peerSelector.pools[0].peers[1].peer) == "b2"
383383
require.True(t, b1orb2)
384-
require.Equal(t, peerSelector.pools[1].rank, 900)
384+
require.Equal(t, peerSelector.pools[1].rank, peerRankDownloadFailed)
385385
require.Equal(t, len(peerSelector.pools[1].peers), 3)
386386
} else { // len(pools) == 3
387387
b1orb2 := peerAddress(peerSelector.pools[1].peers[0].peer) == "b1" || peerAddress(peerSelector.pools[1].peers[0].peer) == "b2"
388388
require.True(t, b1orb2)
389-
require.Equal(t, peerSelector.pools[2].rank, 900)
389+
require.Equal(t, peerSelector.pools[2].rank, peerRankDownloadFailed)
390390
require.Equal(t, len(peerSelector.pools[2].peers), 3)
391391
}
392392

@@ -459,6 +459,7 @@ func TestPeerDownloadDurationToRank(t *testing.T) {
459459
peers2 := []network.Peer{&mockHTTPPeer{address: "b1"}, &mockHTTPPeer{address: "b2"}}
460460
peers3 := []network.Peer{&mockHTTPPeer{address: "c1"}, &mockHTTPPeer{address: "c2"}}
461461
peers4 := []network.Peer{&mockHTTPPeer{address: "d1"}, &mockHTTPPeer{address: "b2"}}
462+
peers5 := []network.Peer{&mockHTTPPeer{address: "e1"}, &mockHTTPPeer{address: "b2"}}
462463

463464
peerSelector := makePeerSelector(
464465
makePeersRetrieverStub(func(options ...network.PeerOption) (peers []network.Peer) {
@@ -469,15 +470,18 @@ func TestPeerDownloadDurationToRank(t *testing.T) {
469470
peers = append(peers, peers2...)
470471
} else if opt == network.PeersConnectedOut {
471472
peers = append(peers, peers3...)
472-
} else {
473+
} else if opt == network.PeersPhonebookArchivalNodes {
473474
peers = append(peers, peers4...)
475+
} else { // PeersConnectedIn
476+
peers = append(peers, peers5...)
474477
}
475478
}
476479
return
477480
}), []peerClass{{initialRank: peerRankInitialFirstPriority, peerClass: network.PeersPhonebookArchivers},
478481
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersPhonebookRelays},
479482
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersConnectedOut},
480-
{initialRank: peerRankInitialFourthPriority, peerClass: network.PeersConnectedIn}},
483+
{initialRank: peerRankInitialFourthPriority, peerClass: network.PeersPhonebookArchivalNodes},
484+
{initialRank: peerRankInitialFifthPriority, peerClass: network.PeersConnectedIn}},
481485
)
482486

483487
_, err := peerSelector.getNextPeer()
@@ -490,7 +494,10 @@ func TestPeerDownloadDurationToRank(t *testing.T) {
490494
require.Equal(t, downloadDurationToRank(500*time.Millisecond, lowBlockDownloadThreshold, highBlockDownloadThreshold, peerRank2LowBlockTime, peerRank2HighBlockTime),
491495
peerSelector.peerDownloadDurationToRank(&peerSelectorPeer{peers3[0], network.PeersConnectedOut}, 500*time.Millisecond))
492496
require.Equal(t, downloadDurationToRank(500*time.Millisecond, lowBlockDownloadThreshold, highBlockDownloadThreshold, peerRank3LowBlockTime, peerRank3HighBlockTime),
493-
peerSelector.peerDownloadDurationToRank(&peerSelectorPeer{peers4[0], network.PeersConnectedIn}, 500*time.Millisecond))
497+
peerSelector.peerDownloadDurationToRank(&peerSelectorPeer{peers4[0], network.PeersPhonebookArchivalNodes}, 500*time.Millisecond))
498+
require.Equal(t, downloadDurationToRank(500*time.Millisecond, lowBlockDownloadThreshold, highBlockDownloadThreshold, peerRank4LowBlockTime, peerRank4HighBlockTime),
499+
peerSelector.peerDownloadDurationToRank(&peerSelectorPeer{peers5[0], network.PeersConnectedIn}, 500*time.Millisecond))
500+
494501
}
495502

496503
func TestLowerUpperBounds(t *testing.T) {
@@ -499,23 +506,26 @@ func TestLowerUpperBounds(t *testing.T) {
499506
classes := []peerClass{{initialRank: peerRankInitialFirstPriority, peerClass: network.PeersPhonebookArchivers},
500507
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersPhonebookRelays},
501508
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersConnectedOut},
502-
{initialRank: peerRankInitialFourthPriority, peerClass: network.PeersConnectedIn}}
509+
{initialRank: peerRankInitialFourthPriority, peerClass: network.PeersConnectedIn},
510+
{initialRank: peerRankInitialFifthPriority, peerClass: network.PeersConnectedIn}}
503511

504512
require.Equal(t, peerRank0LowBlockTime, lowerBound(classes[0]))
505513
require.Equal(t, peerRank1LowBlockTime, lowerBound(classes[1]))
506514
require.Equal(t, peerRank2LowBlockTime, lowerBound(classes[2]))
507515
require.Equal(t, peerRank3LowBlockTime, lowerBound(classes[3]))
516+
require.Equal(t, peerRank4LowBlockTime, lowerBound(classes[4]))
508517

509518
require.Equal(t, peerRank0HighBlockTime, upperBound(classes[0]))
510519
require.Equal(t, peerRank1HighBlockTime, upperBound(classes[1]))
511520
require.Equal(t, peerRank2HighBlockTime, upperBound(classes[2]))
512521
require.Equal(t, peerRank3HighBlockTime, upperBound(classes[3]))
522+
require.Equal(t, peerRank4HighBlockTime, upperBound(classes[4]))
513523
}
514524

515525
func TestFullResetRequestPenalty(t *testing.T) {
516526
partitiontest.PartitionTest(t)
517527

518-
class := peerClass{initialRank: 10, peerClass: network.PeersPhonebookArchivers}
528+
class := peerClass{initialRank: 0, peerClass: network.PeersPhonebookArchivers}
519529
hs := makeHistoricStatus(10, class)
520530
hs.push(5, 1, class)
521531
require.Equal(t, 1, len(hs.requestGaps))
@@ -524,6 +534,30 @@ func TestFullResetRequestPenalty(t *testing.T) {
524534
require.Equal(t, 0, len(hs.requestGaps))
525535
}
526536

537+
// TesPenaltyBounds makes sure that the penalty does not demote the peer to a lower class,
538+
// and resetting the penalty of a demoted peer does not promote it back
539+
func TestPenaltyBounds(t *testing.T) {
540+
partitiontest.PartitionTest(t)
541+
542+
class := peerClass{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersPhonebookArchivers}
543+
hs := makeHistoricStatus(peerHistoryWindowSize, class)
544+
for x := 0; x < 65; x++ {
545+
r0 := hs.push(peerRank2LowBlockTime+50, uint64(x+1), class)
546+
require.LessOrEqual(t, peerRank2LowBlockTime, r0)
547+
require.GreaterOrEqual(t, peerRank2HighBlockTime, r0)
548+
}
549+
550+
r1 := hs.resetRequestPenalty(4, peerRankInitialThirdPriority, class)
551+
r2 := hs.resetRequestPenalty(10, peerRankInitialThirdPriority, class)
552+
r3 := hs.resetRequestPenalty(10, peerRankDownloadFailed, class)
553+
554+
// r2 is at a better rank than r1 because it has one penalty less
555+
require.Greater(t, r1, r2)
556+
557+
// r3 is worse rank at peerRankDownloadFailed because it was demoted and resetRequestPenalty should not improve it
558+
require.Equal(t, peerRankDownloadFailed, r3)
559+
}
560+
527561
// TestClassUpperBound makes sure the peer rank does not exceed the class upper bound
528562
// This was a bug where the resetRequestPenalty was not bounding the returned rank, and was having download failures.
529563
// Initializing rankSamples to 0 makes this works, since the dropped value subtracts 0 from rankSum.
@@ -613,7 +647,7 @@ func TestEvictionAndUpgrade(t *testing.T) {
613647
_, err := peerSelector.getNextPeer()
614648
require.NoError(t, err)
615649
for i := 0; i < 10; i++ {
616-
if peerSelector.pools[len(peerSelector.pools)-1].rank == 900 {
650+
if peerSelector.pools[len(peerSelector.pools)-1].rank == peerRankDownloadFailed {
617651
require.Equal(t, 6, i)
618652
break
619653
}

‎catchup/service.go

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -816,30 +816,34 @@ func createPeerSelector(net network.GossipNode, cfg config.Local, pipelineFetch
816816
if cfg.NetAddress != "" { // Relay node
817817
peerClasses = []peerClass{
818818
{initialRank: peerRankInitialFirstPriority, peerClass: network.PeersConnectedOut},
819-
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersPhonebookArchivers},
820-
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersPhonebookRelays},
821-
{initialRank: peerRankInitialFourthPriority, peerClass: network.PeersConnectedIn},
819+
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersPhonebookArchivalNodes},
820+
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersPhonebookArchivers},
821+
{initialRank: peerRankInitialFourthPriority, peerClass: network.PeersPhonebookRelays},
822+
{initialRank: peerRankInitialFifthPriority, peerClass: network.PeersConnectedIn},
822823
}
823824
} else {
824825
peerClasses = []peerClass{
825-
{initialRank: peerRankInitialFirstPriority, peerClass: network.PeersPhonebookArchivers},
826-
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersConnectedOut},
827-
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersPhonebookRelays},
826+
{initialRank: peerRankInitialFirstPriority, peerClass: network.PeersPhonebookArchivalNodes},
827+
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersPhonebookArchivers},
828+
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersConnectedOut},
829+
{initialRank: peerRankInitialFourthPriority, peerClass: network.PeersPhonebookRelays},
828830
}
829831
}
830832
} else {
831833
if cfg.NetAddress != "" { // Relay node
832834
peerClasses = []peerClass{
833835
{initialRank: peerRankInitialFirstPriority, peerClass: network.PeersConnectedOut},
834836
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersConnectedIn},
835-
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersPhonebookRelays},
836-
{initialRank: peerRankInitialFourthPriority, peerClass: network.PeersPhonebookArchivers},
837+
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersPhonebookArchivalNodes},
838+
{initialRank: peerRankInitialFourthPriority, peerClass: network.PeersPhonebookRelays},
839+
{initialRank: peerRankInitialFifthPriority, peerClass: network.PeersPhonebookArchivers},
837840
}
838841
} else {
839842
peerClasses = []peerClass{
840843
{initialRank: peerRankInitialFirstPriority, peerClass: network.PeersConnectedOut},
841-
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersPhonebookRelays},
842-
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersPhonebookArchivers},
844+
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersPhonebookArchivalNodes},
845+
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersPhonebookRelays},
846+
{initialRank: peerRankInitialFourthPriority, peerClass: network.PeersPhonebookArchivers},
843847
}
844848
}
845849
}
@@ -848,26 +852,30 @@ func createPeerSelector(net network.GossipNode, cfg config.Local, pipelineFetch
848852
if cfg.NetAddress != "" { // Relay node
849853
peerClasses = []peerClass{
850854
{initialRank: peerRankInitialFirstPriority, peerClass: network.PeersConnectedOut},
851-
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersPhonebookRelays},
852-
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersConnectedIn},
855+
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersPhonebookArchivalNodes},
856+
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersPhonebookRelays},
857+
{initialRank: peerRankInitialFourthPriority, peerClass: network.PeersConnectedIn},
853858
}
854859
} else {
855860
peerClasses = []peerClass{
856-
{initialRank: peerRankInitialFirstPriority, peerClass: network.PeersConnectedOut},
857-
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersPhonebookRelays},
861+
{initialRank: peerRankInitialFirstPriority, peerClass: network.PeersPhonebookArchivalNodes},
862+
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersConnectedOut},
863+
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersPhonebookRelays},
858864
}
859865
}
860866
} else {
861867
if cfg.NetAddress != "" { // Relay node
862868
peerClasses = []peerClass{
863869
{initialRank: peerRankInitialFirstPriority, peerClass: network.PeersConnectedOut},
864870
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersConnectedIn},
865-
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersPhonebookRelays},
871+
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersPhonebookArchivalNodes},
872+
{initialRank: peerRankInitialFourthPriority, peerClass: network.PeersPhonebookRelays},
866873
}
867874
} else {
868875
peerClasses = []peerClass{
869876
{initialRank: peerRankInitialFirstPriority, peerClass: network.PeersConnectedOut},
870-
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersPhonebookRelays},
877+
{initialRank: peerRankInitialSecondPriority, peerClass: network.PeersPhonebookArchivalNodes},
878+
{initialRank: peerRankInitialThirdPriority, peerClass: network.PeersPhonebookRelays},
871879
}
872880
}
873881
}

0 commit comments

Comments
 (0)