Increase Effect RPC ping timeout for large snapshots#2885
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
ApprovabilityVerdict: Approved Simple timeout configuration change in a dependency patch - increases RPC ping delay from 5 to 60 seconds to accommodate large snapshots. Low-risk adjustment with no new behavior introduced. You can customize Macroscope's approvability policy. Learn more. |
What Changed
Increases the patched Effect RPC pinger delay from 5 seconds to 60 seconds.
This changes the existing
patches/effect@4.0.0-beta.73.patchso the generated RPC client waits longer before declaring the connection unhealthy due to a missed Pong.Why
Large thread snapshots can occupy the WebSocket long enough that the Effect RPC Pong is queued behind snapshot chunks. On slow remote connections, the current 5 second heartbeat window can expire before the Pong reaches the client, causing a reconnect. The reconnect then requests the same large snapshot again, which can repeat the failure loop.
A 60 second window gives large snapshot sends more time to drain before the client treats the connection as dead.
This is a small mitigation for #2761. It does not solve the deeper issue that large snapshots and control/heartbeat messages share the same WebSocket queue; longer term, snapshots likely need pagination, truncation, backpressure, or control-message prioritization.
Checklist
Note
Increase Effect RPC ping timeout from 5s to 60s for large snapshots
Updates the ping delay in patches/effect@4.0.0-beta.73.patch from 5 seconds to 60 seconds to accommodate large snapshot transfers. Also adds an optional
hooks.onPingcallback that runs beforewritePingwhen provided.Behavioral Change: RPC connections will now wait 60 seconds between pings instead of 5, which may affect detection of stale connections.
Macroscope summarized 81c603e.
Note
Low Risk
Single constant change in a vendor patch; main tradeoff is slower detection of truly dead RPC connections, with no auth or data-model changes.
Overview
Extends the Effect RPC client heartbeat in
patches/effect@4.0.0-beta.73.patchby changing the pinger loop delay from 5 seconds to 60 seconds, so a missedPongis not treated as a timeout while a large thread snapshot is still filling the WebSocket.This is a targeted mitigation for reconnect loops when snapshot traffic delays control messages; stale or dead connections will be noticed more slowly than with the previous 5s window.
Reviewed by Cursor Bugbot for commit 81c603e. Bugbot is set up for automated code reviews on this repo. Configure here.