Daemon robustness: handshake starvation locks out every client (A); a failed COMMIT leaves the writer inside a never-committed transaction (B) #512
Labels
No labels
code-review
correctness
dos
performance
security
severity/high
severity/low
severity/medium
tech-debt
Kind/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
h-dv/code-index#512
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Consolidated scope (triage 2026-11-09)
Priority: Critical: (A) makes the product unusable under load; (B) wedges or silently loses writes after a transient disk-full or BUSY.
This issue is now the canonical tracker for:
generations::transition/faildoOk(()) => conn.execute_batch("COMMIT")?with no ROLLBACK on COMMIT failure (generations.rs:623,658), then later writes go into the open transaction. Fix: one owned-transaction helper and a registry test against bare"COMMIT")?(from #513, closed and merged here; its full report stays readable there)Original report
Summary
A busy daemon can be locked out by its own attaching clients. When every read-pool connection is held, a new session's handshake times out, gets retried every ~50 ms for 60 s, and each abandoned request keeps holding one of the 64 connection slots until the cap is reached. From then on every client gets BUSY, and the new session drops to the read-only in-process fallback.
Failure trace
crates/daemon/src/main.rs:795-799) is held by slow queries, for example several agents runningchange_impacton a large repo.try_handshake, which sendsrpc.health()underHANDSHAKE_TIMEOUT= 2 s (crates/mcp-server/src/main.rs:1432, 1457-1460).LocalIndex::healthstarts withself.conn.get().await(crates/daemon/src/local_index.rs:3828).ReadPool::getwaits on a semaphore with no deadline (crates/daemon/src/read_pool.rs:85-92). The handshake times out.capabilitiescall, sees the old daemon as healthy, and the new daemon gives up (crates/daemon/src/takeover.rs:270-283).DAEMON_POLL_INTERVAL(50 ms) for up toDAEMON_READY_TIMEOUT= 60 s (crates/mcp-server/src/main.rs:1764-1803).spawn_blockingjob and only notices the dead socket when it writes the reply (crates/daemon/src/server.rs:383-393, 431). Each one holds a connection slot.#67 already removed the
stats()scans from this handshake for the same class of reason ("condemned, evicted, respawned, condemned again"). The handshake is still gated on a database connection, though.Proposal (one liveness rule)
capabilitiesreply the takeover probe already trusts, plus identity (root and schema) from the lockfile or a constant the daemon holds.ReadPool::geta deadline that returns a typedbusyrefusal instead of waiting forever.Acceptance
conn.get()back into the handshake path, and the test must go red.Found by an automated robustness review (2026-11-09) and verified by reading
read_pool.rs:85-92andmain.rs:1457. Not reproduced at runtime.Filed from a five-agent review of master @
e3ec6ead(bloat, robustness, agent usefulness, docs, architecture). Every finding was spot-checked in the main session before filing.Daemon handshake starvation: health() waits on the read pool with no deadline, and abandoned requests pin connection slots until every client gets BUSYto Daemon robustness: handshake starvation locks out every client (A); a failed COMMIT leaves the writer inside a never-committed transaction (B)Both parts fixed on branch
codex-512, merged intointegration-1109(B:f892a305, A:f1f4796e)B:
busy::with_owned_transactionreturns the connection to autocommit after a BEGIN, body or COMMIT failure. Every hand-written production transaction goes through it (generations transition/fail/promote_controlled, wal_maintenance, perms probe), and the read-pool snapshot is a documented exception. Registry test: a bareCOMMITreintroduced ingenerations.rsgoes red with its file:line. Behaviour test: a realSQLITE_BUSYon COMMIT (DELETE journal, a second connection holding SHARED) leaves autocommit and a later durable write. Mutation: rollback removed → red.A:
healthtakes no pool connection, runs no SQL, and uses the socket-runtime fast path likecapabilities.schema_versionis captured at pool open.activationis reported asNone: the lead verified there is no production consumer.ReadPool::getwaits ≤ 1 s (below the 2 s handshake), then returns the typedRpcError::BUSY(-7). That error is request-scoped and does not trigger connection-cap handling.twenty_clients_attach_with_every_read_connection_held: every read connection is held, 20 handshakes succeed within the bound, all permits are recovered, and a normal query works afterwards. Seven mutations were run, each red at its intended assertion. Both MCP legs and private-item rustdoc are green.Both parts are released since v0.41.48 (
f892a305,f1f4796e). Checked again on master3d0a584c(v0.41.50):twenty_clients_attach_with_every_read_connection_held(crates/daemon/tests/handshake_saturation_e2e.rs) andfailed_commit_rolls_back_and_the_connection_can_commit_durably(crates/indexer/tests/owned_transaction.rs) are in the tree and run in CI. Closing.