Skip to content

git-remote-entire: hold a pushed pack once; bound the replica cache - #2987

Merged
Soph merged 3 commits into
mainfrom
soph/remote-helper-push-memory-replica-cache
Nov 2, 2026
Merged

Soph merged 3 commits into
mainfrom
soph/remote-helper-push-memory-replica-cache

Conversation

@Soph

@Soph Soph commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #2985. Trail: https://entire.com/gh/entireio/-/trails/2710

Push memory. A push read all of git send-pack's output into one growing buffer, then copied the whole body again to amend the agent capability. Both happened before the server could refuse the push by X-Entire-Push-Size. The request is now kept as the chunks it was read in (gitproto.RequestBody), and the body is sent through an io.SectionReader with ContentLength and GetBody set explicitly, so the 401 retry, replica failover and redirects still resend it. The v0/v1 connect path, which the issue didn't mention, had the same buffering and gets the same fix. A 64 MiB push now allocates 65 MiB instead of 224 MiB. The pack is still held once; going below that would mean streaming the upload and giving up the declared size and replay.

Agent amend. The amend reads pkt-lines across chunks to the first command line (the first line carrying a NUL), copies and amends only that line, and re-slices the chunks around it, so the pack is never copied. Lines are read across chunk boundaries because a shallow push sends shallow <oid> lines before the command, and send-pack's stateless-rpc framing can split the command across 65515-byte packets. An earlier revision of this PR amended only the first chunk. That silently dropped the helper agent on connect-path pushes from shallow clones, and failed v2 pushes with about 1,200 or more grafts.

Replica cache. nodes.json never dropped expired entries and was rewritten in full, under its exclusive lock, on every info/refs response. Expired entries are now pruned on every write, and replicas.Persist skips the write when the same replica set (in any order) is cached with more than half its TTL left. The skip check reads without the lock; an Invalidate racing it costs one extra discovery, never a wrong replica set.

Tests.

  • RequestBody: chunk reads across boundaries; ReadFrom returning reader errors, including ones delivered with the final bytes and wrapped EOFs.
  • Agent amend: the command line split at several points across chunks, shallow lines first on the connect path, and a command straddling send-pack's 65515-byte packet; requests without capabilities left unchanged; truncated and invalid lengths rejected; pack chunks still the original memory. The first-chunk amend fails the split and shallow cases.
  • A real shallow-clone push through git send-pack, pinning that the shallow line comes first and the agent lands on the command.
  • Replay through a real failover and a 307 redirect; cache pruning and skip-unchanged.
  • TestHandlePush_RealSendPack_DeclaresSizeAndLands previously accepted Content-Length -1 (a chunked upload) and now requires the declared length.

mise run check: lint and all tests pass except TestHTTPS_PushFailsWithoutToken/git-refs (hangs) and TestOpenCodeSeedRepoPlantsDeps, which fail the same way on main on this machine.

🤖 Generated with Claude Code


Note

Medium Risk
Changes how push/connect POST bodies are buffered and replayed and when the shared replica cache is locked, which are on the critical git push/fetch path but heavily tested.

Overview
Reduces push memory use and cache lock contention in the remote helper, and makes large receive-pack POST bodies safe to resend on failover and redirects.

Push / connect paths now buffer receive-pack requests in a chunked gitproto.RequestBody instead of one growing []byte. Pack data is stored once per chunk; amending the helper agent copies only the first command pkt-line. POSTs set Content-Length and GetBody via ApplyTo, so X-Entire-Push-Size stays aligned with what the server reads and net/http can replay the body on 401 retry, replica failover, and 307 redirects (covered by new transport tests).

Replica discovery cache prunes expired repo entries on every nodes.json write so the file and lock duration do not grow without bound. Persist skips the exclusive ModifyCache lock when the cached node set is unchanged (order-insensitive) and still has more than half the default TTL left; aging entries are still rewritten to extend expiry.

Reviewed by Cursor Bugbot for commit dfff12d. Configure here.

A push read all of send-pack's output into one growing buffer, then copied the whole body again to amend the agent capability on its first line. A 64 MiB push allocated 224 MiB; it now allocates 65 MiB. The request is kept as the chunks it was read in (gitproto.RequestBody), the amend copies only the first chunk, and the body is replayed through an io.SectionReader with ContentLength and GetBody set explicitly. The v0/v1 connect path had the same buffering and gets the same fix.

The replica cache never removed expired entries and rewrote all of nodes.json under its exclusive lock on every info/refs response. Expired entries are now pruned on every write, and Persist skips the write when the same replica set is cached with more than half its TTL left.

Fixes #2985

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Entire-Checkpoint: 01M679VD5NVJXMNGQ00H59PYPY
Copilot AI balanced review requested due to automatic review settings October 30, 2026 21:01

Copilot AI left a comment

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.

🟡 Changes recommended

Parsing only the first chunk can reject valid split command packets and leave the helper agent unamended on shallow-prefixed connect requests.

1 open issue
What changed in this PR

This PR reduces memory use during remote-helper pushes and limits replica-cache write contention, while making buffered receive-pack bodies replayable.

Changes:

  • Replaces contiguous push buffers with chunked request bodies shared by push and connect paths.
  • Sets request length and replay metadata, with tests for replica failover and HTTP redirects.
  • Prunes expired cache entries and skips writes for unchanged replica sets with sufficient remaining lifetime.
File Description
internal/​remotehelper/​transport/​body_replay_test.go Tests full-body replay on failover and redirects.
internal/​remotehelper/​replicas/​replicas.go Skips unnecessary replica-cache writes.
internal/​remotehelper/​replicas/​replicas_test.go Tests unchanged-set skips and aging-entry refreshes.
internal/​remotehelper/​gitproto/​gitproto.go Reads receive-pack and send-pack data into chunks.
internal/​remotehelper/​gitproto/​gitproto_test.go Adapts protocol tests to chunked bodies.
internal/​remotehelper/​gitproto/​fuzz_test.go Updates send-pack fuzzing for the new body type.
internal/​remotehelper/​gitproto/​body.go Adds chunked storage, replay readers, and agent amendment.
internal/​remotehelper/​gitproto/​body_test.go Tests chunk reads, replay, error handling, and amendment.
internal/​remotehelper/​githelper/​push.go Uses chunked push bodies and shared request headers.
internal/​remotehelper/​githelper/​push_size_git_test.go Requires Content-Length to match the declared push size.
internal/​remotehelper/​githelper/​connect.go Uses chunked bodies for receive-pack connections.
internal/​entireclient/​discovery/​cache.go Adds expiry pruning and order-insensitive node comparison.
internal/​entireclient/​discovery/​cache_test.go Tests pruning and cached-node matching.

Review effort: Balanced

Comment thread internal/remotehelper/gitproto/body.go Resolved
@Soph Soph changed the title soph/remote-helper-push-memory-replica-cache git-remote-entire: hold a pushed pack once; bound the replica cache Oct 30, 2026
@Soph
Soph marked this pull request as ready for review October 30, 2026 21:07
@Soph
Soph requested a review from a team as a code owner October 30, 2026 21:07
Soph and others added 2 commits October 31, 2026 12:03
… chunk

A shallow push sends `shallow <oid>` lines before the first command, and send-pack's stateless-rpc framing can split that command across 65515-byte packets. Amending only the first chunk left the helper agent off every connect-path push from a shallow clone, and failed v2 pushes with enough grafts to push the command past the first packet.

Walk the pkt-lines across chunks to the first line carrying a NUL, copy and amend only that line, and re-slice the chunks around it so the pack stays where it is.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Entire-Checkpoint: 01M68T291PYT5DTSKNFSYD807V
… not cover

The real send-pack shallow test passes on the first-chunk amend too, so its comment now points to gitproto's split-line tests as the regression cases. Persist's comment records that its unlocked skip check can lose a racing Invalidate, at the cost of one extra discovery.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Entire-Checkpoint: 01M6DWR8HMN6AFZV7SH2839XCP
@Soph
Soph merged commit e2b69f1 into main Nov 2, 2026
23 checks passed
@Soph
Soph deleted the soph/remote-helper-push-memory-replica-cache branch November 2, 2026 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

git-remote-entire: push uses ~4× the pack size in memory; replica cache grows without bound

3 participants