Skip to content

Commit

Permalink
bitswap/client: fix PeerResponseTrackerProbabilityOneKnownOneUnknownPeer
Browse files Browse the repository at this point in the history
This was a bug in the implementation where it would incorrectly assume that 0 and 1 block received are equivalent scores.
I think this test used to not be flaky because prior to go1.20 `math/rand` would initialise to a zero state and the zero state would deterministically pass even tho this was 50/50 odds.

It also unmark all some tests in bitswap/client/internal/session non flaky since they are not flaky.
  • Loading branch information
Jorropo committed May 24, 2023
1 parent e468223 commit 1daddd8
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 28 deletions.
9 changes: 2 additions & 7 deletions bitswap/client/internal/session/peerresponsetracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,9 @@ func (prt *peerResponseTracker) choose(peers []peer.ID) peer.ID {
}

// getPeerCount returns the number of times the peer was first to send us a
// block
// block plus one (in order to never get a zero chance).
func (prt *peerResponseTracker) getPeerCount(p peer.ID) int {
count, ok := prt.firstResponder[p]
if ok {
return count
}

// Make sure there is always at least a small chance a new peer
// will be chosen
return 1
return prt.firstResponder[p] + 1
}
9 changes: 0 additions & 9 deletions bitswap/client/internal/session/peerresponsetracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,10 @@ import (
"testing"

"github.com/ipfs/boxo/bitswap/internal/testutil"
"github.com/ipfs/boxo/internal/test"
peer "github.com/libp2p/go-libp2p/core/peer"
)

func TestPeerResponseTrackerInit(t *testing.T) {
test.Flaky(t)

peers := testutil.GeneratePeers(2)
prt := newPeerResponseTracker()

Expand All @@ -28,8 +25,6 @@ func TestPeerResponseTrackerInit(t *testing.T) {
}

func TestPeerResponseTrackerProbabilityUnknownPeers(t *testing.T) {
test.Flaky(t)

peers := testutil.GeneratePeers(4)
prt := newPeerResponseTracker()

Expand Down Expand Up @@ -59,8 +54,6 @@ func TestPeerResponseTrackerProbabilityUnknownPeers(t *testing.T) {
}

func TestPeerResponseTrackerProbabilityOneKnownOneUnknownPeer(t *testing.T) {
test.Flaky(t)

peers := testutil.GeneratePeers(2)
prt := newPeerResponseTracker()

Expand All @@ -86,8 +79,6 @@ func TestPeerResponseTrackerProbabilityOneKnownOneUnknownPeer(t *testing.T) {
}

func TestPeerResponseTrackerProbabilityProportional(t *testing.T) {
test.Flaky(t)

peers := testutil.GeneratePeers(3)
prt := newPeerResponseTracker()

Expand Down
3 changes: 0 additions & 3 deletions bitswap/client/internal/session/sentwantblockstracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,9 @@ import (
"testing"

"github.com/ipfs/boxo/bitswap/internal/testutil"
"github.com/ipfs/boxo/internal/test"
)

func TestSendWantBlocksTracker(t *testing.T) {
test.Flaky(t)

peers := testutil.GeneratePeers(2)
cids := testutil.GenerateCids(2)
swbt := newSentWantBlocksTracker()
Expand Down
9 changes: 0 additions & 9 deletions bitswap/client/internal/session/wantinfo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,9 @@ import (
"testing"

"github.com/ipfs/boxo/bitswap/internal/testutil"
"github.com/ipfs/boxo/internal/test"
)

func TestEmptyWantInfo(t *testing.T) {
test.Flaky(t)

wp := newWantInfo(newPeerResponseTracker())

if wp.bestPeer != "" {
Expand All @@ -18,8 +15,6 @@ func TestEmptyWantInfo(t *testing.T) {
}

func TestSetPeerBlockPresence(t *testing.T) {
test.Flaky(t)

peers := testutil.GeneratePeers(2)
wp := newWantInfo(newPeerResponseTracker())

Expand All @@ -40,8 +35,6 @@ func TestSetPeerBlockPresence(t *testing.T) {
}

func TestSetPeerBlockPresenceBestLower(t *testing.T) {
test.Flaky(t)

peers := testutil.GeneratePeers(2)
wp := newWantInfo(newPeerResponseTracker())

Expand All @@ -62,8 +55,6 @@ func TestSetPeerBlockPresenceBestLower(t *testing.T) {
}

func TestRemoveThenSetDontHave(t *testing.T) {
test.Flaky(t)

peers := testutil.GeneratePeers(2)
wp := newWantInfo(newPeerResponseTracker())

Expand Down

0 comments on commit 1daddd8

Please sign in to comment.