Skip to content

Stream git packs through consumePack instead of buffering them - #656

Merged
ndisidore merged 9 commits into
mainfrom
chore/git-pack-streaming
Oct 5, 2026
Merged

ndisidore merged 9 commits into
mainfrom
chore/git-pack-streaming

Conversation

@ndisidore

@ndisidore ndisidore commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

Mounting a large repo as a worktree ran the Overseer Durable Object out of memory. consumePack held the whole pack, every inflated object and a deflated copy of each before storing anything. For a TypeScript- or vscode-size mount that came to 173-224 MiB, against a 128 MB limit.

decodePackStream now decodes the pack in one pass. consumePackFromGatekeeper stores blobs of up to 1 MiB as they arrive. It holds commits, trees, tags and oversized blobs until the trailer verifies, then stores them one per transaction with commits last, so a failed pack can leave verified trees and blobs behind but never a commit that looks mounted. Loose objects are compressed with native node:zlib instead of pako.

Smaller changes in the same PR, one commit each. A pack entry whose size varint overflows is now rejected; before, its NaN size slipped past the object size cap. Loose objects are deflated at level 1, about 1.5x faster for 9-12% more stored bytes. An object a gatekeeper already stored is left untouched when a pull delivers it again, which took a repeat vscode pull from about 7 s to 3 s locally. A failed git fetch now reports the server's error or the transfer limit, not a stream that disconnected prematurely. The consumePack contract doc now describes streaming, and a new test sends a pack over real Workers RPC.

This should let a repo like vscode mount, as long as its mount pack fits the 64 MiB cap. The real mount packs for react, next.js, TypeScript, vscode and a 52 MiB internal repo went through this code on Durable Object SQLite in workerd. Every object was stored, except TypeScript's two trees over 1 MiB, which were measured and skipped as before. In Node, the decoder held about 8 MiB after GC on the TypeScript pack.

  • CPU, measured on Cloudflare with the real mount packs before the level-1 and re-delivery changes: vscode about 14 s; TypeScript-size packs (65k objects) still about 38 s, over the 30 s default
  • Memory on a deployed instance not yet measured
  • Packs over 64 MiB still rejected (Linux's mount pack is 109 MiB at v4.0, 191 MiB today)
  • Oversized blobs held in memory until a whole-tree blob pull finishes
  • Trees over 1 MiB still not stored, so paths under them can't be read
  • Deltas must follow their bases, true of every pack upload-pack sends without haves

Devin Review

Mounting a large repo as a worktree exceeded the Overseer's 128 MB
memory limit. consumePack collected the whole pack, inflated every
object, and deflated each one again before storing anything, so a
TypeScript- or vscode-size mount held 173-224 MiB of buffers at once.

decodePackStream replaces decodePackBytes. It decodes the pack in one
pass, keeping only each entry's oid by offset, and resolves every delta
base through resolveBase. It reads the stream with a BYOB reader in
64 KiB chunks, enforces the pack size cap as bytes arrive, and hashes
exactly the bytes before the trailer. A failed decode cancels the
source. Every existing malformed-pack check is kept. MAX_DELTA_DEPTH is
gone because nothing recurses any more.

consumePackFromGatekeeper stores blobs of up to 1 MiB as they arrive.
It holds commits, trees, tags and oversized blobs (a later delta may
name one as its base) and stores them in one transaction once the
trailer verifies, so a failed pack never leaves a commit that
fetchCommit would treat as mounted. #storeVerifiedObject now handles
the oversized case for both put() and consumePack.

A delta must now follow its base. That holds for every pack git
upload-pack sends a fetch with no haves, which is the only kind the
gatekeepers request.

Real depth-1 mount packs (react, next.js, TypeScript, vscode, and a
52 MiB internal repo) went through the real code on Durable Object
SQLite in workerd. Every object was stored, or measured if oversized.
A pack with a corrupted trailer left none of its commits or trees.
@github-actions github-actions Bot added the kernel Changes to the Workshop kernel label Oct 3, 2026
@ask-bonk

ask-bonk Bot commented Oct 3, 2026

Copy link
Copy Markdown

LGTM!

github run

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

Preview: pr656-chore-git-pac-4728fd9e

https://pr656-chore-git-pac-4728fd9e-router.cloudflare-os-previews.workers.dev

Dashboard · deleted when this PR closes

@ndisidore
ndisidore marked this pull request as ready for review October 3, 2026 16:35
devin-ai-integration[bot]

This comment was marked as resolved.

@ndisidore
ndisidore marked this pull request as draft October 3, 2026 16:45
Mounting a vscode fork on a preview exceeded the Overseer's 30 s
per-invocation CPU limit. In local workerd, deflating every stored
object with pako in encodeLooseObject was the largest single cost of
consumePackFromGatekeeper (about 4 of 10 s for vscode).

encodeLooseObject and decodeLooseObject now use workerd's native
node:zlib (deflateSync/inflateSync, available under nodejs_compat).
The output is standard zlib at the same default level, so stored
records stay readable by isomorphic-git and by records written before.
decodeLooseObject views inflateSync's Buffer as a plain Uint8Array, so
the payloads it hands out stay plain arrays. The pack decoder keeps
pako, which it needs to find where each entry's zlib stream ends.

consumePackFromGatekeeper in local workerd, three runs each, real
Overseer DO storage, workerd process CPU:
- vscode (23,289 objects): 10.3 s -> 7.4 s
- TypeScript (65,040 objects): 18.6 s -> 13.3 s
Every object was stored or measured, oids recompute, isomorphic-git
reads a sample byte-for-byte, and stored bytes and SQLite size are
unchanged. Whether a vscode mount now fits the production limit is
not yet verified.
@ask-bonk

ask-bonk Bot commented Oct 3, 2026

Copy link
Copy Markdown

LGTM!

github run

A vscode mount still reset the Overseer on the preview, now on CPU: the
invocation running consumePack used 32.5 s against the 30 s default.

Run on Cloudflare against the real vscode mount pack, in a throwaway
Worker's SQLite Durable Object, consumePackFromGatekeeper used 23.7-98 s
of CPU, and 3 of 7 runs were reset on a storage timeout. Most of the
excess was the one transaction around the held commits and trees: each
metadata put inside it opens a nested transaction, and ~29k of those in
one transaction cost 14-18 s there. Locally the whole transaction takes
under 1 s.

Held objects are now stored one per transaction, like the small blobs,
with commits last. No await separates the stores, so they still reach
disk together. A store that throws rolls back only its own object, and
with commits last no commit is stored before the pack's trees. A new
test covers a tree the cache refuses (mode 100664, which git only
reports as informational) arriving after its commit, as in the packs
git sends.

Same production round, interleaved: vscode 13.1-14.5 s (was 23.7-27.4
s), every object stored. TypeScript-size packs (65k objects) still take
about 38 s (was 48.6 s), over the default limit.
@ask-bonk

ask-bonk Bot commented Oct 4, 2026

Copy link
Copy Markdown

LGTM!

github run

@ndisidore
ndisidore marked this pull request as ready for review October 4, 2026 17:05
Comment thread packages/workshop-backend/src/git-codec.ts Outdated
devin-ai-integration[bot]

This comment was marked as resolved.

Maximo-Guk added a commit that referenced this pull request Oct 4, 2026
Each blob consumePack stores as it arrives sat in its own
storage.transaction, and the metadata put inside opens a nested one for
its index. A transaction is there to undo a store that fails after its
first write, and what can fail there is parsing the objects the stored
one names, in #referentEntries and the marking walk. A blob names none,
and an oversized object is only measured, so both are now stored
without the wrapper. Commits, trees and tags keep theirs.

If the metadata write itself failed, the blob's row would stay with no
metadata: a hash-verified object no gatekeeper can read, which the next
pull completes.

First pull in local workerd on Durable Object SQLite, best of three,
on a machine that was busy for part of the "before" runs:
- vscode (23,280 objects): 8.2 s -> 7.3 s
- TypeScript (65,044 objects): 14.5 s -> 11.2 s
Not measured in production, where the third commit of #656 found the
nesting far more expensive than it is locally.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Maximo-Guk added a commit that referenced this pull request Oct 4, 2026
Two things #656 depends on had no test.

Every consumePack test builds a byte stream by hand, the one shape that
is certain to support BYOB reads. Production gets a default stream that
gatekeeper-github's demuxGitFetchResponse makes in another Worker and
Workers RPC carries over; handed to the decoder directly, such a stream
throws "This ReadableStream does not support BYOB reads". The new test
loads a small Worker through the LOADER binding, which calls
consumePack on a GitCacheImpl stub with a default pull stream, as a
gatekeeper does.

A ref-delta whose base comes later in the pack decoded on main and is
now rejected. Git writes a base before its deltas, so nothing real
should send one, but the rule was only stated in a comment.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Maximo-Guk added a commit that referenced this pull request Oct 4, 2026
The GitCache.consumePack doc called it exactly equivalent to a put() of
each object. It differs in ways a gatekeeper can see: an oversized
object is left out of the result where put() throws and, since #656, a
delta has to follow its base and a pack that fails partway can leave
some of its objects stored. The return value was not described at all.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Maximo-Guk added a commit that referenced this pull request Oct 4, 2026
consumePack keeps every object over MAX_GIT_OBJECT_SIZE inflated until
the pack ends, because a later delta may name one as its base. A blob
fetch sends no filter, so a batch read can carry any number of them,
and the pack's 64 MiB cap bounds only their compressed size. Thirty
5 MiB files that compress well are 150 MiB against the Overseer's
128 MB. #656 kept them the same way, in `held`.

They are now kept up to MAX_OVERSIZED_BASE_BYTES (32 MiB), the oldest
let go first and the newest always kept, since git writes a delta soon
after its base. A delta that names one already let go fails the pack
with "delta base ... is unavailable". Every oversized object before it
has been measured by then, so the next pull does not ask for them, and
without them in the pack the blob that needed the base should arrive
whole. That retry is reasoned, not exercised.

Nothing here measures Worker memory; the test checks which base is
still on hand once two blobs pass the budget.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Maximo-Guk added a commit that referenced this pull request Oct 4, 2026
The budget on oversized delta bases failed a pack whenever a delta named
a base it had let go, on the reasoning that the sizes recorded by then
keep those objects out of the next request. That holds only for a
request that named each blob. A mount wants a commit and sends no
haves, so the remote answers a retry with the same pack, and it fails
the same way: a valid pack could never be mounted. Devin's review
caught this.

Bases are now let go only from a pack that has carried nothing but
blobs, which is what the answer to explicit blob wants looks like. A
pack with a commit, tree or tag in it keeps every oversized base, as
#656 does.

For the blob case the retry now happens inside the same read.
ensureGitObjects checked recorded sizes once, before its first pull; it
now checks on every round, so after a pull fails on a dropped base the
objects it measured surface as GitObjectTooLargeError, ensureBlobs
takes them out of the batch, and the next pull brings the dependent
blob whole. This also turns "every connection failed" into the
too-large error whenever a failed pull was what measured the object.

Two tests fail on the previous commit: a pack with a commit and two
blobs past the budget is consumed, and a batch read whose pack fails on
a dropped base completes in one ensureBlobs call with two pulls.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@Maximo-Guk

Copy link
Copy Markdown
Member

🤖 Reviewed, feel free to take whatever makes sense from #660

Maximo-Guk added a commit that referenced this pull request Oct 4, 2026
MAX_GIT_PACK_BYTES did two jobs: it capped the pack, and through
maxObjectSize it capped each object inflated from one. The 64 MiB came
from gatekeeper-context's artifact sync, where it bounds a repository
loaded into memory, and consumePack matched it while it still read the
whole pack into memory before decoding.

Since #656 the pack streams through, so its cap no longer bounds
memory, only how much one pull may download, decode and store. The
per-object cap still is a memory bound. They are now separate:
MAX_GIT_PACK_OBJECT_SIZE stays at 64 MiB, and MAX_GIT_PACK_BYTES goes
to 256 MiB, the smallest round figure that admits a depth-1 mount of
torvalds/linux (190.8 MiB, 98,591 objects).

That pack through consumePack, local workerd on Durable Object SQLite:
every object stored (222.8 MiB of rows), the tree listing all 102,334
paths, in 44 s fed from a local stream and 70 s through an RPC hop in
GitHub-sized pieces. In production it would run past the 30 s CPU
limit, which this does not touch.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Maximo-Guk added a commit that referenced this pull request Oct 4, 2026
fetchGitUploadPack gave the whole fetch 120 s, body included. That
suited a body read straight into memory. Since #656 the pack streams on
to the overseer, which reads it only as fast as it stores it, so the
budget now covers decoding and storing the pack too. A depth-1 mount of
torvalds/linux took 70 s that way in local workerd, and a pack at the
256 MiB cap would be past 90 s.

The 120 s now covers the wait for the response and an error body. Once
the pack is streaming, git-transport.ts fails the fetch if the server
sends nothing for GIT_FETCH_STALL_MS (60 s) while the pull is waiting on
it. Time the reader takes between reads does not count. git's
upload-pack sends a keepalive every few seconds when it has nothing
else to send, so a live fetch stays well inside that.

Two tests with fake timers: a body that goes quiet fails after 60 s,
and a reader that pauses for ten times that between reads does not.
The change to fetchGitUploadPack itself has no test; nothing covered
its timeout before either.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Maximo-Guk added a commit that referenced this pull request Oct 4, 2026
Native inflate needs an entry's whole compressed stream at once, and
joins its output from 16 KiB chunks. For one large object that is the
stream, the chunks and the joined copy all live together: review worked
it through for a 48 MiB blob deflated at level 0, about 150 MiB against
the Overseer's 128 MB. The pako reader in #656 held one read of input,
so the same entry fit there.

An entry declaring more than 1 MiB now goes back to that loop: pako, a
read at a time, with no input kept beyond the read in hand. Its output
is written into one buffer of the declared size, so the chunk list and
the copy that joined it are gone too, and inflating that 48 MiB entry
takes about 48 MiB where #656 took 96. Entries up to 1 MiB, which is
all of a mount pack but the odd large tree, stay on native inflate,
where the most one can buffer is now a little over a megabyte.

pako's Inflate import, its internals interface and that cast are back
for this path. It is given windowBits 15, so it reads zlib only, not
the gzip it would otherwise also accept.

Run for real: the 48 MiB level-0 entry goes through consumePack and is
measured; the vscode, TypeScript and Linux mount packs decode to the
same totals, TypeScript's two trees over 1 MiB on the new path. Memory
is reasoned from what each path holds, not measured.

Co-Authored-By: Claude Code <noreply@anthropic.com>
The decoder checks each entry's declared size against maxObjectSize
before inflating it, but a size varint with enough zero continuation
digits overflows its multiplier to Infinity, and 0 * Infinity leaves a
NaN size that the comparison lets through. On a 1,024-byte cap such an
entry inflated 8 MiB before failing.

The varint loop now rejects the entry once the next digit would be
worth more than the cap, since no size within the cap needs it.
Deflating each object in encodeLooseObject is the largest single CPU
cost of storing a pack, and CPU is what still stops large mounts. On
this repo's 1,378 tracked files of up to 64 KiB, level 1 deflated 1.53x
faster than the default level 6 and stored 11% more bytes. In local
workerd a vscode first pull went from 7.3 s to 6.2 s with 12% more
stored bytes, and a TypeScript one stored 9% more.

Any zlib level is a valid loose object: oids hash the inflated bytes,
isomorphic-git and decodeLooseObject read every level, and rows
already stored at level 6 stay readable. Push packs are re-compressed
by buildPackBytes, so they are unaffected. The extra bytes are kept
for as long as the workspace is, since nothing repacks the store.
A pull sends no haves, so a retried pull, or one for another commit of
an already-mounted repository, delivers mostly objects the store
already holds. #storeVerifiedObject rewrote each of them: a fresh
deflate, an object put and a metadata put, for no change.

An object that is already present, measured and proven on this
gatekeeper's remote is now left as it is. Nothing it would write can
differ: the payload is fixed by the oid, its referents were recorded
and its pending-push marks propagated when it was first stored, and
marking walks descend into present objects themselves. Another
gatekeeper's copy is still recorded as that gatekeeper's proof.

In local workerd, delivering the same vscode pack again went from
6.7-7.9 s to 2.8-3.2 s.
@github-actions github-actions Bot added gatekeeper Changes to a gatekeeper integration workshop/shared Changes to shared Workshop APIs labels Oct 5, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

Devin Review

Comment thread packages/workshop-backend/src/git-codec.ts
@ask-bonk

ask-bonk Bot commented Oct 5, 2026

Copy link
Copy Markdown

LGTM!

github run

The consumePack tests hand the decoder a byte stream. A gatekeeper
sends a default stream from another Worker, which the decoder's BYOB
reader refuses when handed one directly ("This ReadableStream does not
support BYOB reads"); it works only because Workers RPC delivers it as
a byte stream. A new test sends a real pack that way, from a Worker
Loader worker in 100-byte chunks, and checks every object is stored
with its bytes intact.
It said consumePack is exactly equivalent to decoding the pack and
put()ting each object. Since packs are streamed that no longer holds:
a delta must follow its base, and a pack that fails can leave some of
its objects stored, though never a commit without its trees. The doc
also now says what the result holds and that an object too large to
store is left out of it rather than thrown on.
When the fetch response fails -- a server ERR line, the transfer
limit, a truncated response -- consumePack sees the pack stream it
reads across RPC only end early, and rejects with "disconnected
prematurely". That is what gitPull reported, so an agent mounting a
repo too large to fetch was told the connection dropped, and retried.

demuxGitFetchResponse takes an optional onFailure callback, and
pullGitObjectsIntoCache rethrows the failure it reports in place of
consumePack's rejection. A consumePack failure of its own still
surfaces when the fetch itself succeeded.
@ndisidore
ndisidore force-pushed the chore/git-pack-streaming branch from 76c2dfa to 6685c06 Compare October 5, 2026 11:45
@ask-bonk

ask-bonk Bot commented Oct 5, 2026

Copy link
Copy Markdown

LGTM!

github run

@ndisidore
ndisidore merged commit bde379d into main Oct 5, 2026
14 checks passed
@ndisidore
ndisidore deleted the chore/git-pack-streaming branch October 5, 2026 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gatekeeper Changes to a gatekeeper integration kernel Changes to the Workshop kernel workshop/shared Changes to shared Workshop APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants