diff --git a/fs/accounting/stats.go b/fs/accounting/stats.go index 4c44555f6..e41273ee8 100644 --- a/fs/accounting/stats.go +++ b/fs/accounting/stats.go @@ -13,6 +13,9 @@ import ( "github.com/rclone/rclone/fs/rc" ) +// Maximum number of completed transfers in startedTransfers list +const maxCompletedTransfers = 100 + // StatsInfo accounts all transfers type StatsInfo struct { mu sync.RWMutex @@ -560,10 +563,11 @@ func (s *StatsInfo) AddTransfer(transfer *Transfer) { s.mu.Unlock() } -// RemoveTransfer removes a reference to the started transfer. -func (s *StatsInfo) RemoveTransfer(transfer *Transfer) { - s.mu.Lock() - +// removeTransfer removes a reference to the started transfer in +// position i. +// +// Must be called with the lock held +func (s *StatsInfo) removeTransfer(transfer *Transfer, i int) { // add finished transfer onto old time ranges start, end := transfer.TimeRange() if end.IsZero() { @@ -572,12 +576,33 @@ func (s *StatsInfo) RemoveTransfer(transfer *Transfer) { s.oldTimeRanges = append(s.oldTimeRanges, timeRange{start, end}) s.oldTimeRanges.merge() + // remove the found entry + s.startedTransfers = append(s.startedTransfers[:i], s.startedTransfers[i+1:]...) +} + +// RemoveTransfer removes a reference to the started transfer. +func (s *StatsInfo) RemoveTransfer(transfer *Transfer) { + s.mu.Lock() for i, tr := range s.startedTransfers { if tr == transfer { - // remove the found entry - s.startedTransfers = append(s.startedTransfers[:i], s.startedTransfers[i+1:]...) + s.removeTransfer(tr, i) break } } s.mu.Unlock() } + +// PruneTransfers makes sure there aren't too many old transfers +func (s *StatsInfo) PruneTransfers() { + s.mu.Lock() + // remove a transfer from the start if we are over quota + if len(s.startedTransfers) > maxCompletedTransfers+fs.Config.Transfers { + for i, tr := range s.startedTransfers { + if tr.IsDone() { + s.removeTransfer(tr, i) + break + } + } + } + s.mu.Unlock() +} diff --git a/fs/accounting/stats_groups.go b/fs/accounting/stats_groups.go index 417f29511..dc77769ac 100644 --- a/fs/accounting/stats_groups.go +++ b/fs/accounting/stats_groups.go @@ -137,6 +137,8 @@ This returns stats about completed transfers: If group is not provided then completed transfers for all groups will be returned. +Note only the last 100 completed transfers are returned. + Parameters - group - name of the stats group (string) diff --git a/fs/accounting/stats_test.go b/fs/accounting/stats_test.go index 7afc78b46..5ae5ad956 100644 --- a/fs/accounting/stats_test.go +++ b/fs/accounting/stats_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/pkg/errors" + "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/fserrors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -306,3 +307,30 @@ func TestTimeRangeDuration(t *testing.T) { assert.Equal(t, 1*time.Second, makeTimeRanges(t, []string{"1-2"}).total()) assert.Equal(t, 91*time.Second, makeTimeRanges(t, []string{"1-2", "10-100"}).total()) } + +func TestPruneTransfers(t *testing.T) { + max := maxCompletedTransfers + fs.Config.Transfers + + 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() + +} diff --git a/fs/accounting/transfer.go b/fs/accounting/transfer.go index 41a5536b3..9fc7f8f04 100644 --- a/fs/accounting/transfer.go +++ b/fs/accounting/transfer.go @@ -102,6 +102,8 @@ func (tr *Transfer) Done(err error) { } // Signal done with accounting acc.Done() + // free the account since we may keep the transfer + acc = nil } tr.mu.Lock() @@ -113,7 +115,7 @@ func (tr *Transfer) Done(err error) { } else { tr.stats.DoneTransferring(tr.remote, err == nil) } - tr.stats.RemoveTransfer(tr) + tr.stats.PruneTransfers() } // Reset allows to switch the Account to another transfer method.