fix(sync): admit independent SWM and VM responder lanes - #2282
fix(sync): admit independent SWM and VM responder lanes#2282branarakic wants to merge 2 commits into
Conversation
| }; | ||
|
|
||
| const preAuthorizationScheduling: SyncResponderScheduling = { | ||
| plane: isWorkspace ? 'shared_memory' : 'durable', |
There was a problem hiding this comment.
🟡 Issue: Centralize the request-to-plane mapping instead of repeating it in scheduling branches
What's wrong
This PR introduces plane as a new limiter dimension, but the canonical mapping from request shape to plane is scattered across the handler. That makes the scheduling contract harder to audit and gives future edits multiple places to drift. The code can preserve behavior while deleting those repeated branches.
Example
A future handler path or scheduling tweak now has to remember that every stage for the same request must use the same derived plane. That invariant is not named anywhere; it is reconstructed three times from isWorkspace.
Suggested direction
Name the plane once per request, then reuse that value for pre-auth, two-stage responder, and non-priority scheduling. A cleaner version would keep capacity dimensions like plane separate from the priority scheduling literals so callers do not rebuild the limiter’s internal model by hand.
For Agents
In packages/agent/src/sync/responder/sync-handler.ts, derive a single responderPlane after isWorkspace is computed and feed it into the limiter. Prefer keeping plane ownership separate from priority/lane scheduling, or add a small scheduling factory if the object literals remain. Preserve the current VM/SWM concurrency behavior and keep the mixed-plane admission test passing.
| // Store-backed graph planning routinely takes tens of seconds for large CGs. | ||
| // Keep bounded admission, but do not reject a healthy queued page before one | ||
| // legitimate predecessor can finish. Request aborts still remove stale work. | ||
| const SYNC_RESPONDER_MAX_QUEUE_WAIT_MS = 60_000; |
There was a problem hiding this comment.
🟡 Issue: The longer responder queue wait is not covered by a regression test
What's wrong
The PR changes user-visible backpressure behavior by allowing queued responder work to wait much longer before failing, but the test changes only exercise plane concurrency. Without a regression test around the old and new timeout boundaries, this behavioral fix can silently regress while the suite remains green.
Example
A regression that accidentally restores SYNC_RESPONDER_MAX_QUEUE_WAIT_MS to 10_000 would still pass the added tests. A focused test could queue a second same-peer same-plane request behind a blocked first request, advance fake timers past 10,000 ms and assert it is still pending, then advance to 60,000 ms and assert it rejects with sync responder queue wait exceeded.
Suggested direction
Add a timeout-boundary test for the responder limiter so the changed admission window is part of the verified contract.
For Agents
Look in packages/agent/test/sync-responder-protection.test.ts. Add a fake-timer regression test for the responder limiter timeout: one admitted same-plane request blocks, another queues, the queued request must survive the old 10s wait and only time out at the new 60s limit unless released/aborted first.
2 participants
User impact
A node serving a large SWM catch-up no longer blocks unrelated VM recovery behind the same responder slot. SWM and VM keep separate bounded admission lanes, while expensive plan construction gets a bounded allowance instead of being mistaken for an immediate transport failure.
Before
sequenceDiagram participant SWM as SWM receiver participant VM as VM receiver participant R as Provider responder participant S as Store SWM->>R: Large selected SWM request R->>S: Build and read plan VM->>R: Exact VM request Note over VM,R: Waits behind the SWM lane R-->>VM: Timeout or late responseAfter
sequenceDiagram participant SWM as SWM receiver participant VM as VM receiver participant R as Provider responder participant S as Store SWM->>R: Selected SWM request VM->>R: Exact VM request par Bounded SWM lane R->>S: Build and read SWM plan and Bounded VM lane R->>S: Build and read exact VM plan end R-->>SWM: Paged response R-->>VM: Exact responseSafety
Validation
Focused responder protection tests cover lane independence and bounded planning. The complete stack was also exercised by the Testnet Blackbox 500 SWM + 500 VM recovery run; final distributed evidence will be attached to the top PR.