What's wrong
When a locks/batch unlock target has a path but no id, LockFanOut resolves the id per item (GitLfsCache/Locks/LockFanOut.cs:134-146). On a miss it calls RefreshForResolutionAsync (LockFanOut.cs:261-275). That method calls refresher.RefreshAsync directly, which walks every page of the upstream lock listing. The call goes through neither the LockListService single-flight nor the IUpstreamLimiter.
Two things make misses common:
- Any successful fan-out or relayed lock change invalidates the snapshot (
LockFanOut.cs:87-90).
LockSnapshotStore.Invalidate removes the snapshot entirely (LockSnapshotStore.cs:36-40).
Failure scenarios
- A client locks 500 files via
locks/batch, which invalidates the snapshot, then unlocks the same 500 by path. Every item in the first wave, up to MaxFanOutConcurrency (default 8), finds no snapshot, and each one concurrently walks the full listing.
- An unlock-by-path request for 1000 paths, 900 of which hold no lock. Each of the 900 misses triggers another full cursor walk, because resolution still fails after the refresh. These walks bypass the throttle a GitHub 403/429 with
Retry-After is meant to impose, so they can get the token rate-limited.
Suggested fix
- Resolve ids once per request, before fanning out.
- Collect every target that has only a path. If any fails to resolve against the current snapshot, do one refresh. Route it through the same single-flight key the list service uses (
key.ToFlightKey()) and through the upstream limiter.
- Resolve all paths from that one snapshot. Paths still unresolved fail with 404 without any further refresh.
Acceptance criteria
- With no snapshot, a 50-path unlock-by-path batch against a stub upstream that counts
GET locks makes exactly one listing walk.
- A batch in which every path is unlocked makes at most one walk.
- The refresh respects
IUpstreamLimiter.
What's wrong
When a
locks/batchunlock target has a path but no id,LockFanOutresolves the id per item (GitLfsCache/Locks/LockFanOut.cs:134-146). On a miss it callsRefreshForResolutionAsync(LockFanOut.cs:261-275). That method callsrefresher.RefreshAsyncdirectly, which walks every page of the upstream lock listing. The call goes through neither theLockListServicesingle-flight nor theIUpstreamLimiter.Two things make misses common:
LockFanOut.cs:87-90).LockSnapshotStore.Invalidateremoves the snapshot entirely (LockSnapshotStore.cs:36-40).Failure scenarios
locks/batch, which invalidates the snapshot, then unlocks the same 500 by path. Every item in the first wave, up toMaxFanOutConcurrency(default 8), finds no snapshot, and each one concurrently walks the full listing.Retry-Afteris meant to impose, so they can get the token rate-limited.Suggested fix
key.ToFlightKey()) and through the upstream limiter.Acceptance criteria
GET locksmakes exactly one listing walk.IUpstreamLimiter.