feature: Compression for memfile/rootfs assets

[levb]

Summary

Compression for data files (memfile, rootfs). Files are broken into independently decompressible frames (2 MiB, zstd), stored in GCS alongside V4 headers with per-mapping frame tables. Fully backward-compatible: the read path auto-detects V3/V4 headers and routes compressed vs uncompressed reads per-mapping. Gated by compress-config LaunchDarkly flag (per-team/cluster/template targeting).

What changed

• FramedFile interface replaces Seekable — unified GetFrame(ctx, offset, frameTable, decompress, buf, readSize, onRead) handles both compressed and uncompressed data
• V4 header with FrameTable per mapping + BuildFileInfo (uncompressed size, SHA-256 checksum) per build; LZ4-block-compressed header blob
• NFS cache extended for compressed frames (.frm files keyed by compressed offset+size); progressive streaming decompression on cache miss; write-through on upload
• P2P resume integration — peers read uncompressed from origin during upload, then atomically swap to V4 header (CAS) when origin signals use_storage with serialized headers
• compress-build CLI for background compression of existing uncompressed builds (supports --recursive for dependency chains)
• New Chunker with mmap cache, and fetch sessions dedupe replacing streaming_chunk.go

Read path

  NBD/UFFD/Prefetch
    → header.GetShiftedMapping(offset) → BuildMap + FrameTable
    → DiffStore.Get(ctx, diff)         → cached Chunker
    → Chunker.GetBlock(offset, len, ft)
        → mmap hit? return reference
        → miss: fetchSession (dedup) → GetFrame
            → NFS hit? decompress from disk → mmap
            → NFS miss? GCS range read → decompress → mmap + NFS write-back

P2P header switchover

  Origin (pause):
    snapshot → register buildID in Redis → serve mmap cache via gRPC
    background: upload compressed data + V4 headers to GCS
    on completion: uploadedBuilds.Set(buildID, serialized V4 headers)
                → peerRegistry.Unregister(buildID)

  Peer (resume, upload in progress):
    GetFrame(ft=nil) → gRPC stream → origin serves from mmap (uncompressed)

  Peer (origin signals use_storage):
    checkPeerAvailability() → transitionHeaders.Store({memH, rootH})
                            → uploaded.Store(true)
    next GetFrame(ft=nil): ft==nil + transitionHeaders != nil
      → return PeerTransitionedError{headers}
      → build.File.swapHeader(): Deserialize(bytes) → CompareAndSwap(old, new)
        first goroutine wins CAS; others see swapped header on retry
      → retry: GetFrame(ft!=nil) → NFS/GCS compressed (mmap mostly warm)

Benchmark results

End-to-end pause/resume

(BenchmarkBaseImage, 50 iterations, local disk):

  ┌──────────────┬─────────┬────────────┐
  │     Mode     │ Latency │ Build time │
  ├──────────────┼─────────┼────────────┤
  │ Uncompressed │ 97 ms   │ 61.0s      │
  ├──────────────┼─────────┼────────────┤
  │ LZ4:0        │ 100 ms  │ 61.4s      │
  ├──────────────┼─────────┼────────────┤
  │ Zstd:1       │ 100 ms  │ 60.9s      │
  ├──────────────┼─────────┼────────────┤
  │ Zstd:2       │ 102 ms  │ 62.4s      │
  ├──────────────┼─────────┼────────────┤
  │ Zstd:3       │ 98 ms   │ 61.7s      │
  └──────────────┴─────────┴────────────┘

Full architecture doc: docs/compression-architecture.md

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🔴 packages/orchestrator/pkg/sandbox/template/peerserver/seekable_test.go:17-23 — TestSeekableSource_Size mocks the wrong method, breaking CI: the test expects diff.FileSize() but seekableSource.Size (seekable.go:22) actually calls f.diff.Size(ctx). go test -run TestSeekableSource_Size ./packages/orchestrator/pkg/sandbox/template/peerserver/ fails with unexpected method call: Size(*context.cancelCtx) and FAIL: 0 out of 1 expectation(s) were met. FileSize(). Fix is one-character: change the mock to diff.EXPECT().Size(mock.Anything).Return(int64(1234), nil) (or change the production call to FileSize(), but Size(ctx) is the SeekableSource interface contract and is what other tests in the file already expect for similar code paths).

  Extended reasoning...

  What's broken

  In packages/orchestrator/pkg/sandbox/template/peerserver/seekable_test.go:20, the test sets up the mock as:

  diff := buildmocks.NewMockDiff(t)
  diff.EXPECT().FileSize().Return(int64(1234), nil)

  But the production code in packages/orchestrator/pkg/sandbox/template/peerserver/seekable.go:21-23 calls Size(ctx), not FileSize():

  func (f *seekableSource) Size(ctx context.Context) (int64, error) {
      return f.diff.Size(ctx)
  }

  build.Diff is an interface that has both methods (Size(ctx) (int64, error) from the embedded storage.SeekableReader, plus a separate FileSize() (int64, error) method). The test mocks the wrong one of the two.

  Step-by-step proof

  Reproduced directly on the PR branch:

  $ go test -run TestSeekableSource_Size ./packages/orchestrator/pkg/sandbox/template/peerserver/
  --- FAIL: TestSeekableSource_Size (0.00s)
      mock.go:361:
          assert: mock: I don't know what to return because the method call was unexpected.
              Either do Mock.On("Size").Return(...) first, or remove the Size() call.
              This method was unexpected:
                  Size(*context.cancelCtx)
              at: [...mockdiff.go:411 ...seekable.go:22 ...seekable_test.go:28]
      mockdiff.go:24: FAIL:	FileSize()
              at: [...mockdiff.go:260 ...seekable_test.go:20]
      mockdiff.go:24: FAIL: 0 out of 1 expectation(s) were met.
              The code you are testing needs to make 1 more call(s).
  FAIL
  FAIL	github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/template/peerserver	0.017s
  

  Walking through the call:

  1. src.Size(t.Context()) is called (seekable_test.go:28).
  2. That dispatches to seekableSource.Size, which calls f.diff.Size(ctx) (seekable.go:22).
  3. The mock has no expectation for Size — only for FileSize() — so testify panics: Size(*context.cancelCtx) is unexpected.
  4. After panic recovery, mock cleanup reports the unmet FileSize() expectation as a second failure.

  Why this matters / impact

  CI-breaking. The PR's own diff (visible in this review) replaced the previously-correct Size(mock.Anything) expectation with FileSize(), but the production code at seekable.go:22 was not changed to match. This appears to be an in-flight refactor that was committed half-done.

  Fix

  The simpler/safer fix is to update the test to match the production interface contract. Size(ctx context.Context) (int64, error) is part of the storage.SeekableReader interface that build.Diff embeds and is what the rest of the codebase uses for context-cancellable size lookups. Change line 20 to:

  diff.EXPECT().Size(mock.Anything).Return(int64(1234), nil)

  Alternatively, if the intent is to use the no-context FileSize() here (it returns from the local cache without an RPC), seekable.go:22 needs to be changed to return f.diff.FileSize() and the function signature kept as Size(ctx context.Context) to satisfy the SeekableSource interface (with _ context.Context to silence unused-param). Either way, the test and the production code must agree.

[levb]

@cla-bot check

[cla-bot]

We require contributors to sign our Contributor License Agreement, and we don't have @github-actions[bot] on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check'

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[tvi]

@cla-bot check

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[dobrac]

@cla-bot check

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7e65a1f6d5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

🟡 The doc comment for IncompletePendingUpload claims it "is in-memory only (never serialized)", but the V4 wire format reserves bit 0 of a dedicated flags byte for exactly this field — serialization_v4.go:111-118 writes it and serialization_v4.go:198 reads it back. peerserver/header.go:46 explicitly forces this bit ON for V4 headers shipped over P2P, which is the wire mechanism by which peers detect an in-flight build. The flag is never persisted to long-term storage (StoreHeader refuses headers carrying it), but it IS serialized in the V4 wire format. Consider rewording to something like "never persisted to long-term storage by StoreHeader, but is serialized in the V4 wire format for in-flight P2P signaling". Documentation only — runtime behavior is correct.

Extended reasoning...

What the bug is

The comment block at packages/shared/pkg/storage/header/header.go:39-44 reads:

// IncompletePendingUpload is set on diff headers produced by ToDiffHeader and
// cleared on the finalized headers swapped in by the upload pipeline. It
// is in-memory only (never serialized), and signals that the builds data
// has not yet reached object storage — readers must serve from the local
// cache and skip FrameTable lookups for the still-missing self entry.
IncompletePendingUpload bool

The phrase "in-memory only (never serialized)" is factually wrong for V4 headers. This is documentation only — runtime behavior is correct — but it will mislead future maintainers about the V4 wire format and the P2P contract.

Step-by-step proof

1. SerializeHeader (serialization.go:8-14) dispatches to serializeV4 for any header with Metadata.Version >= 4, passing h.IncompletePendingUpload through.

2. serializeV4 (serialization_v4.go:111-119) packs the flag into bit 0 of a dedicated flags byte that becomes part of the on-the-wire bytes:

   var flags uint8
   if incomplete {
       flags |= v4FlagIncomplete
   }
   result := make([]byte, metadataSize+v4FlagsLen+v4SizePrefixLen+len(compressed))
   copy(result, metaBuf.Bytes())
   result[metadataSize] = flags

   v4FlagIncomplete = 1 << 0 (serialization_v4.go:24).

3. deserializeV4 (serialization_v4.go:131, 198) reads the flag back:

   flags := blockData[0]
   ...
   h.IncompletePendingUpload = flags&v4FlagIncomplete != 0

4. peerserver/header.go:38-47 is the wire mechanism — it explicitly forces this bit ON for every V4 header it serves to a peer, regardless of the in-memory state:

   wire := *h
   if wire.Metadata.Version >= header.MetadataVersionV4 {
       wire.IncompletePendingUpload = true
   }
   data, err := header.SerializeHeader(&wire)

   This is the entire mechanism by which a peer learns the origin upload is still in flight.

Why the comment is technically true only for the storage path

StoreHeader (serialization.go:62-67) refuses to persist a header with the flag set:

if h.IncompletePendingUpload {
    return fmt.Errorf("refusing to persist incomplete header for %s", path)
}

So the flag never lands in long-term object storage — that is the only sense in which it is "not serialized". But the V4 wire path absolutely encodes it.

Impact

Documentation-only — the runtime is correct. But the comment will mislead anyone trying to understand:

• the V4 wire format,
• the P2P → "switch to storage" handshake,
• why peerserver/header.go mutates the field before serialization,
• why V3 doesn't need this and V4 does.

A future maintainer trusting the "never serialized" claim might remove the flags byte, the peerserver mutation, or the StoreHeader guard, breaking the P2P in-flight signal in subtle ways.

Suggested fix

Reword the comment to reflect the actual contract, e.g.:

// IncompletePendingUpload is set on diff headers produced by ToDiffHeader and
// cleared on the finalized headers swapped in by the upload pipeline. It is
// never persisted to long-term storage (StoreHeader refuses headers carrying
// it), but it IS serialized in the V4 wire format (bit 0 of the flags byte)
// to signal in-flight builds over P2P. Readers seeing this bit set must serve
// from the local cache and skip FrameTable lookups for the still-missing self
// entry.
IncompletePendingUpload bool

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (2) 📎 Requirement gaps (0)

Issue findings (3)

Findings have been published as inline review comments

 ⓘ Findings that repeat in the code will be published as multiple comments