Correctly clean up progress bars for completed transfers

[brianaydemir]

Summary

A pelican object sync or pelican object get --recursive involving hundreds of thousands of objects will no longer degrade in performance, as in transfer fewer and fewer objects per unit time, when progress bars are enabled (which is the default).

As a consequence, users might notice that progress bars are no longer displayed at all for transfers that complete under ~0.5 seconds.

Note

There is the potential for a pathological scenario where no progress bars are displayed because all transfers are starting and completing too quickly. The solution for that is to implement an overall status indicator — for example, number of objects transferred out of total number to be transferred — which is out of the scope for this PR.

Details

The linked issue contains a test bench that can be used to verify the fix in this PR, as well as the raw data for the graph below.

The linked issue also contains evidence implicating that the client's handling of progress bars is at fault.

The primary loop is over pb.status:

pelican/cmd/progress_bars.go

Lines 112 to 114 in 088217e

	for path := range pb.status {
	if pbMap[path] == nil {
	pbMap[path] = &progressBar{

This map is populated via a callback from the transfer jobs, and keyed by the paths of the objects being transferred.

The fundamental problem is we never remove keys from pb.status, so long after the transfer for a path has completed and the progress bar has been removed from the display, we're still updating it:

pelican/cmd/progress_bars.go

Lines 135 to 136 in 088217e

	pbMap[path].bar.EwmaSetCurrent(newStatus.xfer, tickDuration)
	pbMap[path].progressStatus = newStatus

The whole business around setting pbMap[path].xfer = -1 should be clearing out progress bars for completed transfers. We need only delete keys from pb.status as appropriate, so that the logic there actually plays out as intended.

And indeed, once we do so, we get a more reasonable behavior:

---

---

I would post screenshots of the relevant visualizations from pprof, but I can't find anything significant related to the progress bars anymore, which would suggest that it's below the threshold for caring about.

[brianaydemir]

@patrickbrophy You get this one because you mentioned some tool might have observed a problem with our progress bars.

[brianaydemir]

In the graph above, I believe the extreme downward spikes are correlated with when the client needed to perform another listing (PROPFIND) operation to determine the next chunk of objects to download — and I suppose going through 8192 objects isn't exactly quick.

[patrickbrophy]

The fix does directly address the cause, however the semantics on the RWLock are now incorrect. Very minor change requested, otherwise LGTM

[brianaydemir]

I rebased this on main because I was, locally, I was running into issues with Go 1.24 vs. 1.25.

[patrickbrophy]

LGTM