[simonchrz]

Closes #596.

On a cache miss the proxy stored the whole artifact before sending any of it. Clients with an idle timeout gave up on large artifacts and retried until the detached fetch had finished.

The caller that starts the shared fetch now gets the bytes as they arrive, while they are stored (storeWhileServing in internal/handler/stream_miss.go):

• The upstream body is copied to storage and to the client at once, at the upstream's speed. The client is fed through a buffer of up to 8 MB; once it falls further behind, it stops being fed and continues from the stored copy where it left off. So a slow client can't hold back the store, the callers waiting on the fetch, or artifact_timeout.
• The client's copy is checked against the upstream's digest as it is read, the same way cache_artifacts: false checks it, so a mismatch aborts the response before it completes and nothing is cached.
• If the client goes away, the copy to storage keeps running.
• If storage fails, the copy to the client keeps running. The client gets the whole artifact, and the failure is logged at error level and counted in the new proxy_artifacts_served_uncached_total.
• The client's stream ends once the last byte is checked. The scan of a monitor-mode scanner and the database update run after it, on a context the end of the request doesn't cancel, so the artifact is still stored when the response finished first.
• The response is chunked, without Content-Length. Once stored, the artifact is served with its size.

Unchanged:

• Requests that join the fetch while it runs wait for the stored copy, as before. So do HEAD requests and requests with a Range header. The server marks only GETs without Range (markRequestContext).
• With a block-mode scanner configured for the ecosystem, a miss is stored and scanned before anything is sent (scanner.Group.Blocks).
• Mirror jobs don't stream: their requests aren't marked.

The shared fetch runs on its own goroutine (runSharedFetchStreaming). The caller returns as soon as the fetch offers a stream, and waiters and the fetch lease still see the fetch end only once the artifact is stored.

Tests (internal/handler/stream_miss_test.go) go through the Git LFS handler with an upstream that sends the first half of an object and waits:

• the client reads the first 64 KiB before the upstream continues, then the artifact is cached and served from the cache with its size;
• a tampered object aborts the response and isn't cached;
• with storage failing, the client gets the whole artifact, and the error log and metric record it;
• a client that leaves after the first bytes still gets the artifact cached;
• a second request during the fetch gets the stored copy, with one upstream download;
• a client that reads nothing doesn't hold back a second request waiting on the fetch, and then gets every byte, the rest from the stored copy (seekable and non-seekable storage);
• a client that fell behind gets an error, not a short body, when storing fails;
• a block-mode scanner prevents streaming, a monitor-mode one doesn't;
• without a detached fetch, a slow storage backend still finishes storing after the response ended.

With streaming disabled, the six streaming tests fail. The block-mode scanner test and the slow-storage test check behaviour that doesn't depend on it: the first fails when storeFetched ignores block-mode scanners, the second when the store runs on the request's context. TestUnrecordedStreamedMiss (unrecorded_test.go) streams an unrecorded refetch: the caller handed the stream takes no hold on the object, the waiter still gets it, and it is deleted afterwards, also when the client fell behind.

The streaming tests passed 10 times in a row with -race.

I also ran git-lfs 3.5.1 in a container against Hugging Face through the proxy, with the upstream throttled to about 2 MB/s, pulling a 125 MB object (openai-community/gpt2, 64-8bits.tflite):

• main: git-lfs gave up on the first GET after 30 seconds and on its retry after another 30 (logged as 502, context canceled). The third attempt got the object from the cache once the detached download had finished.
• This branch: a single GET streamed the object in 65 s.

The pull succeeded both times only because the download fit within git-lfs's retries.

[andrew]

copyToBoth writes each chunk to storeW and then to clientW in the same loop. clientW is an io.Pipe, so Write blocks until the HTTP client has read the chunk, and the upstream read and the store copy run at the client's speed. Every coalesced waiter and every instance waiting on the fetch lease is held back by that one client. The upstream body is also read through artifactClient, whose timeout is artifact_timeout (5 minutes by default). Take a 500 MB artifact sent to a client reading at 1 MB/s: the download is cut at 300 seconds, nothing is cached, and the waiters get the error. On main the same download runs at upstream speed and caches. Please decouple the client so it can't stall the store copy, for example by dropping it once it falls a bounded buffer behind, or by serving it from what has already been written. Please also add a test with a slow-reading client and a second request waiting.

Main now has #506, which conflicts with this branch in runSharedFetch. When storeArtifact returns errUnrecorded and the caller was handed the stream, the unrecorded block adds a hold for this caller and calls serveUnrecorded. The streaming goroutine closes that reader straight away, which costs the storage read the handoff return is meant to avoid. If the handoff return goes first instead, the caller's hold is never released and the file is never deleted. When handedOver(), the caller's refs.Add(1) and serveUnrecorded should be skipped and only the waiters' placeholder kept. Please also add a test in unrecorded_test.go that goes through a streamed miss.

The file-level comment at the top of stream_miss.go describes what the proxy used to do. It should say what streaming does and why: clients with idle timeouts. The storeWhileServing doc comment can be cut to two points. The client copy is checked before its last bytes, so a mismatch aborts the response. A storage failure leaves the client copy running.