Skip to content

feat(storage): opt-in eager merge so the first restore can be a direct download - #267

Merged
LouisHaftmann merged 3 commits into
devfrom
feat/eager-merge
Sep 13, 2026
Merged

LouisHaftmann merged 3 commits into
devfrom
feat/eager-merge

Conversation

@LouisHaftmann

@LouisHaftmann LouisHaftmann commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator

📝 Summary

With ENABLE_DIRECT_DOWNLOADS the first restore of every cache entry is still proxied through the server, because the merge only runs on the first download and a signed URL needs the merged object. Contributor jrarmstro raised this in #262 with a fork that composes parts on S3 via UploadPartCopy.

This adds EAGER_MERGE (default false). When on, the merge starts right after upload completion. On S3 it runs inside the bucket when every part but the last is at least 5 MiB, none is above 5 GiB and there are at most 10,000 parts. On GCS it runs inside the bucket via compose with no size limits. Otherwise, and always on the filesystem driver, the existing streaming merge runs immediately. The strategy is chosen up front from the part sizes listed at completion, no exception-driven fallback. Completion waits only for the merge lease, not the merge.

Lazy and eager merges now share one lease-fenced runner, so ADR-0002 and ADR-0004 apply unchanged. ADR-0009 documents the decision and the costs. Buildx uploads 1 MiB blocks and always takes the streaming path.

📦 Changes

  • EAGER_MERGE env var, Helm config.eagerMerge (chart 1.4.0)
  • StorageAdapter.composeParts on S3 (CreateMultipartUpload / UploadPartCopy / CompleteMultipartUpload, aborted on failure) and GCS (compose, 32 sources per call folded through a top-level temp object)
  • StorageAdapter.listFolder replaces getFolderSize; upload completion does one LIST for part count, size and per-part sizes
  • Shared startMerge runner used by the download path and the eager path
  • ADR-0009, glossary terms Eager Merge and Server-side Merge

✅ Verification

  • pnpm type-check: pass
  • pnpm lint: pass
  • pnpm test:run (filesystem + sqlite): 43 passed, 4 skipped
  • VITEST_STORAGE_DRIVER=s3 pnpm test:run (minio + sqlite): 45 passed, 2 skipped, compose path covered
  • VITEST_STORAGE_DRIVER=gcs pnpm test:run (fake-gcs-server + sqlite): 44 passed, 3 skipped, compose and 33-part fold covered

🔍 Reviews

Review Found Fixed
code-review (standards) 7 4
code-review (spec) 6 3
ponytail-review 3 3

Unfixed, out of scope: countFilesInFolder could become listFolder().length (pre-existing, many test callers); catch (err: any) matches five pre-existing sites; a warn log when eviction removes the location before the eager merge lease is taken (rare, harmless); the compose limits could be named constants.

🔗 Issues

Closes #262

⚠️ Needs attention

  • Eager merge doubles storage per entry until cleanup:parts runs, for every entry, not only downloaded ones. sizeBytes still counts parts only.
  • A worker killed mid-compose leaves an incomplete multipart upload on S3. Operators should set an AbortIncompleteMultipartUpload lifecycle rule. On GCS it leaves a top-level temp object that orphaned-storage cleanup reclaims.
  • The filesystem driver has no server-side copy and always streams. Eager merge doubles disk use there for little gain, since it has no direct downloads.
  • The env var reference on gha-cache-server.falcondev.io lives outside this repo and needs EAGER_MERGE added.

🤖 Generated with Claude Code

LouisHaftmann and others added 3 commits September 13, 2026 13:48
Created with AI. Verified by a human.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
With EAGER_MERGE=true the merge starts right after upload completion instead of on the first download, so the first restore can be a direct download. On S3 the merge runs inside the bucket via UploadPartCopy when every part but the last is at least 5 MiB; otherwise the existing streaming merge runs immediately. Lazy and eager merges share one lease-fenced runner. Upload completion lists the parts folder once to derive part count, size and per-part sizes.

Refs #262

Created with AI. Verified by a human.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Folds up to 32 sources per call into a top-level temp object and composes `merged` in one final call. No minimum part size, so GCS never falls back to streaming for size reasons.

Created with AI. Verified by a human.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@LouisHaftmann
LouisHaftmann merged commit 19f1ae3 into dev Sep 13, 2026
24 checks passed
@LouisHaftmann
LouisHaftmann deleted the feat/eager-merge branch September 13, 2026 13:18
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.

Avoid proxying first cache hit via eager merge

1 participant