Repository navigation
fix(lsp): admit acknowledgement receipts off the session lock - #4859
ScriptedAlchemy wants to merge 1 commit into
Conversation
An LSP frame acknowledgement published its delivery receipt to the recorder's spool (file and directory fsyncs) inline, on the serving thread, while holding the daemon-wide LSP session map lock. Every invocation takes that lock to expire sessions, so each acknowledgement's fsyncs serialized every daemon request behind them and parked a serving worker. Measured with TRACEDECAY_SPAN_TIMINGS on the production LSP journey (runtime_surface_acceptance::production_lsp_negotiates_and_projects_ canonical_context, idle macOS), the acknowledge span was 8-13 ms busy with ~2 us idle, all of it in private_fs.framed_log.publish, against ~160 us for a poll. After the change it is 25-50 us busy; the wait is an await on the blocking pool with no lock held. The receipt is still durable before the acknowledgement is answered. Claim the in-flight attempt under the lock and admit it afterwards on the blocking pool, for acknowledgement and detach. Drop keeps inline admission because it cannot await. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if let Some(delivery) = delivery { | ||
| let _ = delivery.admit().await; |
There was a problem hiding this comment.
🔴 Acknowledgement receipts lost during shutdown
If shutdown follows an acknowledgement's claim, delivery.admit() can run after the recorder closes. The claimed attempt has left its session, so the delivered receipt is lost.
Learn more
An LSP acknowledgement takes the in-flight attempt before queuing its durable receipt. expire_all clears sessions before stopping project runtimes. Recorder shutdown changes the recorder to STOPPING, and try_record then refuses new receipts. The new await allows shutdown to pass the session clear while a claimed receipt still waits on the blocking pool. The attempt is no longer in the actor, so its drop fallback cannot recover it.
Example: An ACK claims frame 7, and its blocking task waits in the pool. Shutdown clears the session and drains its recorder. The task then tries to admit frame 7 and gets delivery_settlement_recorder_closed; no Delivered observation exists.
Recommended fix: Track claimed-but-unadmitted receipts as invocation-owned work and join them before project_runtimes.shut_down_all closes recorders. Ensure project retirement also fences or joins claims before shutting its store recorder.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if let Ok(delivery) = session.claim_in_flight_delivery( | ||
| tracedecay_domain::DeliverySettlementOutcomeV1::Dropped, | ||
| Some(tracedecay_domain::DeliveryDropReasonV1::Disconnected), | ||
| ); | ||
| ) { | ||
| let _ = delivery.admit().await; |
There was a problem hiding this comment.
🟡 Cancelled detach retains orphaned lease
If a detached session has a lease, cancelling delivery.admit() skips the later lease cancellation. The session is already removed, but its lease task remains active until expiry.
Learn more
A disconnected session has a timed lease task registered by disconnect_lsp_session. A later detach closes the endpoint and removes the actor before waiting for the newly queued receipt. If the request future is dropped while waiting, the work still records the receipt, but the next lease cancellation never executes. The expiry task remains in LspLeaseTaskRegistry until the original lease deadline.
Example: A client disconnects with an outbound frame pending, then sends an explicit detach with the same credential. The detach queues a slow receipt write; its request times out. The session is gone, but its disconnected-session lease continues waiting.
Recommended fix: Cancel the lease before awaiting receipt admission, or move lease cleanup into an owned guard that survives cancellation after session removal. Preserve receipt admission even if the client stops waiting.
Was this helpful? React with 👍 or 👎 to provide feedback.
Problem
production_lsp_negotiates_and_projects_canonical_contextfailed on Linux withacknowledge daemon LSP frame: DeadlineExceeded(PR #4828, run 42724276448, job 129532080196,runtime_acceptance_suite/test.log: 140 passed / 1 failed, suite 126 s). Poll and send had already succeeded on the same connection.The acknowledgement is the only LSP invocation that does durable disk I/O on the request path.
acknowledge_lsp_framecalledBoundedDeliverySettlementRecorderV1::try_recordinline, which publishes the receipt to the spool (file fsync, rename, directory fsync). It did so on the serving thread and while holding the daemon-widelsp_sessionslock. Every daemon invocation takes that lock to expire sessions, so one acknowledgement's fsyncs stalled every other request behind it.detachhad the same inline admission.Measurement
TRACEDECAY_SPAN_TIMINGS=1with the daemon trace-scoped to the LSP and private-fs targets, on the production LSP journey (idle macOS, current master):daemon.service.lsp.polldaemon.service.lsp.acknowledgebeforedaemon.service.lsp.acknowledgeafterBefore, essentially all of the acknowledge span was
private_fs.framed_log.publish(two fsyncs, 3–6 ms each). Client-observed acknowledgement latency is unchanged (7–18 ms) because the receipt is still durable before the acknowledgement is answered.Change
Claim the in-flight attempt under the session lock, release it, then admit the receipt on the blocking pool (
native_runtime::spawn_blocking, the pattern the scope-set compare-and-swap already uses). Applies to acknowledge and detach. The attempt leaves the session when claimed, so it cannot be settled twice, and the work is queued at the call, so a dropped connection abandons only the wait.Dropkeeps inline admission because it cannot await. No timeout, retry, or durability change.Evidence
lsp_acknowledgement_admits_its_delivery_receipt_off_the_serving_threaddrives the real service, real recorder, real spool and global DB, and captures the thread that opensprivate_fs.framed_log.publish. It fails on the unfixed code ("the acknowledgement published its receipt on the serving thread") and passes with the fix. It also asserts the receipt settles durably as oneDelivered. The existing detach and disconnect-expiry drop tests still pass.//crates/tracedecay-daemon-service:unit_testand:invocation_observabilitypass;//:rustfmt_checkand--config=clippyon the crate are clean.Not established
This is not a reproduction of the Linux failure. Current master passes the journey on macOS, and the full
runtime_acceptance_suitealso passes locally (141/141, max acknowledgement 17 ms), so no fsync amplification appeared on this disk. The Linux failure is consistent with fsync stalls under the 141-test parallel suite, but that is not proven. If a single acknowledgement's own fsyncs still exceed the client's 5 s deadline on a stalled disk, this change does not shorten that wait; it removes the worker and global-lock amplification. The macOSdocumentSymbolProviderinitialize failures logged on #4834 are a separate negotiation/readiness symptom and are not touched here.Other synchronous
try_recordcallers (Work delivery ACK inconnection_serving,tracedecay-mcprmcp) are untouched.🤖 Generated with Claude Code