Reduce Routing Table churn

bucket.go

@@ -57,18 +57,26 @@ func (b *bucket) peers() []PeerInfo {
 	return ps
 }
 
-// findFirst returns the first peer in the bucket that satisfies the given predicate.
-// It is NOT safe for the predicate to mutate the given `PeerInfo` as we pass in a pointer to it.
+// returns the "minimum" peer in the bucket based on the `lessThan` comparator passed to it.
+// It is NOT safe for the comparator to mutate the given `PeerInfo`
+// as we pass in a pointer to it.
 // It is NOT safe to modify the returned value.
-func (b *bucket) findFirst(p func(p *PeerInfo) bool) *PeerInfo {
-	for e := b.list.Front(); e != nil; e = e.Next() {
+func (b *bucket) min(lessThan func(p1 *PeerInfo, p2 *PeerInfo) bool) *PeerInfo {
+	if b.list.Len() == 0 {
+		return nil
+	}
+
+	minVal := b.list.Front().Value.(*PeerInfo)
+
+	for e := b.list.Front().Next(); e != nil; e = e.Next() {
 		val := e.Value.(*PeerInfo)
-		if p(val) {
-			return val
+
+		if lessThan(val, minVal) {
+			minVal = val
 		}
 	}
 
-	return nil
+	return minVal
 }
 
 // updateAllWith updates all the peers in the bucket by applying the given update function.

bucket_test.go

@@ -1,37 +1,40 @@
 package kbucket
 
 import (
+	"testing"
+	"time"
+
 	"github.com/libp2p/go-libp2p-core/test"
+
 	"github.com/stretchr/testify/require"
-	"testing"
 )
 
-func TestBucketFindFirst(t *testing.T) {
+func TestBucketMinimum(t *testing.T) {
 	t.Parallel()
 
 	b := newBucket()
-	require.Nil(t, b.findFirst(func(p1 *PeerInfo) bool { return true }))
+	require.Nil(t, b.min(func(p1 *PeerInfo, p2 *PeerInfo) bool { return true }))
 
 	pid1 := test.RandPeerIDFatal(t)
 	pid2 := test.RandPeerIDFatal(t)
 	pid3 := test.RandPeerIDFatal(t)
 
-	// first is replacable
-	b.pushFront(&PeerInfo{Id: pid1, replaceable: true})
-	require.Equal(t, pid1, b.findFirst(func(p *PeerInfo) bool {
-		return p.replaceable
+	// first is min
+	b.pushFront(&PeerInfo{Id: pid1, LastUsefulAt: time.Now()})
+	require.Equal(t, pid1, b.min(func(first *PeerInfo, second *PeerInfo) bool {
+		return first.LastUsefulAt.Before(second.LastUsefulAt)
 	}).Id)
 
-	// above peer is stll the replacable one
-	b.pushFront(&PeerInfo{Id: pid2, replaceable: false})
-	require.Equal(t, pid1, b.findFirst(func(p *PeerInfo) bool {
-		return p.replaceable
+	// first is till min
+	b.pushFront(&PeerInfo{Id: pid2, LastUsefulAt: time.Now().AddDate(1, 0, 0)})
+	require.Equal(t, pid1, b.min(func(first *PeerInfo, second *PeerInfo) bool {
+		return first.LastUsefulAt.Before(second.LastUsefulAt)
 	}).Id)
 
-	// new peer is replacable.
-	b.pushFront(&PeerInfo{Id: pid3, replaceable: true})
-	require.Equal(t, pid3, b.findFirst(func(p *PeerInfo) bool {
-		return p.replaceable
+	// second is the min
+	b.pushFront(&PeerInfo{Id: pid3, LastUsefulAt: time.Now().AddDate(-1, 0, 0)})
+	require.Equal(t, pid3, b.min(func(first *PeerInfo, second *PeerInfo) bool {
+		return first.LastUsefulAt.Before(second.LastUsefulAt)
 	}).Id)
 }
 

table.go

@@ -213,11 +213,11 @@ func (rt *RoutingTable) addPeer(p peer.ID, queryPeer bool, isReplaceable bool) (
 
 	// the bucket to which the peer belongs is full. Let's try to find a peer
 	// in that bucket which is replaceable.
-	replaceablePeer := bucket.findFirst(func(p *PeerInfo) bool {
-		return p.replaceable
+	replaceablePeer := bucket.min(func(p1 *PeerInfo, p2 *PeerInfo) bool {
+		return p1.replaceable
 	})
 
-	if replaceablePeer != nil {
+	if replaceablePeer != nil && replaceablePeer.replaceable {
 		// let's evict it and add the new peer
 		if rt.removePeer(replaceablePeer.Id) {
 			bucket.pushFront(&PeerInfo{