refactor: session peer manager ordering

sessionpeermanager/sessionpeermanager.go

@@ -277,30 +277,42 @@ type getPeersMessage struct {
 	resp chan<- []bssd.OptimizedPeer
 }
 
+// Get all optimized peers in order followed by randomly ordered unoptimized
+// peers, with a limit of maxOptimizedPeers
 func (prm *getPeersMessage) handle(spm *SessionPeerManager) {
-	randomOrder := rand.Perm(len(spm.unoptimizedPeersArr))
+	// Number of peers to get in total: unoptimized + optimized
+	// limited by maxOptimizedPeers
 	maxPeers := len(spm.unoptimizedPeersArr) + len(spm.optimizedPeersArr)
 	if maxPeers > maxOptimizedPeers {
 		maxPeers = maxOptimizedPeers
 	}
+
+	// The best peer latency is 1 if we have recorded at least one peer's
+	// latency, 0 otherwise
 	var bestPeerLatency float64
 	if len(spm.optimizedPeersArr) > 0 {
 		bestPeerLatency = float64(spm.activePeers[spm.optimizedPeersArr[0]].latency)
 	} else {
 		bestPeerLatency = 0
 	}
+
 	optimizedPeers := make([]bssd.OptimizedPeer, 0, maxPeers)
-	for i := 0; i < maxPeers; i++ {
-		if i < len(spm.optimizedPeersArr) {
+
+	// Add optimized peers in order
+	for i := 0; i < maxPeers && i < len(spm.optimizedPeersArr); i++ {
 		p := spm.optimizedPeersArr[i]
 		optimizedPeers = append(optimizedPeers, bssd.OptimizedPeer{
 			Peer:               p,
 			OptimizationRating: bestPeerLatency / float64(spm.activePeers[p].latency),
 		})
-		} else {
-			p := spm.unoptimizedPeersArr[randomOrder[i-len(spm.optimizedPeersArr)]]
-			optimizedPeers = append(optimizedPeers, bssd.OptimizedPeer{Peer: p, OptimizationRating: 0.0})
 	}
+
+	// Add unoptimized peers in random order
+	randomOrder := rand.Perm(len(spm.unoptimizedPeersArr))
+	remaining := maxPeers - len(optimizedPeers)
+	for i := 0; i < remaining; i++ {
+		p := spm.unoptimizedPeersArr[randomOrder[i]]
+		optimizedPeers = append(optimizedPeers, bssd.OptimizedPeer{Peer: p, OptimizationRating: 0.0})
 	}
 	prm.resp <- optimizedPeers
 }

sessionpeermanager/sessionpeermanager_test.go

@@ -2,6 +2,7 @@ package sessionpeermanager
 
 import (
 	"context"
+	"fmt"
 	"math/rand"
 	"sync"
 	"testing"
@@ -185,10 +186,13 @@ func TestOrderingPeers(t *testing.T) {
 
 	sessionPeers := sessionPeerManager.GetOptimizedPeers()
 	if len(sessionPeers) != maxOptimizedPeers {
-		t.Fatal("Should not return more than the max of optimized peers")
+		t.Fatal(fmt.Sprintf("Should not return more (%d) than the max of optimized peers (%d)", len(sessionPeers), maxOptimizedPeers))
 	}
 
 	// should prioritize peers which are fastest
+	// peer1: ~10ms
+	// peer2: 10 + 50 = ~60ms
+	// peer3: 10 + 50 + 10 = ~70ms
 	if (sessionPeers[0].Peer != peer1) || (sessionPeers[1].Peer != peer2) || (sessionPeers[2].Peer != peer3) {
 		t.Fatal("Did not prioritize peers that received blocks")
 	}
@@ -204,7 +208,7 @@ func TestOrderingPeers(t *testing.T) {
 		t.Fatal("Did not assign rating to other optimized peers correctly")
 	}
 
-	// should other peers rating of zero
+	// should give other non-optimized peers rating of zero
 	for i := 3; i < maxOptimizedPeers; i++ {
 		if sessionPeers[i].OptimizationRating != 0.0 {
 			t.Fatal("Did not assign rating to unoptimized peer correctly")
@@ -222,10 +226,13 @@ func TestOrderingPeers(t *testing.T) {
 	// call again
 	nextSessionPeers := sessionPeerManager.GetOptimizedPeers()
 	if len(nextSessionPeers) != maxOptimizedPeers {
-		t.Fatal("Should not return more than the max of optimized peers")
+		t.Fatal(fmt.Sprintf("Should not return more (%d) than the max of optimized peers (%d)", len(nextSessionPeers), maxOptimizedPeers))
 	}
 
 	// should sort by average latency
+	// peer1: ~10ms
+	// peer3: (~70ms + ~0ms) / 2 = ~35ms
+	// peer2: ~60ms
 	if (nextSessionPeers[0].Peer != peer1) || (nextSessionPeers[1].Peer != peer3) ||
 		(nextSessionPeers[2].Peer != peer2) {
 		t.Fatal("Did not correctly update order of peers sorted by average latency")