refactor: restructure major crates part 1 #461
Conversation
nyannyacha
force-pushed
the
refactor-dec-2
branch
from
97b9986 to
4576c09
Compare
nyannyacha
force-pushed
the
refactor-dec-2
branch
from
4576c09 to
d3ab51e
Compare
| @@ -495,6 +499,7 @@ globalThis.bootstrapSBEdge = (opts, ctx) => { | |||
| "beforeunload", | |||
| "unload", | |||
| "unhandledrejection", | |||
| "drain", | |||
There was a problem hiding this comment.
What's the difference between beforeunload and drain event? When should a user function listen to drain over beforeunload?
Is this an event triggered in Deno runtime as well?
There was a problem hiding this comment.
Is this an event triggered in Deno runtime as well?
No, it's an edge-runtime specific event.
If the resource exceeds the soft limit, the worker is marked as the retired state, and the worker pool no longer relays new requests to this worker, but the request queue may still contain requests.
When all remaining requests have been processed, the drain event is triggered. This is used to safely remove the task of the http listener from the event loop. This is a different event from beforeunload.
There was a problem hiding this comment.
Is it expected to be used by user functions? It feels more like an internal event if it only used for a specific purpose.
There was a problem hiding this comment.
Yeah, basically it has a purpose to be used internally.
I could have replaced it with a direct function call instead of firing an event, but I just implemented it as an event firing, thought some users might use it.
There was a problem hiding this comment.
I'm a bit concerned it may be confusing for users - when to use drain vs beforeunload and the order of them firing. Can an event handler on drain block the unload of the worker?
There was a problem hiding this comment.
The threshold for the beforeunload trigger is specified by the cli flag. (The default is 90%)
drain indicates that there will be no more requests, and beforeunload indicates that the worker will soon be unloaded because the resource has exceeded the threshold specified by the cli flag.
Can an event handler on drain block the unload of the worker?
All events will be monitored by the supervisor to prevent cheating by user scripts.
Even if an event handler attempts to prevent termination, it will be defeated by a call to v8's terminate_execution API. This is true for all events triggered by the runtime.
There was a problem hiding this comment.
ok sounds good! We'll probably have to add docs later for different events to make it clear for users.
| @@ -115,12 +122,15 @@ function serve(args1, args2) { | |||
|
|
|||
| try { | |||
| for await (const requestEvent of currentHttpConn) { | |||
| ACTIVE_REQUESTS++; | |||
There was a problem hiding this comment.
what's the purpose of tracking ACTIVER_REQUESTS? Where is it used?
There was a problem hiding this comment.
It was used for debugging, but it seems that I didn't clear it.
I'll open another PR to erase this. (Several branches I'm working on directly depend on this PR's commits, so if I modify this PR, I will have to perform an unnecessary force push.)
|
🎉 This PR is included in version 1.66.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
What kind of change does this PR introduce?
Refactor
Description
This PR refactors the major crates. In part 1, the following points and aspects were considered.
AbortController(AbortSignal).Blocked-by: #458 #459