Skip to content

fix: keep the GC insertion marker until the manifest is written - #162

Open
RaphaelFakhri wants to merge 1 commit into
cloudflare:mainfrom
RaphaelFakhri:fix/await-put-manifest-insertion-marker
Open

RaphaelFakhri wants to merge 1 commit into
cloudflare:mainfrom
RaphaelFakhri:fix/await-put-manifest-insertion-marker

Conversation

@RaphaelFakhri

Copy link
Copy Markdown

Summary

R2Registry.putManifest returned this.putManifestInner(...) without await inside a try/finally. The finally block, which calls gc.cleanInsertion, ran right away instead of after the manifest was written. A garbage collection that started during the push saw no insertion marker and deleted the layers of the manifest being pushed, which left a manifest pointing at missing blobs.

This change awaits putManifestInner, so the insertion marker stays in place until the manifest objects are written. This matches GarbageCollector.collect, which already uses return await for the same pattern.

Testing

  • Adds garbage collector > keeps the insertion marker until the manifest is written. The test wraps the R2 bucket so that an unreferenced garbage collection runs right before the manifest is written, then checks that the layers still exist and that the GC refused to run.
  • Without the fix, the test fails: expected +0 to deeply equal 2 (both layers deleted).
  • With the fix, pnpm test passes (65 tests), and pnpm run typecheck, pnpm run lint, pnpm run format:check and pnpm run generate-types:check are clean.

Fixes #161

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.

Garbage collection can delete layers of a manifest that is being pushed

1 participant