bitswap/httpnet: error_tracker: use normal mutex-locked map.

Author: hsanjuan

bitswap/network/httpnet/error_tracker.go

@@ -3,7 +3,6 @@ package httpnet
 import (
 	"errors"
 	"sync"
-	"sync/atomic"
 
 	"github.com/libp2p/go-libp2p/core/peer"
 )
@@ -14,50 +13,34 @@ type errorTracker struct {
 	ht *Network
 
 	mux    sync.RWMutex
-	errors map[peer.ID]*atomic.Uint64
+	errors map[peer.ID]int
 }
 
 func newErrorTracker(ht *Network) *errorTracker {
 	return &errorTracker{
 		ht:     ht,
-		errors: make(map[peer.ID]*atomic.Uint64),
-	}
-}
-
-func (et *errorTracker) startTracking(p peer.ID) {
-	et.mux.Lock()
-	defer et.mux.Unlock()
-
-	// Initialize the error count for the peer if it doesn't exist
-	if _, exists := et.errors[p]; !exists {
-		et.errors[p] = &atomic.Uint64{}
+		errors: make(map[peer.ID]int),
 	}
 }
 
 func (et *errorTracker) stopTracking(p peer.ID) {
 	et.mux.Lock()
-	defer et.mux.Unlock()
 	delete(et.errors, p)
+	et.mux.Unlock()
 }
 
 // logErrors adds n to the current error count for p. If the total count is above the threshold, then an error is returned. If n is 0, the the total count is reset to 0.
-func (et *errorTracker) logErrors(p peer.ID, n uint64, threshold uint64) error {
-	et.mux.RLock()
-	count, ok := et.errors[p]
-	et.mux.RUnlock()
-
-	if !ok {
-		// i.e. we disconnected but there were pending requests
-		log.Debug("logging errors for untracked peer: %s", p)
-		return nil
-	}
+func (et *errorTracker) logErrors(p peer.ID, n int, threshold int) error {
+	et.mux.Lock()
+	defer et.mux.Unlock()
 
 	if n == 0 { // reset error count
-		count.Store(0)
+		et.errors[p] = 0
 		return nil
 	}
-
-	total := count.Add(n)
+	count, _ := et.errors[p]
+	total := count + n
+	et.errors[p] = total
 	if total > threshold {
 		return errThresholdCrossed
 	}

bitswap/network/httpnet/error_tracker_test.go

@@ -8,30 +8,10 @@ import (
 	"github.com/libp2p/go-libp2p/core/peer"
 )
 
-func TestErrorTracker_StartTracking(t *testing.T) {
-	et := newErrorTracker(&Network{})
-	p := peer.ID("testpeer")
-
-	// Start tracking the peer
-	et.startTracking(p)
-
-	// Check if the error count is initialized to 0
-	count, ok := et.errors[p]
-	if !ok {
-		t.Errorf("Expected peer %s to be tracked but it was not", p)
-	}
-	if count.Load() != 0 {
-		t.Errorf("Expected initial error count for peer %s to be 0 but got %d", p, count.Load())
-	}
-}
-
 func TestErrorTracker_StopTracking(t *testing.T) {
 	et := newErrorTracker(&Network{})
 	p := peer.ID("testpeer")
 
-	// Start tracking the peer
-	et.startTracking(p)
-
 	// Stop tracking the peer
 	et.stopTracking(p)
 
@@ -45,9 +25,6 @@ func TestErrorTracker_LogErrors_Reset(t *testing.T) {
 	et := newErrorTracker(&Network{})
 	p := peer.ID("testpeer")
 
-	// Start tracking the peer
-	et.startTracking(p)
-
 	// Log some errors
 	err := et.logErrors(p, 5, 10)
 	if err != nil {
@@ -65,18 +42,15 @@ func TestErrorTracker_LogErrors_Reset(t *testing.T) {
 	if !ok {
 		t.Errorf("Expected peer %s to be tracked but it was not", p)
 	}
-	if count.Load() != 0 {
-		t.Errorf("Expected error count for peer %s to be 0 after reset but got %d", p, count.Load())
+	if count != 0 {
+		t.Errorf("Expected error count for peer %s to be 0 after reset but got %d", p, count)
 	}
 }
 
 func TestErrorTracker_LogErrors_ThresholdCrossed(t *testing.T) {
 	et := newErrorTracker(&Network{})
 	p := peer.ID("testpeer")
 
-	// Start tracking the peer
-	et.startTracking(p)
-
 	// Log errors until threshold is crossed
 	err := et.logErrors(p, 11, 10)
 	if err != errThresholdCrossed {
@@ -88,19 +62,8 @@ func TestErrorTracker_LogErrors_ThresholdCrossed(t *testing.T) {
 	if !ok {
 		t.Errorf("Expected peer %s to be tracked but it was not", p)
 	}
-	if count.Load() != 11 {
-		t.Errorf("Expected error count for peer %s to be 10 after logging errors above threshold but got %d", p, count.Load())
-	}
-}
-
-func TestErrorTracker_LogErrors_UntrackedPeer(t *testing.T) {
-	et := newErrorTracker(&Network{})
-	p := peer.ID("testpeer")
-
-	// Log errors for an untracked peer
-	err := et.logErrors(p, 5, 10)
-	if err != nil {
-		t.Errorf("Expected no error when logging errors for an untracked peer but got %v", err)
+	if count != 11 {
+		t.Errorf("Expected error count for peer %s to be 10 after logging errors above threshold but got %d", p, count)
 	}
 }
 
@@ -109,12 +72,9 @@ func TestErrorTracker_ConcurrentAccess(t *testing.T) {
 	et := newErrorTracker(&Network{})
 	p := peer.ID("testpeer")
 
-	// Start tracking the peer
-	et.startTracking(p)
-
 	var wg sync.WaitGroup
 	numRoutines := 10
-	threshold := uint64(100)
+	threshold := 100
 
 	for i := 0; i < numRoutines; i++ {
 		wg.Add(1)
@@ -134,7 +94,7 @@ func TestErrorTracker_ConcurrentAccess(t *testing.T) {
 		t.Errorf("Expected peer %s to be tracked but it was not", p)
 	}
 	expectedCount := threshold
-	actualCount := count.Load()
+	actualCount := count
 	if actualCount != expectedCount {
 		t.Errorf("Expected error count for peer %s to be %d after concurrent logging but got %d", p, expectedCount, actualCount)
 	}

bitswap/network/httpnet/httpnet.go

@@ -163,7 +163,7 @@ func WithHTTPWorkers(n int) Option {
 // be that pending requests will still happen. The HTTP connection might be
 // kept until it times-out per the IdleConnTimeout. Requests will resume if a
 // provider record is found causing us to "reconnect" to the peer.
-func WithMaxDontHaveErrors(threshold uint64) Option {
+func WithMaxDontHaveErrors(threshold int) Option {
 	return func(net *Network) {
 		net.maxDontHaveErrors = threshold
 	}
@@ -195,7 +195,7 @@ type Network struct {
 	maxIdleConns            int
 	insecureSkipVerify      bool
 	maxHTTPAddressesPerPeer int
-	maxDontHaveErrors       uint64
+	maxDontHaveErrors       int
 	httpWorkers             int
 	allowlist               map[string]struct{}
 	denylist                map[string]struct{}
@@ -484,7 +484,6 @@ func (ht *Network) Connect(ctx context.Context, pi peer.AddrInfo) error {
 
 	ht.connEvtMgr.Connected(p)
 	ht.pinger.startPinging(p)
-	ht.errorTracker.startTracking(p)
 	log.Debugf("connect success to %s (supports HEAD: %t)", p, supportsHead)
 	// We "connected"
 	return nil

bitswap/network/httpnet/msg_sender.go

@@ -502,7 +502,7 @@ WANTLIST_LOOP:
 	go func() {
 		bsresp := bsmsg.New(false)
 		totalResponses := 0
-		totalClientErrors := uint64(0)
+		totalClientErrors := 0
 
 		for result := range resultsCollector {
 			// Record total request time.