Add exponential backoff on all mesh agent control-channel retry paths - #74
Conversation
- Reset backoff only after a 60s-stable authenticated session (halve it on shorter ones) instead of mid-handshake, closing the zero-delay reconnect flood when the server upgrades but errors before AuthConfirm - Route the 20s dial-timeout fallback through MeshServer_Connect and coalesce scheduling via retryTimerPending so each cycle grows the delay exactly once - Drop stale agent-keyed retry/lockout timers on connection establishment - Raise the backoff cap from 4-6 to 8-10 minutes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThe reconnect logic now tracks authenticated session duration, applies partial or full backoff resets on disconnect, and coalesces pending retry timers across connection paths. ChangesReconnect stability and retry scheduling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
meshcore/agentcore.c (1)
5027-5034: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClamp the computed retry delay to the 8–10 minute cap.
The cap is selected from the current
retryTimebefore jitter is added. For example,retryTime == 479999can produce a delay of nearly 960 seconds, exceeding the documented 8–10 minute maximum.Proposed adjustment
else { delay = agent->retryTime + (timeout % agent->retryTime); + if (delay >= 480000) { delay = 480000 + (timeout % 120000); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@meshcore/agentcore.c` around lines 5027 - 5034, Clamp the final computed delay in the retry logic after jitter is added so it never exceeds the documented 8–10 minute cap. Update the branch around agent->retryTime and the delay calculation to apply the cap to the resulting value, including retryTime values just below 480000, while preserving the existing jitter behavior within the valid range.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@meshcore/agentcore.c`:
- Around line 4231-4234: Update the short-session branch near agent->authTick so
halving agent->retryTime never produces a value below MESH_BACKOFF_MIN_RETRY_MS,
while still reducing eligible retry times such as 3 seconds. Preserve the
stable-session reset behavior and clear agent->authTick afterward.
---
Outside diff comments:
In `@meshcore/agentcore.c`:
- Around line 5027-5034: Clamp the final computed delay in the retry logic after
jitter is added so it never exceeds the documented 8–10 minute cap. Update the
branch around agent->retryTime and the delay calculation to apply the cap to the
resulting value, including retryTime values just below 480000, while preserving
the existing jitter behavior within the valid range.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 09291a3f-a938-47a5-9866-dc2602f8cbf4
📒 Files selected for processing (2)
meshcore/agentcore.cmeshcore/agentcore.h
… cap Addresses CodeRabbit review: short-lived authenticated sessions now always land retryTime in [2s floor, half of previous], and the jittered doubling can no longer overshoot the 8-10 minute cap to ~16 minutes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
Summary
Implements 86ajmt7ty: the mesh agent must back off exponentially on all server-communication retry paths and never flood the server with rapid retries.
The existing backoff in
MeshServer_Connect(jittered doubling) already covered pre-upgrade failures (DNS, TCP, TLS, non-101 responses), but two paths defeated it:MeshServer_SendAgentInforesetretryTime = 0mid-handshake, before the server confirmed agent auth. A server that completes the WebSocket upgrade but errors/closes beforeAuthConfirmcaused an immediate-reconnect loop bounded only by TCP+TLS handshake speed.MeshServer_ConnectExdirectly, overriding the backoff delay its own cancel path had just scheduled — a fixed 20s loop against a hung/blackholed server.Changes
MESH_BACKOFF_STABLE_SESSION_MS); shorter post-auth sessions halveretryTime(floor ~2s) so a flapping-but-working server still recovers quickly while an auth-then-die crash loop stays paced.authTickis stamped inMeshServer_ServerAuthenticated(single choke point covering both auth-completion orders).MeshServer_ConnectEx_NetworkErrornow falls back toMeshServer_Connect; a newretryTimerPendingflag coalesces scheduling so each failure cycle grows the delay exactly once. The flag is cleared at the top ofMeshServer_ConnectExbefore the re-entry guard (ordering is load-bearing: a guard-blocked stale timer must not strand the flag) and on connection establishment.ILibLifeTime_AddExdedupes by key, so duplicate timers are structurally impossible).Verification
-Wallwarnings (verified via syntax-only compile of the committed file).ILibWebClient_CancelRequestconfirmed to synchronously fireOnResponse(header=NULL)on the chain thread, making the NetworkError fallback a coalesced no-op in the normal case; no watchdog depends on the previous ~6 min cap (service auto-restart disabled in 7f2ad09).Known-accepted (out of scope): IP-change handler still resets backoff to 3s (event-driven, bounded); pre-existing dormancy on ServerID/MeshID config errors; multi-server failover now waits the backoff delay instead of 20s (single-URL configs unaffected).
🤖 Generated with Claude Code
Summary by CodeRabbit