Feat/concurrent blob pull#140
Merged
Merged
Conversation
FetchBlobs inlined the whole per-blob download path inside an anonymous function, leaving no seam to reuse it. The upcoming concurrent worker pool needs to invoke that exact path from each goroutine, so it is lifted into fetchSingleBlob. This is a pure refactor: the sequential loop and observable behavior are unchanged. Signed-off-by: Mike Sul <mike.sul@foundries.io> Assisted-by: Claude Code:claude-opus-4-8
Blobs were fetched strictly one at a time, so on high-latency links or apps with many small blobs the transfer was bottlenecked by round-trip latency rather than bandwidth. FetchBlobs now drives the downloads through a bounded errgroup pool so several blobs transfer at once while capping how many run together. The pool is fail-fast: the first error cancels the derived context, aborting in-flight and queued downloads, and that error is returned. Each blob is owned by exactly one worker and the shared progress counters stay single-writer, so no new shared mutable state is introduced. Worker count is configurable via WithFetchWorkers and defaults to DefaultFetchWorkers (3) when unset. Signed-off-by: Mike Sul <mike.sul@foundries.io> Assisted-by: Claude Code:claude-opus-4-8
mike-sul
force-pushed
the
feat/concurrent-blob-pull
branch
from
cd7d2f5 to
808a0db
Compare
The concurrent fetch pool added to FetchBlobs had no user-facing control, so the parallelism was fixed and could not be tuned to a link or a registry's tolerance. This exposes it as a --workers/-w flag on the pull command, defaulting to 3. The value is validated up front against a strict 1-10 range, mirroring the existing watermark check, so an out-of-bounds argument fails with a clear message and a non-zero exit before any pull work starts. An e2e test covers both the range validation and a successful multi-worker pull. Signed-off-by: Mike Sul <mike.sul@foundries.io> Assisted-by: Claude Code:claude-opus-4-8
The dev container image is Alpine-based and shipped no C compiler, so CGO resolved to disabled and the Go race detector could not run. That left the new concurrent blob-fetch worker pool without automated data-race verification. Install gcc and musl-dev in the dev image and enable CGO_ENABLED=1 with -race in the test-unit target so every unit run is race-instrumented. The cgo setting is scoped to the test-unit recipe line, leaving the static production build (which is built with cgo off elsewhere) unchanged. Signed-off-by: Mike Sul <mike.sul@foundries.io> Assisted-by: Claude Code:claude-opus-4-8
mike-sul
force-pushed
the
feat/concurrent-blob-pull
branch
from
808a0db to
b4032f0
Compare
Collaborator
Author
|
@mike-scott I think having three concurrent workers pulling app blobs by default is a reasonable default value based on my observation (at least for unoq pulling data over wifi). We might wanna introduce a logic that determines worker number dynamically, just don't think it is a good time for such advanced stuff while we have a few moving pieces going on - reducing storage usage by ingesting blobs directly to the containerd storage and developing a composefs snapshotter. |
doanac
approved these changes
Jun 10, 2026
doanac
left a comment
doanac
left a comment
Member
There was a problem hiding this comment.
Looks good at a high level. I sort of skimmed the tty logic
Collaborator
Author
I tested it quite well under different use-cases. Initially it wasn't ideal, so I had to make claude improve it. |
The pull progress handler tracked every blob in a single redraw region and moved the cursor up by its full length each frame. Because completed blobs were never dropped, that region grew to the total blob count; once it exceeded the terminal height the cursor-up clamped at the top of the screen and bottom-row newlines scrolled it, desynchronizing the redraw. In-flight blobs were then reprinted on fresh lines, so a single blob appeared on several lines. Bound the live region to only the in-flight blobs and graduate each blob to one permanent line when it completes, so the region can never outgrow the screen and history scrolling is harmless. Each in-flight blob thus owns exactly one in-place updated line. Without a TTY cursor control is meaningless, so output is append-only instead: each blob gets a line when it first appears and whenever it crosses another 10% of its size, plus one final completion line. Previously only the completion line was printed, so a pull dominated by one large layer looked hung for minutes in log collectors such as journalctl. The shared line formatter now reads the cross-goroutine byte counters via atomic loads, removing a latent race with the concurrent fetch workers. Signed-off-by: Mike Sul <mike.sul@foundries.io> Assisted-by: Claude Code:claude-fable-5
mike-sul
force-pushed
the
feat/concurrent-blob-pull
branch
from
b4032f0 to
ecf1265
Compare
Collaborator
Author
|
The progress output in non-tty wasn't implemented entirely correctly, this ecf1265 fixes it. |
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
3 participants
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.
No description provided.