fix(envoy): use global instance, add signal handlers#4565
Conversation
|
🚅 Deployed to the rivet-pr-4565 environment in rivet-frontend
|
Code ReviewOverall: This PR simplifies the envoy client by introducing a global singleton and OS signal handlers, removing per-actor serverless shutdown tracking. The direction is correct, but there are a few things worth addressing. Issues1.
2. Non-global instances get no signal handlers Signal handlers ( 3. If 4. Removed The original if (ctx.serverless) throw new Error("Already started serverless actor");
ctx.serverless = true;The rename removes this guard entirely. Is there a replacement? Calling 5. SSE abort no longer triggers shutdown The old code called
Now an SSE abort just logs. What's the replacement mechanism? Without a shutdown call, a serverless actor whose SSE connection drops could run until it hits an unrelated timeout. If the ping loop's 6.
7. Removed The old code sent Minor
SummaryThe core direction (global singleton + signal handlers) is sound and simplifies the lifecycle model. The main things to resolve before merging: clarify the guard removal for |
de70131 to
4639183
Compare
6c9af76 to
94f54ba
Compare
4639183 to
fdd0c18
Compare
94f54ba to
ef85f3e
Compare
ef85f3e to
5bcccc7
Compare
fdd0c18 to
17a60a7
Compare
17a60a7 to
85acf51
Compare
5bcccc7 to
8d964a5
Compare
Merge activity
|
8d964a5 to
5fdf3ad
Compare

Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: