Repository navigation
fix: bound memory used by docs menu builds - #267
Merged
Merged
Conversation
Stream tarball decompression instead of inflating the whole repo into memory, cap concurrent menu builds at 4, and time out GitHub fetches. Crawler traffic across many versions while GitHub was slow caused unbounded concurrent builds that OOM-crashed every production machine.
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.
What problem is this PR solving?
JavaScript heap out of memoryuntil Fly stopped restarting them, and docs pages returned Fastly 503s.Note
Root cause: each menu build (one per docs version) downloads the full repo tarball and inflated all of it into one buffer. Crawler traffic was hitting docs for dozens of old versions (about 90 menu fetches in 100s), and GitHub was slow (about 4.5s per request), so many builds ran at the same time and filled the heap. Each crash emptied the in-memory caches, so after a restart every request missed the cache again. Restarting couldn't break that loop. Nothing had been deployed since Oct 22.
Changes:
tarball.ts(zlib.createGunzip()→parseTar) instead of inflating the whole archive up front.docs.ts. Extra builds wait in a queue, andlru-cachealready merges concurrent fetches for the same key.Local benchmark (20 concurrent builds of the
maintarball × 8 rounds, after GC):In production the concurrency cap keeps actual usage well below this.
Warning
Temporary mitigation in place: production was scaled from 1 GB to 2 GB per machine (
fly scale memory 2048 -a reactrouter) to stop the outage. Once this PR is deployed and stable, scale back withfly scale memory 1024 -a reactrouter.🎩 Tophatting
fly logs -a reactrouterwhile browsing docs for several versions (e.g./6.4.5/start/overview,/7.6.0/start/modes) → pages render, and you should see noFATAL ERROR ... heap out of memory.fly ssh console -a reactrouter -C 'sh -c "ps -o etime,rss,args | grep [n]ode"'after a few minutes → RSS stays well under 1 GB (it was 1.6–1.9 GB within ~3 minutes during the outage).Generated by an AI agent on behalf of Brooks Lybrand. Model: Shopify Auto Best (shopify/auto-best); reasoning: unknown.