accounting: allow MaxCompletedTransfers to be configurable

rclone library users might be intrested in changing default value to
other, or even disabling it. With current version it's impossible which
leads to races when number of uploaded objects exceeds default limit.

Fixes #3732

fs/accounting/stats.go

@@ -13,8 +13,8 @@ import (
 	"github.com/rclone/rclone/fs/rc"
 )
 
-// Maximum number of completed transfers in startedTransfers list
-const maxCompletedTransfers = 100
+// MaxCompletedTransfers specifies maximum number of completed transfers in startedTransfers list
+var MaxCompletedTransfers = 100
 
 // StatsInfo accounts all transfers
 type StatsInfo struct {
@@ -627,11 +627,15 @@ func (s *StatsInfo) RemoveTransfer(transfer *Transfer) {
 	s.mu.Unlock()
 }
 
-// PruneTransfers makes sure there aren't too many old transfers
+// PruneTransfers makes sure there aren't too many old transfers by removing
+// single finished transfer.
 func (s *StatsInfo) PruneTransfers() {
+	if MaxCompletedTransfers < 0 {
+		return
+	}
 	s.mu.Lock()
 	// remove a transfer from the start if we are over quota
-	if len(s.startedTransfers) > maxCompletedTransfers+fs.Config.Transfers {
+	if len(s.startedTransfers) > MaxCompletedTransfers+fs.Config.Transfers {
 		for i, tr := range s.startedTransfers {
 			if tr.IsDone() {
 				s.removeTransfer(tr, i)

fs/accounting/stats_test.go

@@ -382,28 +382,52 @@ func TestTimeRangeDuration(t *testing.T) {
 }
 
 func TestPruneTransfers(t *testing.T) {
-	max := maxCompletedTransfers + fs.Config.Transfers
+	for _, test := range []struct {
+		Name                     string
+		Transfers                int
+		Limit                    int
+		ExpectedStartedTransfers int
+	}{
+		{
+			Name:                     "Limited number of StartedTransfers",
+			Limit:                    100,
+			Transfers:                200,
+			ExpectedStartedTransfers: 100 + fs.Config.Transfers,
+		},
+		{
+			Name:                     "Unlimited number of StartedTransfers",
+			Limit:                    -1,
+			Transfers:                200,
+			ExpectedStartedTransfers: 200,
+		},
+	} {
+		t.Run(test.Name, func(t *testing.T) {
+			prevLimit := MaxCompletedTransfers
+			MaxCompletedTransfers = test.Limit
+			defer func() { MaxCompletedTransfers = prevLimit }()
+
+			s := NewStats()
+			for i := int64(1); i <= int64(test.Transfers); i++ {
+				s.AddTransfer(&Transfer{
+					startedAt:   time.Unix(i, 0),
+					completedAt: time.Unix(i+1, 0),
+				})
+			}
+
+			s.mu.Lock()
+			assert.Equal(t, time.Duration(test.Transfers)*time.Second, s.totalDuration())
+			assert.Equal(t, test.Transfers, len(s.startedTransfers))
+			s.mu.Unlock()
+
+			for i := 0; i < test.Transfers; i++ {
+				s.PruneTransfers()
+			}
+
+			s.mu.Lock()
+			assert.Equal(t, time.Duration(test.Transfers)*time.Second, s.totalDuration())
+			assert.Equal(t, test.ExpectedStartedTransfers, len(s.startedTransfers))
+			s.mu.Unlock()
 
-	s := NewStats()
-	for i := int64(1); i <= int64(max+100); i++ {
-		s.AddTransfer(&Transfer{
-			startedAt:   time.Unix(i, 0),
-			completedAt: time.Unix(i+1, 0),
 		})
 	}
-
-	s.mu.Lock()
-	assert.Equal(t, time.Duration(max+100)*time.Second, s.totalDuration())
-	assert.Equal(t, max+100, len(s.startedTransfers))
-	s.mu.Unlock()
-
-	for i := 0; i < 200; i++ {
-		s.PruneTransfers()
-	}
-
-	s.mu.Lock()
-	assert.Equal(t, time.Duration(max+100)*time.Second, s.totalDuration())
-	assert.Equal(t, max, len(s.startedTransfers))
-	s.mu.Unlock()
-
 }