Skip to content

When the leader request is aborted, every waiting follower fetches the same object from upstream separately (N+1 fetches instead of one) #58

Description

@matt-edmondson

What's wrong

Single-flight coalescing doesn't survive the leader going away:

  • Fetching/SingleFlight.cs (Ticket.Dispose): a leader that never reported an outcome completes its followers with false.
  • Endpoints/ObjectRouteHandler.cs (around lines 163–201): a follower whose WaitForLeaderAsync returns false logs LeaderDidNotFinish and then calls StreamFromUpstreamAsync(..., storeLocally: true, ...) itself. It never calls coalescer.Acquire again.

The leader's upstream fetch runs on its own client's RequestAborted token. So when that one client disconnects mid-transfer (a killed CI pod, a client timeout, Ctrl-C), all waiting followers start their own upstream fetch of the same object at the same moment. They also race to stage and publish it.

This is the thundering herd the README's "one upstream fetch per object" promise is meant to prevent. It happens exactly when many CI jobs start together and pull the same large object.

Reproduction

Test with a gated upstream, 1 leader and 5 followers:

  1. Queue the 5 followers behind the leader.
  2. Cancel the leader's request.
  3. Release the upstream gate.

Result: all 5 followers returned 200, but upstream saw 6 object fetches (1 before the abort, 5 after). The expected count is 2 at most: the aborted one plus a single replacement.

Suggested fix

Either of these, or both:

  • A follower released with false, including after a FollowerTimeout, calls coalescer.Acquire(upstream, oid) again. One follower becomes the new leader and the rest keep waiting, with a bound on how many times this can repeat.
  • Run the leader's upstream fetch-and-store on a token that isn't tied to the leader's client. The object is then still fetched and published for the followers when the leader disconnects; only the leader's copy to its own response stops.

Acceptance: a test with N followers and an aborted leader sees at most one additional upstream fetch.

Activity

  1. matt-edmondson commented on Sep 28, 2026

    @matt-edmondson
    ContributorAuthor

    Triage

    • Category: Bug
    • Priority: Medium. When one client disconnects mid-transfer, every waiting follower fetches the same object from upstream separately (6 fetches instead of 2 in the repro). That is the thundering herd the README promises to prevent, and it happens precisely when many CI jobs pull the same large object together. Clients still get their bytes; the cost is upstream bandwidth and rate limit.
    • Area / suggested assignment: Fetching, in GitLfsCache/Fetching/SingleFlight.cs (Ticket.Dispose) and GitLfsCache/Endpoints/ObjectRouteHandler.cs (lines ~163-201)
    • Duplicates: none found among open issues in the org
    • In progress: no direct match. Open PR Refuse to publish a staging file whose write failed #59 (refuse to publish a failed staging write) touches the same download/publish path, so expect to rebase.

    Notes: Of the two options, decoupling the leader's fetch-and-store from its client's RequestAborted addresses the root cause. Re-acquiring on false is a good bounded backstop for FollowerTimeout. Doing both is reasonable.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

readyFully specified; implement as written

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions