Repository navigation
Retry package-set heap exhaustion with clean compiler output - #815
thomashoneyman wants to merge 1 commit into
Conversation
| isCompilerHeapExhaustion :: CompilerFailure -> Boolean | ||
| isCompilerHeapExhaustion = case _ of | ||
| UnknownError err -> String.contains (String.Pattern "purs: Heap exhausted") err | ||
| _ -> false |
There was a problem hiding this comment.
I'm intentionally classifying the unknown text rather than expanding the compiler failure sum type, as this information is really only needed in niche scenarios and not something every consumer of the compiler failure enum should need to think about
There was a problem hiding this comment.
makes sense. can we replace the (not very useful) docstring with a comment that contains this note?
|
@f-f I deployed this branch before merging and reran the same 60-package update on the server, leaving the existing compiler output in place. It hit heap exhaustion, logged exactly one clean-output retry, and then succeeded. The run took about 20 minutes and published 81.8.0. No new out of memory kills, server restarts, or job resets during the run. So this has now handled the actual production failure, not just the local reproduction. |
a0b5e3c to
9c8503a
Compare
| isCompilerHeapExhaustion :: CompilerFailure -> Boolean | ||
| isCompilerHeapExhaustion = case _ of | ||
| UnknownError err -> String.contains (String.Pattern "purs: Heap exhausted") err | ||
| _ -> false |
There was a problem hiding this comment.
makes sense. can we replace the (not very useful) docstring with a comment that contains this note?
| case result of | ||
| Left err | Purs.isCompilerHeapExhaustion err -> do | ||
| Log.warn "Compiler heap exhausted; clearing package-set output and retrying once." | ||
| -- Keep the backup intact so a failed retry can still roll back. |
There was a problem hiding this comment.
this comment is above a line that removes a folder. why do we talk about backups?
| Spec.it "Recognizes heap exhaustion without treating its empty JSON output as a race" do | ||
| let | ||
| err = UnknownError $ String.joinWith "\n" | ||
| [ "purs: Heap exhausted;" | ||
| , "purs: Current maximum heap size is 3221225472 bytes (3072 MB)." | ||
| , "JSON: Unexpected end of JSON input" | ||
| ] | ||
| Purs.isCompilerHeapExhaustion err `Assertions.shouldEqual` true | ||
| Purs.isCompilerRaceCondition err `Assertions.shouldEqual` false | ||
| Purs.isCompilerHeapExhaustion (UnknownError "JSON: Unexpected end of JSON input") `Assertions.shouldEqual` false | ||
|
|
There was a problem hiding this comment.
I don't think this test is very useful. We are checking that a function that matches a certain string can in fact match the same string.
A more useful test would be to have the compiler actually exhaust the heap and see if we can match that, but that is more setup and may be not worth it.
This branch has not been deployed
2 participants
So, we’ve been getting health check alerts because the compiler runs out of memory while building a package set with 60 updates. The previous change (#813) stopped those failures from taking down the server, but the build still hit the compiler’s memory limit. Surprising, since I had run the build locally in the same circumstances (so I thought) seen on the server!
Unfortunately, my error was to reproduce building the whole set from a clean state, whereas what happens in an actual package set build is that we build the current package set, install the updates, then compile again using the existing output. In a new local reproduction that failed with heap exhaustion. Removing only the compiler output and rebuilding the same sources succeeded within the memory budget.
The compiler loads existing compiled module information before deciding which modules dependency changes force it to rebuild, so an incremental build can have different memory costs from a clean one. I haven’t pinned down the exact source of the extra memory, but clearing the output consistently resolved this build’s failure.
So: in this PR, we detect the explicit heap-exhaustion message and allow one retry with clean output for package-set builds. For typical builds you get an incremental, fast one. For big builds that would exhaust memory, you get a retry. It keeps the rollback backup intact, leaves ordinary package builds unchanged. We were also misclassifying heap exhaustion as a "compiler race” (the unexpected JSON output bug).
The app tests pass. I also ran the full sequence through the updated handler under a 3.5 GiB memory limit. It hit the same error, cleared the output, and successfully rebuilt all 60 updates on its one retry, without using swap. So now I’m feeling confident this will work as expected.
The alternative as always is to host this somewhere with more RAM, but for now, this is a reasonable stopgap IMO.