Repository navigation
fix: close the newest capture when memory nears the container limit - #26
Conversation
One heavy page (a 7000x7000 image drawn by MapLibre in software WebGL) pushed Chromium past the 2GiB limit in production. The kernel then killed the whole container, failing every capture in flight on it, and the worker's retry did the same to the other replica.
🔵 Tier A · Mergeable after minor fixes
The pull request adds a cgroup-v2 working-set guard that tracks screenshot and Lighthouse captures, closes the newest context when usage exceeds a configurable budget, and reports that capture as a 507; the README documents the setting and response. It adds constrained-container recovery coverage and updates SSRF fixtures to use httpbin.dev. The latest commits simplify the guard constructor, fix polling and settle intervals at their defaults, and remove the standalone memory unit tests. Latest changes: The latest commit replaces the options-object constructor with positional arguments, fixes polling and settling at 100 ms and 1 s, and drops the memory guard unit tests.
Fix with agent prompt### Issue 1
src/utils/memory.ts:91
**[minor] Retain multi-capture guard coverage**
The latest commit removes the configurable polling and settle intervals and drops `tests/unit/memory.test.ts`; the remaining recovery test exercises only one capture, so newest-first selection and waiting before closing another capture are no longer asserted. Could we keep equivalent automated coverage, using production timings or fake timers?
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.📂 Walkthrough · 6
Reviewed the commits since |
Docker Image Stats
Screenshot benchmark: Average of 3 runs on https://appwrite.io |
httpbin.org now refuses redirects to internal addresses and serves /base64 as text/plain with nosniff, so the browser never attempted the private hop and the four redirect and script tests captured a 200 instead of a 403.
| this.tracked.push(entry); | ||
|
|
||
| if (this.enabled && this.timer === null) { | ||
| this.timer = setInterval(() => this.check(), POLL_INTERVAL_MS); |
There was a problem hiding this comment.
[minor] Retain multi-capture guard coverage
The latest commit removes the configurable polling and settle intervals and drops tests/unit/memory.test.ts; the remaining recovery test exercises only one capture, so newest-first selection and waiting before closing another capture are no longer asserted. Could we keep equivalent automated coverage, using production timings or fake timers?
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/utils/memory.ts
Line: 91
Comment:
**[minor] Retain multi-capture guard coverage**
The latest commit removes the configurable polling and settle intervals and drops `tests/unit/memory.test.ts`; the remaining recovery test exercises only one capture, so newest-first selection and waiting before closing another capture are no longer asserted. Could we keep equivalent automated coverage, using production timings or fake timers?
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · testing · Reply if this doesn't apply.
Why
On 2026-11-01, both
appwrite-browserpods in fra1 were OOMKilled twice. A customer's Astro site has a MapLibre map backed by a 7000×7000 webp (2.2 MB on the wire, 187 MiB decoded). Headless Chromium has no GPU, so the image is held as decoded pixels in the renderer, MapLibre's buffer, and a software WebGL texture in the GPU process. The worker captures light and dark in parallel, which takes about 1.55 GiB per job.Kubernetes kills the whole container on OOM, so every capture in flight failed. The worker's retry then took down the other replica:
What
src/utils/memory.tspolls the container's working set (memory.current − inactive_file, the figure the OOM killer and kubelet use) every 100 ms while captures are in flight. When it passesMEMORY_BUDGET×memory.max(default0.85), it closes the newest capture's context, waits 1 s for memory to drop, and checks again. That capture returns507 {"error":"Page exceeded the memory budget and was closed."}. Everything else keeps running.Memory can't be measured per page because all pages share one GPU process, so this works on the container total. Without a cgroup v2 memory limit, the guard is off.
Measured
docker run --memory 2g. Three concurrent captures of the customer page (light, dark, light) plus oneexample.com:example.comLog:
Memory budget exceeded: 1895 of 2048 MiB, closing the newest captureTests
tests/recovery/memory.test.tsappwrite/browser:localwith--memory 1g -e MEMORY_BUDGET=0.05: capture →507+ JSON error,/v1/healthstill 200. Fails on0.3.10(gets 200)This branch includes #27's commit (SSRF fixtures moved to httpbin.dev; httpbin.org now refuses internal redirects), which
mainneeds to pass CI. Merge #27 first or close it as superseded.Notes
cloud-browserOOMKill alert should stop firing for this site.