Repository navigation
perf(daemon): serve Work/Workflow requests off native workers - #4052
Conversation
Drive a workflow registration, a run read, a Work attempt list and a fan-out binding read through the daemon invocation service on a native worker, and assert native_thread_writer_waits() does not move. On master it moves by 21.
Work and Workflow storage ports are synchronous and wait on the SQLite writer by parking their thread. Since #4003 broker-socket connections run on native workers, so every such wait parked a native worker (594 waits across daemon_suite). native_runtime::off_native_thread moves a future from a native thread to one blocking-pool thread and polls it there with a thread-park waker; other callers poll in place. execute_work_application, execute_workflow_application and work_fan_out_binding run through it, so every caller (including guided-job steps that recurse into Work dispatch) leaves the native workers in one place, one pool thread per request. Fixes #4046
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if Runtime::current_handle().is_none() { | ||
| return Box::pin(future.map(Ok)); | ||
| } | ||
| spawn_blocking(move || poll_on_current_thread(future)) |
There was a problem hiding this comment.
🔴 Detached requests outlive project admission
When an invocation is aborted, off_native_thread keeps its handler running after the caller releases its project lease. Project quiescence can then retire the runtime while that handler still uses it.
Learn more
A project request lease keeps its runtime admitted until the invocation finishes. ProjectRequestRuntimesV1 owns that lease in the caller's invocation future, while drain_roots waits for leases before retiring runtimes. The new blocking-pool handoff owns only the Work or Workflow handler. If the caller drops its wait, the lease disappears but the queued or running handler continues. The runtime can therefore retire before the handler completes its storage operation.
Example: A client starts a Work effect, then cancels its invocation while the handler waits on the writer. The invocation releases the project lease; quiescence observes no requests and retires the project. The blocking-pool handler resumes against that retired runtime instead of finishing within its admission.
Recommended fix: Keep a clone of the exact project admission lease with detached Work and Workflow handlers until they finish, or make the handoff cancel and join safely before releasing admission. Cover cancellation during a writer wait followed by project quiescence.
Was this helpful? React with 👍 or 👎 to provide feedback.
Fixes #4046
Work and Workflow requests no longer park native runtime workers on SQLite writer waits. Since #4003, broker-socket connections run on native workers. Each Work or Workflow request then called synchronous storage ports on that worker, and every writer wait parked it. Across
//crates/tracedecay:daemon_suitethat was 594 parked waits on current master. With this change it is 0.What changed
native_runtime::off_native_thread(future)(tracedecay-rusqlite-runtime) is new. Called from a native runtime thread, it moves the whole future to one blocking-pool thread and polls it there with a waker that unparks that thread. Called from any other thread, it polls the future in place. Native wakes reach the pool thread through an ordinaryWaker, so the handler's awaits keep working.execute_work_applicationandexecute_workflow_applicationrun their whole handler through it. This is option (b) from the issue: the dispatch boundary, with no change to any storage port contract. Every caller goes through these two functions, including guided-job steps that recurse into Work dispatch. On the pool thread that recursion runs in place, so a request uses at most one pool thread.work_fan_out_bindingis called while draining retained invocation responses inconnection_serving. It runs its one port call through the same helper.Layer choice
Two design candidates were compared.
dispatch_work_applicationinto a sync dispatcher on the pool plus async heads for its five async arms. This needs surgery in leak adjudication, evidence retrieval, experience and guided jobs.Per-call wrapping, option (a), would have meant dozens of sites and a pool hop for every port call. Async ports, option (c), would have changed contracts across four crates.
native_runtime::block_onon the pool thread was ruled out: it makes the thread count as native again.Behavior notes
Unavailableand the failure is logged.recv_writer_replyguardThe detector and the
native_thread_writer_waitscounter stay. Section 4 of the T2 scope plans to delete them after a full suite run reads zero. Since that plan was written, about a dozen tests across daemon-service, dashboard-api, the CLI and the new test below have started using the counter as their check. #4046 also asks for a test that holds the counter at zero. Deleting the counter would leave all of those tests with nothing to assert. Only the doc pointer changed, which now namesoff_native_thread.Measurements
Hazard counts come from #4045's instrumentation, which logs every native-thread writer wait with its frames. It ran on a scratch build and is not committed. The run used the full
//crates/tracedecay:daemon_suite, with child daemons inheriting the log path.f9c2cf26afWorkflowEffectAuthorityPortV1WorkflowRunStoragePortWorkRunControlStoragePortWorkProductAttemptAdmissionPortV1WorkflowFanOutCensusStoragePortWorkProductEventPortV1WorkAttemptStoragePortWorkRetryStoragePortV1WorkflowDefinitionAuthorityPortWorkflowArtifactStorePortPaired latency used the #4003 method. Release builds of base
f9c2cf26afand this branch ran as two private daemons side by side, each with an isolated HOME. Each of 8 workers sent every request to both daemons in alternating order, one CLI process per call. That gave 240 pairs per operation over a workflow-heavy mix, at host load 30 to 33.workflow_register_definitionworkflow_list_definitionsworkflow_get_runwork_list_attemptsThe fix-over-base ratios fall inside the base-over-base control (load 28), so the pool hop adds no measurable per-request cost. CLI process startup takes most of each call, so this probe cannot resolve sub-millisecond differences.
advanced_workflow_journey_test::mounted_fan_out_recovers_then_synthesizes_and_hands_offfailed under load in #4045's gate. Here it was stressed with--runs_per_testand the same number of--local_test_jobs.Tests
daemon::invocation_tests::work_tests::work_and_workflow_dispatch_never_park_a_native_worker_on_the_writer. On a native worker it drives a workflow definition registration, a run read, a Work attempt list and a fan-out binding read throughDaemonInvocationService. It asserts that the counter does not move. On master it fails withleft: 21, right: 0. It passes with the fix.native_runtime::tests::off_native_thread_moves_a_native_caller_and_keeps_its_awaits_working. A native caller is moved off its thread, and native timers andspawn_blockingstill complete from the pool thread. A plain thread polls in place.Gates
At
43569e8a41://crates/tracedecay:unit_testPASSED (896 tests).:daemon_suitePASSED.:mcp_suitePASSED.//crates/tracedecay-rusqlite-runtime:unit_testPASSED.//crates/tracedecay-daemon-service:unit_testPASSED.bazel build --config=clippypassed for//crates/tracedecay-rusqlite-runtime:all,//crates/tracedecay-daemon-service:alland//crates/tracedecay:all.rustfmt_check_2024passed for all three crates.unit_testrun at host load 86 failedscheduler_config::concurrent_reenable_creates_one_live_scheduler_owner, an automation scheduler test this change does not touch. The rerun passed 896 of 896.