diff --git a/bucket.go b/bucket.go index 2b74b73..22b8e8e 100644 --- a/bucket.go +++ b/bucket.go @@ -43,7 +43,7 @@ func newBucket() *bucket { // returns all peers in the bucket // it is safe for the caller to modify the returned objects as it is a defensive copy func (b *bucket) peers() []PeerInfo { - var ps []PeerInfo + ps := make([]PeerInfo, 0, b.len()) for e := b.list.Front(); e != nil; e = e.Next() { p := e.Value.(*PeerInfo) ps = append(ps, *p) @@ -51,6 +51,28 @@ func (b *bucket) peers() []PeerInfo { return ps } +// 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) 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 lessThan(val, minVal) { + minVal = val + } + } + + return minVal +} + // return the Ids of all the peers in the bucket. func (b *bucket) peerIds() []peer.ID { ps := make([]peer.ID, 0, b.list.Len()) diff --git a/bucket_test.go b/bucket_test.go new file mode 100644 index 0000000..e0f6208 --- /dev/null +++ b/bucket_test.go @@ -0,0 +1,39 @@ +package kbucket + +import ( + "testing" + "time" + + "github.com/libp2p/go-libp2p-core/test" + + "github.com/stretchr/testify/require" +) + +func TestBucketMinimum(t *testing.T) { + t.Parallel() + + b := newBucket() + 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 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) + + // 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) + + // 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) +} diff --git a/table.go b/table.go index 93d526a..356ceee 100644 --- a/table.go +++ b/table.go @@ -152,16 +152,17 @@ func (rt *RoutingTable) addPeer(p peer.ID, queryPeer bool) (bool, error) { // the bucket to which the peer belongs is full. Let's try to find a peer // in that bucket with a LastSuccessfulOutboundQuery value above the maximum threshold and replace it. - allPeers := bucket.peers() - for _, pc := range allPeers { - if time.Since(pc.LastUsefulAt) > rt.usefulnessGracePeriod { - // let's evict it and add the new peer - if bucket.remove(pc.Id) { - bucket.pushFront(&PeerInfo{Id: p, LastUsefulAt: lastUsefulAt, LastSuccessfulOutboundQueryAt: time.Now(), - dhtId: ConvertPeerID(p)}) - rt.PeerAdded(p) - return true, nil - } + minLast := bucket.min(func(first *PeerInfo, second *PeerInfo) bool { + return first.LastUsefulAt.Before(second.LastUsefulAt) + }) + + if time.Since(minLast.LastUsefulAt) > rt.usefulnessGracePeriod { + // let's evict it and add the new peer + if bucket.remove(minLast.Id) { + bucket.pushFront(&PeerInfo{Id: p, LastUsefulAt: lastUsefulAt, LastSuccessfulOutboundQueryAt: time.Now(), + dhtId: ConvertPeerID(p)}) + rt.PeerAdded(p) + return true, nil } } diff --git a/table_test.go b/table_test.go index 3e6f8b4..13087f4 100644 --- a/table_test.go +++ b/table_test.go @@ -78,6 +78,7 @@ func TestBucket(t *testing.T) { t.Fatalf("split failed. found id with cpl == 0 in non 0 bucket") } } + } func TestRemovePeer(t *testing.T) {