Skip to content

feat: add real-time progress tracking for multipart weight uploads#2688

Merged
markphelps merged 3 commits intomainfrom
mp/multi-part-weights-upload
Feb 5, 2026
Merged

feat: add real-time progress tracking for multipart weight uploads#2688
markphelps merged 3 commits intomainfrom
mp/multi-part-weights-upload

Conversation

@markphelps
Copy link
Copy Markdown
Contributor

Summary

  • Implements multipart uploads for large weight blobs using Content-Range headers
  • Adds real-time progress tracking during chunk uploads (updates every ~32-64KB instead of every 25MB chunk)
  • Fixes bug where commit used stale location URL instead of the final location with updated state hash

Changes

  • Modified uploadChunk to wrap the chunk reader with progress tracking, reporting progress as bytes are written to the network
  • Updated uploadBlobChunks to return the final location URL for proper commit handling
  • Unified channel handling to use non-blocking sends across both single-part and multipart uploads
  • Added defensive progress capping at totalSize to prevent overshoot

@markphelps markphelps requested a review from a team as a code owner February 5, 2026 17:06
Copy link
Copy Markdown
Contributor

@mfainberg-cf mfainberg-cf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to eventually setup parallel uploads, small MPUs at 25M could benefit from it.

Comment thread pkg/registry/registry_client.go Outdated
Comment thread pkg/registry/registry_client.go Outdated
Comment on lines +823 to +849
// commitUpload finalizes the upload by sending a PUT request with the digest.
func (c *RegistryClient) commitUpload(ctx context.Context, client *http.Client, location string, digest v1.Hash) error {
u, err := url.Parse(location)
if err != nil {
return fmt.Errorf("parsing location URL: %w", err)
}

// Add digest query parameter
q := u.Query()
q.Set("digest", digest.String())
u.RawQuery = q.Encode()

req, err := http.NewRequestWithContext(ctx, http.MethodPut, u.String(), nil)
if err != nil {
return err
}

req.Header.Set("Content-Type", "application/octet-stream")

resp, err := client.Do(req)
if err != nil {
return err
}
defer resp.Body.Close()

return transport.CheckError(resp, http.StatusCreated)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're terminally failing here maybe we should cancel the MPU? Are we planning to have a resume MPU function in the case the CLI crashes or is SIGINT'd? These are questions not any changes needed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good questions. yeah we should do cleanup on early exit. will do in a followup. first let me check if the registry has gc too

@markphelps
Copy link
Copy Markdown
Contributor Author

We may want to eventually setup parallel uploads, small MPUs at 25M could benefit from it.

its uploading 4 in parallel currently, well 4 layers in parallel. i might tune that to use gomaxprocs, although its not cpu bound but network bound so maybe a hard cap on parallel layers is sufficient

… logic

Address PR feedback:
- Set multipart threshold to 50MB (was 25MB) to avoid MPU overhead for smaller files
- Keep chunk size at 25MB for actual multipart uploads
- Extract multipart fallback logic into tryMultipartWithFallback() for cleaner code
Signed-off-by: Mark Phelps <[email protected]>
@markphelps markphelps merged commit edad085 into main Feb 5, 2026
28 of 29 checks passed
@markphelps markphelps deleted the mp/multi-part-weights-upload branch February 5, 2026 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants