connection: add per-peer exponential reconnection backoff#951
Conversation
|
👋 Hi! This PR is now in draft status. |
d29aa81 to
fec6507
Compare
Previously, the background reconnection task retried every persisted peer on a fixed 60s interval with no backoff, so an unreachable peer was retried indefinitely at the same cadence — log spam and wasted work. This became more visible after lightningdevkit#895 retained peers across force-closes so that channel_reestablish recovery can run. Track per-peer reconnect state in ConnectionManager: on failure, double the retry interval up to PEER_RECONNECTION_MAX_INTERVAL (30 min); on success (including user-initiated connects), clear the state so a subsequent drop retries promptly. The 60s tokio::time::interval is kept as the wakeup, gated per-peer by next_retry_at, since lightningdevkit#588's inline-sleep form does not generalize to N peers. Backoff state is in-memory and resets on restart — a fresh post-restart attempt is the correct behavior. State is also cleared when a peer is removed from the persisted store. Closes lightningdevkit#918.
fec6507 to
1172194
Compare
|
Un-drafting now that #895 has landed. Changes since the initial draft: Rebased on main. As promised above, the backoff clear that lived in After another self-review pass:
One known limitation, documented in a comment: a peer that connects inbound cc @tnull |
Closes #918.
Summary
Today the reconnection loop retries every disconnected peer on a fixed
PEER_RECONNECTION_INTERVAL(60s). For a peer that is persistently unreachable this means a reconnect attempt every minute indefinitely, which is wasteful and noisy. This adds per-peer exponential backoff so repeated failures are spaced out, while a successful connection (or a user-initiated connect) resets a peer back to the base interval.What this does
PeerReconnectStateinconnection.rs, tracked per peer in aMutex<HashMap<PublicKey, PeerReconnectState>>onConnectionManager.PEER_RECONNECTION_MAX_INTERVAL(30 min).is_reconnect_duerather than attempting every peer every tick.do_connect_peer), on explicitdisconnect(), and when a peer is removed after its last channel closes.reconnection also resets backoff.
Tests
reconnect_state_doubles_until_capped- verifies doubling and the cap.reconnect_state_schedules_relative_to_failure_time- verifies the next retry is scheduled relative to the failure instant.A note on "configurable"
The issue title says "configurable." This PR keeps the base/cap as
pub(crate)constants (matching the existing reconnection-interval constant) rather than exposing them onConfig. Happy to wire them intoConfigas a follow-up (or here) if reviewers prefer — wanted to keep the surface minimal first.Dependency / draft status
Marking this as draft: it builds on #895, which relocates last-channel
peer removal into the
event.rsChannelClosedhandler. Once #895 lands Iwill rebase and move the
clear_reconnect_statecall to sit alongside therelocated removal (and pick up the now-async
remove_peer.awaits from #919). Will un-draft after #895 is merged.