fix(engine): push node context only to reachable nodes, with bounded concurrency - #442
Merged
Merged
Conversation
…concurrency Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Devin Review found 1 potential issue.
3 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
Row order is not a priority order. With fan-out bounded to a few in-flight sends, http_push targets that hold a slot until their timeout could delay healthy WebSocket nodes behind them. Queue every WebSocket push (a millisecond Durable Object call) before any http_push POST, so a slow HTTP target can only delay other HTTP targets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Problem
Production workspace
rw_7ccfea89kept returning429 workspace_busyon channel writes, agent events and spawn channel joins.Admission allows two in-flight write leases per lane per workspace (
RATE_LIMIT_DO, keywrite-admission-v1, 120s TTL). The worker releases each lease at the end of the request. A production tail showed that everyPOST /v1/channels/{general,engineering}/{join,mute}in this workspace lost its release:The write-admission Durable Object answered every other call within 0–20ms and never received these releases. Once two leases leaked, the write lane stayed at
active_leases: 2and rejected every write for up to 120s (oldest_lease_age_msclimbing from 59s to 117s). Over 14 minutes: 121 write-lane and 39 control-lane rejections, plus 11 leaked leases, all from join/mute or a follow-up PATCH.Cause
sendNodeContextForChannel,sendNodePresenceContextand the agent-scoped variants pick targets onagent_node_bindings.status = 'active'alone. They never check whether the node is live. They then push to every target at once withPromise.allSettled.In this workspace:
So one join sent thousands of
NODE_DOfetches in the background of the request. An offline node has no socket, so every one of those fetches fails. They filled the invocation's concurrent-connection budget, and the lease release waited behind them until its 2s timeout. Forgeneralthey also pushed the isolate past its memory limit.Fix
offline, plushttp_pushnodes, which have no liveness status.drainingnodes keep their socket until they disconnect, so they stay eligible.context.updateis ephemeral and never replayed, and a push to an offline node already failed, so this drops no event that was delivered before.sendContextToRowsruns at mostNODE_CONTEXT_SEND_CONCURRENCY(4) pushes at once, so even a large live audience leaves connections free for the request's own subrequests. Failures are still collected and reported as oneAggregateError.This ships to hosted after an engine release and a
relaycast-cloudengine bump. Both are human-gated.Tests
eventDispatch.test.ts: channel, presence, and single and batched agent-scoped fan-out skip offline ws nodes and keep draining ones. Pushes stay bounded (at most 4 in flight) and still reach all 13 live nodes. Failure reporting is unchanged under the bound.origin/main'snodeContext.tsand pass with this change.packages/engine:vitest run99 files / 1119 tests pass,tsc --noEmitclean, eslint clean.npm run test:engine:regression: 814 PASS, 0 FAIL.🤖 Generated with Claude Code
Note
Medium Risk
Changes real-time fan-out semantics for channel/presence/agent context on hosted workspaces; behavior is intentional (skip offline WS) but could surprise operators who relied on failed pushes to offline nodes.
Overview
Node
context.updatefan-out no longer targets every active binding and is capped so hosted requests can finish their own subrequests (e.g. write-admission lease release).Channel, presence, and agent-scoped context pushes now resolve targets with a reachability predicate: WebSocket nodes must not be
offline(drainingstays eligible);http_pushnodes remain eligible. Offline sockets were never deliverable for ephemeralcontext.update, so this removes wasted Durable Object round-trips in workspaces with thousands of stale offline nodes.Delivery uses bounded concurrency (
NODE_CONTEXT_SEND_CONCURRENCY= 4) via lazy task pooling instead of unboundedPromise.allSettled. WebSocket pushes are scheduled beforehttp_pushPOSTs so slow HTTP targets do not occupy slots ahead of live sockets.AggregateErrorbatch failure reporting is unchanged.Tests in
eventDispatch.test.tscover offline skipping, draining inclusion, concurrency bounds, WS-before-HTTP ordering, and failures under the cap. Changelog documents the productionworkspace_busymotivation.Reviewed by Cursor Bugbot for commit 7352d8e. Bugbot is set up for automated code reviews on this repo. Configure here.