-
Notifications
You must be signed in to change notification settings - Fork 5
Record a presence read that ran out of time (#322) - #324
Merged
Merged
Conversation
`drain-stall` fires only past `drainLeaseDuration`, which is 300s, while a presence read is bounded at 45s. So a drain stalled by a hung read released long before the threshold and left no line at all — the stall that actually happens was the one nothing recorded. That is how #311 stayed invisible for a day: frozen and working looked identical from outside. An end-to-end rerun of that wedge against the shipped 0.1.64 found delivery correctly recovering at the deadline and, in the same runs, zero drain-stall and zero delivery-stall lines. The only trace was a `handle_ms` on a command that happened to be riding the wedged drain; a wedge the presence poller hits with no client command in flight left nothing. The timeout is the event, and the lease is only the backstop, so the line belongs where the read gives up. It carries the node whose backend stopped answering and the bound it broke — a UUID and a duration, never a title or any other user text, matching what `delivery-stall` already records. Any deadline shorter than the stall threshold it feeds is invisible by construction; this was the one that mattered because the presence poll runs without a client command to carry its timing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP43ags4cn8fq2ZZdv85J9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One log line, and a test that fails without it.
The gap
drain-stallfires only pastdrainLeaseDuration(300 s). A presence read is bounded atpresenceReadDeadline(45 s). So a drain stalled by a hung read releases long before the threshold and leaves no line at all — the stall that actually happens in practice is the one nothing records.Measured end-to-end against the shipped 0.1.64 during the #311 wedge rerun:
Delivery recovers correctly at the deadline, exactly once, nothing lost. But those runs produced zero
drain-stalland zerodelivery-stalllines. The only trace washandle_ms=48787.6on amessageNodethat happened to be riding the wedged drain — and a wedge the presence poller hits, with no client command in flight, left nothing at all.That is how #311 stayed invisible for a day: frozen and working looked identical from outside. #289 shipped so that could not happen again, and for the wedge that actually occurs it was silent.
The change
Log where the read gives up. The timeout is the event; the lease is only the backstop.
The line carries the node whose backend stopped answering and the bound it broke — a UUID and a duration, never a title or any other user text. That matches what
delivery-stallalready records, and keeps #289's constraint that no user content reaches the log.Test
aTimedOutPresenceReadIsRecorded— a presence read that sleeps past a 50 ms deadline must leave apresence-stallline naming the node and the deadline.Verified the way that actually proves something: with the fix removed from
GraphStore.swift, the test fails; restored, it passes.Worth recording how nearly I got that wrong — my first attempt used
-only-testing:with a method name, which matched nothing and exited 0. A vacuous pass I almost recorded as evidence. The suite-level filter is what the project's own skill prescribes, for exactly this reason.Gate
1693 tests / 180 suites / 0 failures, no restarts;
scripts/cli-smoke.shexit 0; swiftlint 0 errors; swift-format clean.Worth a follow-up, not in this PR
Any deadline shorter than the stall threshold it feeds is invisible by construction. This was the one that mattered because the presence poll runs without a client command to carry its timing, but the pattern is worth an audit of the other bounded waits.
Closes #322
🤖 Generated with Claude Code
https://claude.ai/code/session_01BP43ags4cn8fq2ZZdv85J9