fix(filler): close the ship socket before a hard exit - #198
Merged
Conversation
The filler leaves on process.exit(1) from an unhandled rejection, an uncaught exception, and both watchdog paths, and none of them closed the state_history websocket first. Abandoning a socket the client could have closed leaves the node holding a half-open session until it works out the peer is gone, so a filler restarted straight after a crash opens a second session beside the first. A repeating foreign-key crash showed the shape of it in the field: an immediate restart crawled at the live chain rate while the same restart with a short pause ran at full speed. The node half of that is inference rather than something measured, but the abandoned socket is wrong on its own terms. Both watchdog paths already pause before exiting, so they take the full graceful stop and the close frame leaves while the process is still alive. The crash handlers cannot await anything safely, since a wedged reader is exactly what they exist to escape, so they send the frame and exit on a short fixed timer instead. Signed-off-by: Rob Konsdorf <rob@facings.io>
There was a problem hiding this comment.
Pull request overview
Ensures filler shutdown paths close the state-history websocket before terminating.
Changes:
- Gracefully stops readers before watchdog exits.
- Adds delayed crash-handler exits after closing SHIP sockets.
- Tests successful and rejected reader shutdowns.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/filler/filler.ts |
Adds safe reader shutdown before watchdog exits. |
src/filler/filler.test.ts |
Tests shutdown success and failure handling. |
src/bin/filler.ts |
Closes active SHIP sockets before crash exits. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Merged
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
The filler leaves on
process.exit(1)from an unhandled rejection, an uncaught exception, and both watchdog paths, and none of them closed thestate_historywebsocket first. Abandoning a socket the client could have closed leaves the node holding a half-open session until it works out the peer is gone, so a filler restarted straight after a crash opens a second session beside the first.A filler caught in a crash-restart loop shows the shape of it: an immediate restart ran at the live chain rate with
[DS:0|SH:0], while the same restart after a short pause ran at full speed withSHclimbing into the twenties. The node half of that is inference and not something measured here, and transient node load is not ruled out. The abandoned socket is wrong on its own terms regardless, which is what this fixes.Both watchdog paths already pause before exiting, so they take the full graceful stop and the close frame leaves while the process is still alive. The crash handlers cannot safely await anything, since a wedged reader is exactly what they exist to escape, so they send the close frame and exit on a short fixed timer instead.
Validation
pnpm testgoes from 419 passing to 421, 37 pending unchanged, with the two added tests covering the new seam: that it stops the reader, and that a rejection from the stop path cannot hold up the exit.pnpm check-typesandpnpm lintare clean. The crash-handler path insrc/bin/filler.tsis not unit tested, because the module forks a cluster at import time; the change there is confined to the close call and the timer.