Repository navigation
fix(daemon-service): write scope-set replicas on the blocking pool - #4068
Conversation
compare_and_swap_scope_set wrote each replica through the synchronous scope-set storage port inside the async request. On a native worker every write parked the worker on the SQLite writer. Run the replica loop on native_runtime::spawn_blocking; the loop still finishes every replica once it starts.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| let next = tracedecay_runtime_core::native_runtime::spawn_blocking(move || { | ||
| for storage in storages { | ||
| match storage.compare_and_swap(expected_revision, &next).ok()? { |
There was a problem hiding this comment.
🔴 Cancelled scope sets commit after admission
When the blocking pool delays spawn_blocking, a cancelled or expired scope-set request still writes every replica. spawn_blocking keeps queued work alive independently of the request, and the worker never rechecks its controls.
Learn more
The request checks its cancellation and deadline immediately before queuing the write, but a blocking-pool worker can start later. The blocking runtime queues work immediately and keeps it alive even if the returned future is dropped. Neither control is checked by the worker before its first compare-and-swap. The request dispatcher can report cancellation or timeout, and in-process settlement can stop waiting while the queued work remains pending.
Example: A scope-set create passes the deadline check at 12:00:00. The pool starts its worker after the deadline at 12:00:45. The request has timed out, but the worker creates revision one anyway; a retry with the same expected revision now conflicts.
Recommended fix: Move the final deadline and cancellation check into the blocking closure immediately before its first CAS. Preserve the rule that, after the first CAS starts, every replica is attempted even if the request is subsequently cancelled.
Was this helpful? React with 👍 or 👎 to provide feedback.
Part of #3736
DaemonInvocationService::compare_and_swap_scope_setwrote every scope-set replica with the synchronousAuthorizedScopeSetSqliteStorage::compare_and_swapinside the async request. Since #4003, daemon invocations run on native workers. Each of those writes waits on the SQLite writer, and that wait parks the native worker for the whole write (thenative_thread_writer_waitshazard). This PR runs the replica loop onnative_runtime::spawn_blocking. Once the first write starts, the loop still finishes every replica. Dropping the request now only abandons the wait, so a partial effect cannot result.Measured
A scratch build logged every writer wait with its thread and caller frames. That build also routed the in-process invocation executor onto native tasks, so
mcp_suitedrives the multi-root routes on native workers. Neither scratch change is in this PR.mcp_suitethen recorded:DaemonInvocationState::invoke_for_project. They ran throughAuthorizedScopeSetSqliteStorage::compare_and_swapand itsbegin_transaction/execute.Verified at
d4e8c589b1//crates/tracedecay:unit_test(includingmulti_root_journey's scope-set CAS tests) passed in 153.8s,:mcp_suitein 120.2s,:daemon_suitein 122.0s and:session_suitein 61.3s.//crates/tracedecay-daemon-service:unit_testpasses. Its first run, at host load average about 1000, failed five tests on SQLite lease expiry and interrupted queries in tests that never reach this code. The rerun passed in 18.7s.bazel build --config=clippypasses over//crates/tracedecay-daemon-service/....rustfmt_check_2024andscripts/check-ambient-tokio.pypass.