Fix model download resume with RFC-compliant Range requests - #1250
Open
Carton wants to merge 1 commit into
Open
Conversation
The contribute client already tried to resume interrupted model downloads
by appending to the partial file and sending a Range header for the
remaining bytes. However, the header omitted the byte-range unit required
by RFC 7233, for example:
Range: 100000000-211558521
instead of:
Range: bytes=100000000-211558521
RFC-compliant servers treat the malformed header as if no Range was sent
and reply 200 with the full content. The client then appended the full body
at the old offset, corrupting the file. The receiver stopped once
totalDataSize exceeded the expected size, and the
"totalDataSize >= modelInfo.bytes" entry condition then returned a bogus
success. This surfaced as the confusing error "Model file was incompletely
downloaded, only got 271379114 bytes out of 271375520", after which the
outer retry loop restarted the entire download. On flaky links this made
every interruption cost a full re-download.
The model host (media.katagotraining.org, GCS behind Cloudflare) supports
Range requests when the header is well formed, returning 206 to a curl -r
request.
Fixes:
- Send "Range: bytes=start-end" so compliant servers return 206 and the
append-and-continue logic resumes correctly.
- Validate status and Content-Range before streaming the response body. If
a resumed request receives 200, or receives 206 with a missing or
unexpected Content-Range, reject the body, discard the partial file,
remember that Range is unavailable, and restart from byte 0.
- Replace the "totalDataSize >= modelInfo.bytes" early return with an
explicit overshoot check. Only retained bytes count as partial success;
discarded data and interrupted full downloads after Range fallback
consume the normal retry budget rather than resetting it indefinitely.
- Reject non-200/206 response bodies before they can be written to the
model file.
- Log "Resuming download of model at byte N" when a retry resumes.
Verification included the official katagotraining.org API and a local
model mirror simulating two failure modes: (A) Range honored plus a
mid-stream connection drop, resuming via 206 from the dropped offset; and
(B) Range ignored with a 200 response plus a connection drop, falling back
to a clean full download. Both produced the expected sha256. The combined
change also builds successfully and passes the built-in test suite.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Range: bytes=start-endheaders when resuming model downloads.Content-Rangebefore appending data.Validation
katagoexecutable successfully.