[rush-daemon] Publish coalesced phased results as soon as each client's own selection completes - #6092
Conversation
… selection completes A coalesced SHARED-BUILD participant now receives its result as soon as every operation in its own selection has completed and its output has drained, while the iteration and leases continue for the remaining participants. The last participant keeps the result-after-lease-release contract. Fixes #6084 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…tstanding Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Full
Neither test goes through a coalesced batch. They are single-request paths, and this PR leaves the single-request contract unchanged (the last participant still gets its result after the lease is released). They look like pre-existing load-sensitive timeouts in real-native-build tests, similar to the |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The lifecycle, ordering, cancellation, failure isolation, and lease-release behavior are consistently implemented and covered by focused tests.
Review effort: Balanced
Findings: None
What changed in this PR
Enables coalesced Rush daemon clients to receive results when their own operations settle, avoiding head-of-line blocking.
Changes:
- Tracks per-client operation settlement and safely publishes early results.
- Preserves execution-lease behavior for the final participant.
- Adds coverage for early success, failure, cancellation, and ordered output.
| File | Description |
|---|---|
libraries/rush-daemon/src/PhasedRequestRouter.ts |
Coordinates early result production and lease release. |
libraries/rush-daemon/src/PhasedRequestEventSink.ts |
Detects when client-selected operations settle. |
libraries/rush-daemon/src/test/PhasedRequestBatching.test.ts |
Tests early-result batching behavior. |
libraries/rush-daemon/src/test/PhasedRequestEventSink.test.ts |
Tests settlement tracking and abort suppression. |
libraries/rush-daemon/src/test/PhasedRequestRouterTestUtilities.ts |
Adds execution-lease test instrumentation. |
libraries/rush-daemon/README.md |
Documents revised batching and lease guarantees. |
common/changes/@rushstack/rush-daemon/rushd-coalesced-early-result_2026-09-24.json |
Records the patch release change. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
When the daemon coalesces compatible
SHARED-BUILDrequests into one graph iteration, each client now gets its final result as soon as every operation in its own selection has completed and its output has drained. Before this change, every client waited for the whole union iteration. The iteration, graph lease, and native execution lease keep running for the other participants.Fixes #6084
Root cause
PhasedRequestBatchCoordinator.#executeBatchAsyncawaitedexecuteScheduledIterationAsync()for the whole union before it called#finishEntryAsyncfor any entry. ThencreateBatchReleaseBarriermade every entry's result wait for the native execution lease release, and that release only happened after all entries arrived. So a client that asked for--to p04sat idle until an unrelated coalesced--to p16finished.Fix
PhasedRequestEventSink: new optionalonActiveOperationsSettledcallback.onIterationScheduledrecords which of the client's active operations are scheduled and not yet terminal.onOperationCompletedremoves each one. rush-lib emits that event fromfinalizeOperation()after the collated writer closes, so the operation's header, chunks, status and stream-closed events are already queued on the client's ordered writer.ABORTED. That covers iteration abort, a failed start, and discarded work, and those results stay on the batch path.PhasedRequestRouter(batch coordinator):#finishSettledEntryunsubscribes the entry and produces its result right away. The result drains the sink first. Outcomes come from the statuses the client observed in the current iteration, because retained results may be from the previous iteration while it runs.#finishEntryAsyncis memoized throughentry.finishPromise, so each entry gets exactly one result. The batch's finalPromise.allalso waits for early results.createBatchReleaseBarriercounts only entries that haven't started finishing.#rejectEntryAsyncwaits for an in-flight finish instead of racing it.#needsIteration(live and not yet finishing) replaces#isEntryLivein the "does anyone still need this iteration?" checks. If every remaining client cancels after others got early results, the iteration is still aborted.Compared with the analysis prototype (
A02-fix-38-early-result.diff):setImmediatedeferral. The completion event already follows all of the operation's output.Compatibility with #6067 and #6065
admissionController.acquireAsynctoacquireGraphExecutionAsyncinenqueueAsync. It doesn't overlap these hunks, so the two compose without changes.finishStarted,#finishDetachedEntry, an in-progress flag oncollectOperationOutcomes, and a barrier that skips entries whose finish already started. The two PRs will conflict textually in those spots, but the semantics compose. Whichever lands second should mergefinishStartedintofinishPromise(or the reverse) and make#needsIterationexclude detached entries. The barrier and outcome changes are the same idea in both PRs.Tests
PhasedRequestBatching.test.ts:EXECUTINGand before the execution lease is released. B's result comes afterreleased. One schedule, A ran once.PhasedRequestEventSink.test.ts: settlement fires exactly once after all scheduled active operations complete (other operations and duplicate completions are ignored). It does not fire when an active operation was aborted.Linux validation (WSL Ubuntu-24.04, node 22)
rush build --to @rushstack/rush-daemon --to @rushstack/rush-cli-client: passes, including lint.heft test --test-path-pattern "PhasedRequest(Batching|EventSink|Router)": 37/37 passing, 3 runs in a row. The fullrush test --only @rushstack/rush-daemonresult will be posted as a comment.labrun: privatemkws-synthworkspace, 16-project chain, 1.5 s per op,p16= 20 s, all sources dirtied, clients started at the same time. "Before" is the unfixed toolchainrush-client; "after" is this branch'srush-client. Load average was about 13–20.--to p04/--to p08/--to p12/--to p16--only p03/--only p09/--to p16All clients exited 0. Per-client output was identical before and after (4/8/12/16 and 1/1/16 "completed successfully" blocks). Each scenario ran 16 executions over 16 unique operations, so there was no duplicate work.
Follow-ups (not in this PR)
BLOCKEDdependent. rush-lib only finalizes blocked records at iteration end, so that client waits for the iteration as before.This came out of the automated rushd Linux analysis ("Rushd Hive", bug #38, prototype by analysis agent A02).