-
Notifications
You must be signed in to change notification settings - Fork 4
feat: Add load shedding with canAccept hook #141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Implement early request rejection to prevent request queue buildup during
overload. The new `canAccept` hook allows consumers to implement custom
load detection logic that runs before any worker thread communication.
Key changes:
- Add `canAccept` hook option that receives request context including port
- Add `findAccepting()` method to RoundRobin to iterate all workers
- Add `LoadSheddingError` with 503 status code for rejected requests
- Requests are shed immediately without entering any queue
Usage:
```javascript
const interceptor = createThreadInterceptor({
domain: '.local',
canAccept: (ctx) => {
// ctx contains: hostname, method, path, headers, port
return !isWorkerBusy(ctx.port)
}
})
```
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Document the canAccept hook and LoadSheddingError for early request rejection. Includes examples for memory-based shedding, per-worker inflight tracking, and method-based shedding. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Allow attaching metadata to workers when routing to help identify them
in the canAccept hook. The metadata is passed as a third argument to
route() and is available in the canAccept context as ctx.meta.
Usage:
```javascript
interceptor.route('api', worker1, { id: 'w1', maxLoad: 100 })
interceptor.route('api', worker2, { id: 'w2', maxLoad: 50 })
canAccept: (ctx) => {
return loadMap.get(ctx.meta.id) < ctx.meta.maxLoad
}
```
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
ShogunPanda
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit, but it LGTM.
lib/interceptor.js
Outdated
| if (!port) { | ||
| const route = routes.get(hostname) | ||
| if (!route || route.length === 0) { | ||
| throw new Error(`No target found for ${hostname} in thread ${threadId}.`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're missing handler.onError here like you did on line 60.
Address review feedback: use handler.onError instead of throwing directly when no route is found, for consistency with load shedding error handling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
b0b2f54 to
acaf74c
Compare
Resolved conflicts in: - lib/coordinator.js: Combined metadata support with async wiring - lib/roundrobin.js: Added both kMeta and kReady imports - lib/interceptor.js: Added null check for round-robin when no ready workers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I as a user want to specify the canAccept hook on a server side where I have all the info about the thread, server state, contexts, etc. How it should know that the worker can't accept more request with just a MessagePort instance?
If I want a server to stop accepting requests I need to write some logic in a handler and put it in all other "client" workers. For example if we want to set diff limits for diff stackables: next, node or just get it from watt.json in the future it will be a problem.
How about putting the hook on a server side, calling it when it receives reqs and sending the MESSAGE_ROUTE_UPDATE with the ready set to true/false to resume/pause the traffic.
Of course it's will have some delay and requests will pile up a bit, but I don't see how it can be used in a current state. Maybe I'm missing some usecase how it will be used in platformatic.
| for (let i = 0; i < this.ports.length; i++) { | ||
| const portIndex = (this.index + i) % this.ports.length | ||
| const port = this.ports[portIndex] | ||
| if (canAcceptFn({ ...context, port, meta: port[kMeta] })) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we check the can accept logic on each call it might have some perf impact. I would check it at least every n calls and set a flag to a port[kCanAccept] and then check the flag.
|
I see the
|
In Watt, we can rely on health events to track this, and know with a resolution of ~1s if a service is in trouble. I designed this to be able to use this. I concur that adopting a "flow control" system might be better. Let me iterate. |
Ok, but watt health events are emitted in the main thread. Then you need to propagate this info to all other workers, so they stop sending requests to the unhealthy worker. Or this feature is only for the "coordinator" thread? |
Summary
Implements early request rejection (load shedding) to prevent request queue buildup during overload. This addresses the "early rejection problem" where requests pile up in Node.js's event loop queue, consuming memory and increasing latency.
canAccepthook that runs before routing to workersfalse), immediately return 503LoadSheddingErrorroute(url, worker, meta)to identify workersUsage
Changes
lib/utils.js- AddLoadSheddingErrorclass andkMetasymbollib/hooks.js- AddcanAccepthook supportlib/roundrobin.js- AddfindAccepting()method with metadatalib/interceptor.js- Add load shedding check before dispatchlib/coordinator.js- Accept metadata inroute()index.js- ExportLoadSheddingErrorREADME.md- Full documentation with examplestest/load-shedding.test.js- 12 tests covering all functionalityTest plan
🤖 Generated with Claude Code