Cache generated images in S3 - #10
Merged
Merged
Conversation
Rendering an image costs a few seconds of headless Chrome at 2GB, and until now we paid that on every cache miss. Edge caches can't prevent that on their own: they're per-PoP, they evict the long tail, and a flood of requests for distinct URLs misses them entirely. July's bill showed what that costs when someone points a bot at it. Write each generated image through to S3 so a given URL is only ever rendered once. Cost becomes a function of how many images exist rather than how many times they're requested. Reads and writes both fail soft, so a missing bucket or policy degrades to today's behaviour rather than breaking image generation. Timestamped URLs are immutable, so their stored copy is used forever. Legacy untimestamped URLs point at mutable content, so a stored copy is only reused for 24h — matching the Cache-Control we already return. Also set explicit navigation and selector timeouts. puppeteer defaults both to 30s, which is longer than the Lambda's own 20s timeout, so a hung render burned the full 20s at 2GB rather than failing fast. Since a render can now fail instead of taking the container down with it, the browser is closed in a finally block so it can't leak into the next warm invocation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018iQj4EdMfNsWP6NtFTqwUU
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
image_generatorwas ~$455 of July's $459 Lambda bill (836,207 invocations). Most of that was the Cloudflare outage, now fixed — but the underlying shape is the real problem: cost scales with requests, not with how many images exist.Edge caches can't fix that on their own. Cloudflare and CloudFront are both per-PoP, they evict the long tail (most preview images are fetched a handful of times ever, so each PoP refills from scratch), and a flood of requests for distinct URLs misses them entirely. That last part is not hypothetical — it's what late July's bot traffic did.
The decisive number is what a miss costs:
Roughly 300x. Writing through to S3 means a given URL is only ever rendered once, so cost becomes O(images) instead of O(requests) — and a bot wave stops being able to run the bill up.
What this does
-{timestamp}.jpg) are immutable, so the stored copy is used indefinitely. Legacy untimestamped URLs point at mutable content, so a stored copy is only reused for 24h — matching theCache-Controlwe already hand the CDN.Also included, because they're the same cost story:
browser.close()moved into afinally. Now that a render can fail rather than taking the container down with it, the browser would otherwise leak into the next warm invocation.Bucket choice
Defaults to
exercism-v3-assets— it's ineu-west-2alongside the Lambda (no cross-region latency or transfer), it's a general-purpose assets bucket, and it already carries ascreenshots/prefix.exercism-assets,exercism-staticandexercism-uploadsare alleu-west-1;exercism-v3-iconsis the right region but semantically wrong for generated user content.Everything is env-configurable (
IMAGE_BUCKET,IMAGE_KEY_PREFIX,NAVIGATION_TIMEOUT_MS,SELECTOR_TIMEOUT_MS), so overriding is a one-line change.Required before this does anything
The Lambda's execution role needs
s3:GetObjectands3:PutObjectonarn:aws:s3:::exercism-v3-assets/generated-images/*. That's a separate terraform change.This is safe to merge first — without the policy it fails soft and behaves exactly as it does today, so there's no ordering constraint.
Testing
No test suite exists in this repo, so this is unverified beyond a syntax check and a smoke test of the error path (unmappable path returns 500 without launching Chrome or touching S3). The S3 hit/miss paths have not been exercised against real AWS. Worth a careful look at
fetchFromS3, and worth watching the invocation count drop after deploy to confirm the cache is being populated.