fix(peer): notice a dead link in 1.5s, not 3.2s — resume under the R1 gate - #84
Conversation
… gate netsim (#82) measured resume at 4.08s direct and 4.50s relayed, of which 3.23s was detection: iceDisconnectedTimeout (2s) plus LinkGrace (1s). The redial it gates was already sub-second, so the beta's "under 3s" could only come out of detection. iceDisconnectedTimeout 2s -> 1s (two missed 500ms keepalives instead of four) and LinkGrace 1s -> 500ms. Keepalive is unchanged, so a single lost packet still cannot tear down a live session. Measured over 9 flip reps each, medians: flip-prc resume 4077ms -> 2564ms (detection 3.23s -> 1.75s) flip-turn resume 4496ms -> 2996ms Detection reads ~250ms above the 1.5s arithmetic because pion checks liveness on a keepalive-driven ticker, so the transition lands up to one interval late. Flap accounting is untouched and stayed honest: every one of 18 flips redialed on the first attempt with no backoff sleep (redial 822-832ms direct, i.e. the healthy-drop path), 18/18 continuations held, and no session was torn down before its flip. Also here: - peer_link_test.go pins the detection-plus-redial sum against the 3s gate, so a future retune has to face it deliberately. - netsim records redials per rep and reports any that needed more than one; that is the signal to watch after a detection retune. - netsim/run.sh honours an exported NETSIM_REPS, which it previously clobbered. Closes #83 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KeiotDVE94wEzvc7wcvm1y
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c73dddc7f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const ( | ||
| resumeGate = 3 * time.Second | ||
| measuredRedial = 1250 * time.Millisecond // netsim flip-turn, the slower path | ||
| detectionBudget = iceDisconnectedTimeout + LinkGrace |
There was a problem hiding this comment.
When the ICE liveness check runs near the end of its keepalive tick, the disconnected transition can arrive up to iceKeepAlive later than iceDisconnectedTimeout; the commit's own netsim notes describe this jitter and report flip-turn samples above the 3 s gate. By budgeting only iceDisconnectedTimeout + LinkGrace, this test asserts 2.75 s while the configured upper budget is 3.25 s after adding the 1.25 s TURN redial, so it passes despite not actually pinning the stated p90 requirement. Include the keepalive scheduling delay in the budget or validate the empirical p90 instead.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Netsim (#82, #84) measured the matrix; the gap that remains is real-network data, so say that instead. Claude-Session: https://claude.ai/code/session_01KeiotDVE94wEzvc7wcvm1y Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
netsim (#82) measured resume at 4.08 s direct and 4.50 s relayed, and showed
where it went: 3.23 s of it was detection —
iceDisconnectedTimeout(2 s)plus
LinkGrace(1 s) — while the redial those constants gate was alreadysub-second. So the beta's "under 3 s" could only come out of detection.
This takes it out.
iceDisconnectedTimeout2 s → 1 s,LinkGrace1 s →500 ms.
iceKeepAlivestays at 500 ms, so detection is still two missedkeepalives rather than four: a single lost packet cannot tear down a live
session.
iceFailedTimeoutis unchanged.Closes #83.
Measured
Nine flip reps per scenario, on the same harness and host as #82. Medians:
flip-prcflip-turnSpread after, over 9 reps each:
flip-prc2550–2685 ms,flip-turn2962–3015 ms. Continuation 18/18. Full table in
netsim/results/results.md.The redial is unchanged, as expected — 820–832 ms direct, 1227–1254 ms relayed.
The entire gain is detection.
Two honest caveats
Detection measures ~1.75 s, not the 1.5 s the arithmetic implies. pion checks
liveness on a keepalive-driven ticker, so the
disconnectedtransition lands upto one 500 ms interval late. The ~250 ms is that jitter, and it is stable across
all 18 reps (1702–1850 ms).
The relayed path meets the gate without margin.
flip-turnresumes clusterbetween 2.96 s and 3.02 s, so roughly half land just over 3 s. Its extra ~400 ms
over the direct path is the TURN allocation inside the redial. Holding the gate
on relayed sessions with room to spare means making that redial faster, which no
amount of detection tuning can do — worth its own issue rather than pushing
these constants further.
Separately, a ~1 s TURN retransmit occasionally lands on a dial or redial. It
predates this change — it is visible in the
turn-onlyrow that merged with#82 (1262 / 2247 ms) — and looks like an ICE/TURN retransmission timer. When it
lands on a resume, that rep reads ~4 s. It showed up in 2 of 24 post-change
flip-turn samples and in the pre-change run alike.
Flap honesty
The concern with faster detection is that a marginal link trips teardown more
easily and gets misread as a flap. The classification itself is untouched —
ReconnectPolicy.MinHealthy(5 s) and the failure budget are unchanged — and themeasurements say it stayed honest:
That is directly visible in the numbers: a flap-classified drop sleeps
Base= 1 s before retrying, so redial would read ≥1.8 s. It read 822–832 msdirect and 1227–1254 ms relayed — the healthy-drop path, which resets the
budget and redials promptly.
flip, across every rep of every scenario.
The real trade, stated plainly: a blip that heals between 1.5 s and 3 s used to
be absorbed and now costs a redial (~0.8 s direct). Since the redial is cheap and
the alternative is a frozen terminal, that is the right way round — but it is a
behaviour change, not a free win.
Also here
peer_link_test.gopins the sum. A new test asserts detection plus themeasured redial fits the 3 s gate, that detection is at least two keepalives,
and that
failedstill outlastsdisconnected. Retuning any of it is fine;doing so without facing the gate is how it quietly reopens.
attempts) and lists any rep that neededmore than one under a "Retried redials" heading. That is the signal to watch
after a detection retune — a teardown the loop then could not immediately
recover from. This run had none.
netsim/run.shhonours an exportedNETSIM_REPS, which it previouslyclobbered with the per-scenario default.
NETSIM_REPS=9 ./run.sh flip-turnnowsamples a tail properly, which is how the numbers above were taken.
Checks
cd go && go test ./...green,-racegreen on peer/client/agent,gofmt -l .empty,
go vet ./...clean,cd web && npm test152/152 (afternpm ci),./scripts/verify-reproducible.shbyte-identical. No web constant changed —disconnect-grace.jscites browser-side timings, not these.🤖 Generated with Claude Code
https://claude.ai/code/session_01KeiotDVE94wEzvc7wcvm1y
Note
Medium Risk
Changes production ICE teardown timing on all attaches, which can increase redials on marginal links but is bounded by existing reconnect policy and netsim flap checks.
Overview
Cuts ICE link-death detection so post-flip resume can meet the R1 “under 3s” gate:
iceDisconnectedTimeoutgoes from 2s to 1s andLinkGracefrom 1s to 500ms (still two missed 500ms keepalives beforedisconnected). Netsim showed ~3.2s of a ~4s resume was detection while redial was already sub-second.Direct flip resume median drops to ~2.56s; relayed
flip-turnclusters around ~3.0s with little margin. Tradeoff: brief outages that heal between ~1.5s and ~3s now trigger a cheap redial instead of being absorbed.Harness and guardrails:
peer_link_testasserts detection budget + measured TURN redial stays under 3s; netsim records per-rep reconnect attempts and highlights multi-attempt redials;run.shrespects an exportedNETSIM_REPSfor tail sampling. Updated README and committedresults.mdfrom 9-rep flip runs.Reviewed by Cursor Bugbot for commit c73dddc. Bugbot is set up for automated code reviews on this repo. Configure here.