Skip to content

connection: add per-peer exponential reconnection backoff - #951

Closed
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff
Closed

connection: add per-peer exponential reconnection backoff#951
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff

Conversation

@Jolah1

Copy link
Copy Markdown
Contributor

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

  • Introduces PeerReconnectState in connection.rs, tracked per peer in a Mutex<HashMap<PublicKey, PeerReconnectState>> on ConnectionManager.
  • Each consecutive failure schedules the next retry at the current backoff and doubles it, capped at PEER_RECONNECTION_MAX_INTERVAL (30 min).
  • The reconnect loop gates each peer on is_reconnect_due rather than attempting every peer every tick.
  • Backoff state is cleared on a successful connect (do_connect_peer), on explicit disconnect(), and when a peer is removed after its last channel closes.
  • Already-connected peers have their state cleared each tick, so an inbound
    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 on Config. Happy to wire them into Config as 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.rsChannelClosed handler. Once #895 lands I
will rebase and move the clear_reconnect_state call to sit alongside the
relocated removal (and pick up the now-async remove_peer.awaits from #919). Will un-draft after #895 is merged.

@ldk-reviews-bot

ldk-reviews-bot commented Jun 26, 2026

Copy link
Copy Markdown

👋 Hi! This PR is now in draft status.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@Jolah1
Jolah1 marked this pull request as draft June 26, 2026 08:01
@Jolah1
Jolah1 marked this pull request as ready for review July 1, 2026 13:26
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from d29aa81 to fec6507CompareJuly 1, 2026 14:39
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from fec6507 to 1172194CompareJuly 9, 2026 13:04
@Jolah1

Copy link
Copy Markdown
ContributorAuthor

Un-drafting now that #895 has landed. Changes since the initial draft:

Rebased on main. As promised above, the backoff clear that lived in
close_channel_internal moved to #895's relocated peer removal in the
ChannelClosed event handler, so backoff state is dropped exactly where the
peer leaves the store.

After another self-review pass:

  • Backoff bookkeeping is now anchored to the reconnection loop's tick instant
    instead of each attempt's completion time. Previously a first failure was
    scheduled at attempt_end + 60s, which always slipped past the next 60s
    tick and effectively doubled the first retry delay.
  • The loop now skips a peer that already has a connection attempt in flight
    (has_pending_connection) rather than subscribing to it. This avoids
    recording someone else's failure — possibly for a different, user-supplied
    address
    — as a failure of the persisted address.
  • Backoff state is pruned against the persisted peer store each tick. This
    guards a race where a failed in-flight attempt could resurrect state for a
    peer that was concurrently removed (leaking the entry), and makes cleanup
    self-healing if future code adds remove_peer call sites.
  • connect_peer_if_necessary clears backoff on its already-connected early
    return, since the peer is demonstrably reachable.
  • Simplified record_reconnect_attempt(&Result) into
    record_reconnect_failure called only on Err; do_connect_peer is now
    the single owner of clear-on-success.

One known limitation, documented in a comment: a peer that connects inbound
and drops again entirely within one tick keeps its accumulated backoff, since
connectivity is only observed at tick time. Clearing eagerly on inbound
connects would require hooking the LDK peer_connected handlers, which felt
out of scope here .

cc @tnull

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.
Closeslightningdevkit#918.
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from 1172194 to a9d0d13CompareJuly 10, 2026 04:40
@Jolah1

Jolah1 commented Jul 10, 2026

Copy link
Copy Markdown
ContributorAuthor

The build (macos-latest, stable) failure is the known reorg_test flake (tests/reorg_test.rs:170, "Unexpected balance state!") that #972 fixes.
The other two red jobs are fail-fast cancellations from it.

@Jolah1

Jolah1 commented Jul 16, 2026

Copy link
Copy Markdown
ContributorAuthor

Closing per the discussion on #918#983 covers the motivating case.
@tnull

@tnulltnull closed this Jul 17, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add configurable reconnection backoff for persisted peers

3 participants

@Jolah1@ldk-reviews-bot@tnull
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
connection: add per-peer exponential reconnection backoff by Jolah1 · Pull Request #951 · lightningdevkit/ldk-node · GitHub
Skip to content

connection: add per-peer exponential reconnection backoff - #951

Closed
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff
Closed

connection: add per-peer exponential reconnection backoff#951
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff

Conversation

@Jolah1

Copy link
Copy Markdown
Contributor

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

  • Introduces PeerReconnectState in connection.rs, tracked per peer in a Mutex<HashMap<PublicKey, PeerReconnectState>> on ConnectionManager.
  • Each consecutive failure schedules the next retry at the current backoff and doubles it, capped at PEER_RECONNECTION_MAX_INTERVAL (30 min).
  • The reconnect loop gates each peer on is_reconnect_due rather than attempting every peer every tick.
  • Backoff state is cleared on a successful connect (do_connect_peer), on explicit disconnect(), and when a peer is removed after its last channel closes.
  • Already-connected peers have their state cleared each tick, so an inbound
    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 on Config. Happy to wire them into Config as 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.rsChannelClosed handler. Once #895 lands I
will rebase and move the clear_reconnect_state call to sit alongside the
relocated removal (and pick up the now-async remove_peer.awaits from #919). Will un-draft after #895 is merged.

@ldk-reviews-bot

ldk-reviews-bot commented Jun 26, 2026

Copy link
Copy Markdown

👋 Hi! This PR is now in draft status.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@Jolah1
Jolah1 marked this pull request as draft June 26, 2026 08:01
@Jolah1
Jolah1 marked this pull request as ready for review July 1, 2026 13:26
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from d29aa81 to fec6507CompareJuly 1, 2026 14:39
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from fec6507 to 1172194CompareJuly 9, 2026 13:04
@Jolah1

Copy link
Copy Markdown
ContributorAuthor

Un-drafting now that #895 has landed. Changes since the initial draft:

Rebased on main. As promised above, the backoff clear that lived in
close_channel_internal moved to #895's relocated peer removal in the
ChannelClosed event handler, so backoff state is dropped exactly where the
peer leaves the store.

After another self-review pass:

  • Backoff bookkeeping is now anchored to the reconnection loop's tick instant
    instead of each attempt's completion time. Previously a first failure was
    scheduled at attempt_end + 60s, which always slipped past the next 60s
    tick and effectively doubled the first retry delay.
  • The loop now skips a peer that already has a connection attempt in flight
    (has_pending_connection) rather than subscribing to it. This avoids
    recording someone else's failure — possibly for a different, user-supplied
    address
    — as a failure of the persisted address.
  • Backoff state is pruned against the persisted peer store each tick. This
    guards a race where a failed in-flight attempt could resurrect state for a
    peer that was concurrently removed (leaking the entry), and makes cleanup
    self-healing if future code adds remove_peer call sites.
  • connect_peer_if_necessary clears backoff on its already-connected early
    return, since the peer is demonstrably reachable.
  • Simplified record_reconnect_attempt(&Result) into
    record_reconnect_failure called only on Err; do_connect_peer is now
    the single owner of clear-on-success.

One known limitation, documented in a comment: a peer that connects inbound
and drops again entirely within one tick keeps its accumulated backoff, since
connectivity is only observed at tick time. Clearing eagerly on inbound
connects would require hooking the LDK peer_connected handlers, which felt
out of scope here .

cc @tnull

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.
Closeslightningdevkit#918.
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from 1172194 to a9d0d13CompareJuly 10, 2026 04:40
@Jolah1

Jolah1 commented Jul 10, 2026

Copy link
Copy Markdown
ContributorAuthor

The build (macos-latest, stable) failure is the known reorg_test flake (tests/reorg_test.rs:170, "Unexpected balance state!") that #972 fixes.
The other two red jobs are fail-fast cancellations from it.

@Jolah1

Jolah1 commented Jul 16, 2026

Copy link
Copy Markdown
ContributorAuthor

Closing per the discussion on #918#983 covers the motivating case.
@tnull

@tnulltnull closed this Jul 17, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add configurable reconnection backoff for persisted peers

3 participants

@Jolah1@ldk-reviews-bot@tnull
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' connection: add per-peer exponential reconnection backoff by Jolah1 · Pull Request #951 · lightningdevkit/ldk-node · GitHub
Skip to content

connection: add per-peer exponential reconnection backoff - #951

Closed
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff
Closed

connection: add per-peer exponential reconnection backoff#951
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff

Conversation

@Jolah1

Copy link
Copy Markdown
Contributor

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

  • Introduces PeerReconnectState in connection.rs, tracked per peer in a Mutex<HashMap<PublicKey, PeerReconnectState>> on ConnectionManager.
  • Each consecutive failure schedules the next retry at the current backoff and doubles it, capped at PEER_RECONNECTION_MAX_INTERVAL (30 min).
  • The reconnect loop gates each peer on is_reconnect_due rather than attempting every peer every tick.
  • Backoff state is cleared on a successful connect (do_connect_peer), on explicit disconnect(), and when a peer is removed after its last channel closes.
  • Already-connected peers have their state cleared each tick, so an inbound
    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 on Config. Happy to wire them into Config as 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.rsChannelClosed handler. Once #895 lands I
will rebase and move the clear_reconnect_state call to sit alongside the
relocated removal (and pick up the now-async remove_peer.awaits from #919). Will un-draft after #895 is merged.

@ldk-reviews-bot

ldk-reviews-bot commented Jun 26, 2026

Copy link
Copy Markdown

👋 Hi! This PR is now in draft status.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@Jolah1
Jolah1 marked this pull request as draft June 26, 2026 08:01
@Jolah1
Jolah1 marked this pull request as ready for review July 1, 2026 13:26
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from d29aa81 to fec6507CompareJuly 1, 2026 14:39
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from fec6507 to 1172194CompareJuly 9, 2026 13:04
@Jolah1

Copy link
Copy Markdown
ContributorAuthor

Un-drafting now that #895 has landed. Changes since the initial draft:

Rebased on main. As promised above, the backoff clear that lived in
close_channel_internal moved to #895's relocated peer removal in the
ChannelClosed event handler, so backoff state is dropped exactly where the
peer leaves the store.

After another self-review pass:

  • Backoff bookkeeping is now anchored to the reconnection loop's tick instant
    instead of each attempt's completion time. Previously a first failure was
    scheduled at attempt_end + 60s, which always slipped past the next 60s
    tick and effectively doubled the first retry delay.
  • The loop now skips a peer that already has a connection attempt in flight
    (has_pending_connection) rather than subscribing to it. This avoids
    recording someone else's failure — possibly for a different, user-supplied
    address
    — as a failure of the persisted address.
  • Backoff state is pruned against the persisted peer store each tick. This
    guards a race where a failed in-flight attempt could resurrect state for a
    peer that was concurrently removed (leaking the entry), and makes cleanup
    self-healing if future code adds remove_peer call sites.
  • connect_peer_if_necessary clears backoff on its already-connected early
    return, since the peer is demonstrably reachable.
  • Simplified record_reconnect_attempt(&Result) into
    record_reconnect_failure called only on Err; do_connect_peer is now
    the single owner of clear-on-success.

One known limitation, documented in a comment: a peer that connects inbound
and drops again entirely within one tick keeps its accumulated backoff, since
connectivity is only observed at tick time. Clearing eagerly on inbound
connects would require hooking the LDK peer_connected handlers, which felt
out of scope here .

cc @tnull

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.
Closeslightningdevkit#918.
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from 1172194 to a9d0d13CompareJuly 10, 2026 04:40
@Jolah1

Jolah1 commented Jul 10, 2026

Copy link
Copy Markdown
ContributorAuthor

The build (macos-latest, stable) failure is the known reorg_test flake (tests/reorg_test.rs:170, "Unexpected balance state!") that #972 fixes.
The other two red jobs are fail-fast cancellations from it.

@Jolah1

Jolah1 commented Jul 16, 2026

Copy link
Copy Markdown
ContributorAuthor

Closing per the discussion on #918#983 covers the motivating case.
@tnull

@tnulltnull closed this Jul 17, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add configurable reconnection backoff for persisted peers

3 participants

@Jolah1@ldk-reviews-bot@tnull
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' connection: add per-peer exponential reconnection backoff by Jolah1 · Pull Request #951 · lightningdevkit/ldk-node · GitHub
Skip to content

connection: add per-peer exponential reconnection backoff - #951

Closed
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff
Closed

connection: add per-peer exponential reconnection backoff#951
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff

Conversation

@Jolah1

Copy link
Copy Markdown
Contributor

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

  • Introduces PeerReconnectState in connection.rs, tracked per peer in a Mutex<HashMap<PublicKey, PeerReconnectState>> on ConnectionManager.
  • Each consecutive failure schedules the next retry at the current backoff and doubles it, capped at PEER_RECONNECTION_MAX_INTERVAL (30 min).
  • The reconnect loop gates each peer on is_reconnect_due rather than attempting every peer every tick.
  • Backoff state is cleared on a successful connect (do_connect_peer), on explicit disconnect(), and when a peer is removed after its last channel closes.
  • Already-connected peers have their state cleared each tick, so an inbound
    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 on Config. Happy to wire them into Config as 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.rsChannelClosed handler. Once #895 lands I
will rebase and move the clear_reconnect_state call to sit alongside the
relocated removal (and pick up the now-async remove_peer.awaits from #919). Will un-draft after #895 is merged.

@ldk-reviews-bot

ldk-reviews-bot commented Jun 26, 2026

Copy link
Copy Markdown

👋 Hi! This PR is now in draft status.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@Jolah1
Jolah1 marked this pull request as draft June 26, 2026 08:01
@Jolah1
Jolah1 marked this pull request as ready for review July 1, 2026 13:26
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from d29aa81 to fec6507CompareJuly 1, 2026 14:39
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from fec6507 to 1172194CompareJuly 9, 2026 13:04
@Jolah1

Copy link
Copy Markdown
ContributorAuthor

Un-drafting now that #895 has landed. Changes since the initial draft:

Rebased on main. As promised above, the backoff clear that lived in
close_channel_internal moved to #895's relocated peer removal in the
ChannelClosed event handler, so backoff state is dropped exactly where the
peer leaves the store.

After another self-review pass:

  • Backoff bookkeeping is now anchored to the reconnection loop's tick instant
    instead of each attempt's completion time. Previously a first failure was
    scheduled at attempt_end + 60s, which always slipped past the next 60s
    tick and effectively doubled the first retry delay.
  • The loop now skips a peer that already has a connection attempt in flight
    (has_pending_connection) rather than subscribing to it. This avoids
    recording someone else's failure — possibly for a different, user-supplied
    address
    — as a failure of the persisted address.
  • Backoff state is pruned against the persisted peer store each tick. This
    guards a race where a failed in-flight attempt could resurrect state for a
    peer that was concurrently removed (leaking the entry), and makes cleanup
    self-healing if future code adds remove_peer call sites.
  • connect_peer_if_necessary clears backoff on its already-connected early
    return, since the peer is demonstrably reachable.
  • Simplified record_reconnect_attempt(&Result) into
    record_reconnect_failure called only on Err; do_connect_peer is now
    the single owner of clear-on-success.

One known limitation, documented in a comment: a peer that connects inbound
and drops again entirely within one tick keeps its accumulated backoff, since
connectivity is only observed at tick time. Clearing eagerly on inbound
connects would require hooking the LDK peer_connected handlers, which felt
out of scope here .

cc @tnull

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.
Closeslightningdevkit#918.
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from 1172194 to a9d0d13CompareJuly 10, 2026 04:40
@Jolah1

Jolah1 commented Jul 10, 2026

Copy link
Copy Markdown
ContributorAuthor

The build (macos-latest, stable) failure is the known reorg_test flake (tests/reorg_test.rs:170, "Unexpected balance state!") that #972 fixes.
The other two red jobs are fail-fast cancellations from it.

@Jolah1

Jolah1 commented Jul 16, 2026

Copy link
Copy Markdown
ContributorAuthor

Closing per the discussion on #918#983 covers the motivating case.
@tnull

@tnulltnull closed this Jul 17, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add configurable reconnection backoff for persisted peers

3 participants

@Jolah1@ldk-reviews-bot@tnull
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' connection: add per-peer exponential reconnection backoff by Jolah1 · Pull Request #951 · lightningdevkit/ldk-node · GitHub
Skip to content

connection: add per-peer exponential reconnection backoff - #951

Closed
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff
Closed

connection: add per-peer exponential reconnection backoff#951
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff

Conversation

@Jolah1

Copy link
Copy Markdown
Contributor

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

  • Introduces PeerReconnectState in connection.rs, tracked per peer in a Mutex<HashMap<PublicKey, PeerReconnectState>> on ConnectionManager.
  • Each consecutive failure schedules the next retry at the current backoff and doubles it, capped at PEER_RECONNECTION_MAX_INTERVAL (30 min).
  • The reconnect loop gates each peer on is_reconnect_due rather than attempting every peer every tick.
  • Backoff state is cleared on a successful connect (do_connect_peer), on explicit disconnect(), and when a peer is removed after its last channel closes.
  • Already-connected peers have their state cleared each tick, so an inbound
    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 on Config. Happy to wire them into Config as 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.rsChannelClosed handler. Once #895 lands I
will rebase and move the clear_reconnect_state call to sit alongside the
relocated removal (and pick up the now-async remove_peer.awaits from #919). Will un-draft after #895 is merged.

@ldk-reviews-bot

ldk-reviews-bot commented Jun 26, 2026

Copy link
Copy Markdown

👋 Hi! This PR is now in draft status.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@Jolah1
Jolah1 marked this pull request as draft June 26, 2026 08:01
@Jolah1
Jolah1 marked this pull request as ready for review July 1, 2026 13:26
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from d29aa81 to fec6507CompareJuly 1, 2026 14:39
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from fec6507 to 1172194CompareJuly 9, 2026 13:04
@Jolah1

Copy link
Copy Markdown
ContributorAuthor

Un-drafting now that #895 has landed. Changes since the initial draft:

Rebased on main. As promised above, the backoff clear that lived in
close_channel_internal moved to #895's relocated peer removal in the
ChannelClosed event handler, so backoff state is dropped exactly where the
peer leaves the store.

After another self-review pass:

  • Backoff bookkeeping is now anchored to the reconnection loop's tick instant
    instead of each attempt's completion time. Previously a first failure was
    scheduled at attempt_end + 60s, which always slipped past the next 60s
    tick and effectively doubled the first retry delay.
  • The loop now skips a peer that already has a connection attempt in flight
    (has_pending_connection) rather than subscribing to it. This avoids
    recording someone else's failure — possibly for a different, user-supplied
    address
    — as a failure of the persisted address.
  • Backoff state is pruned against the persisted peer store each tick. This
    guards a race where a failed in-flight attempt could resurrect state for a
    peer that was concurrently removed (leaking the entry), and makes cleanup
    self-healing if future code adds remove_peer call sites.
  • connect_peer_if_necessary clears backoff on its already-connected early
    return, since the peer is demonstrably reachable.
  • Simplified record_reconnect_attempt(&Result) into
    record_reconnect_failure called only on Err; do_connect_peer is now
    the single owner of clear-on-success.

One known limitation, documented in a comment: a peer that connects inbound
and drops again entirely within one tick keeps its accumulated backoff, since
connectivity is only observed at tick time. Clearing eagerly on inbound
connects would require hooking the LDK peer_connected handlers, which felt
out of scope here .

cc @tnull

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.
Closeslightningdevkit#918.
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from 1172194 to a9d0d13CompareJuly 10, 2026 04:40
@Jolah1

Jolah1 commented Jul 10, 2026

Copy link
Copy Markdown
ContributorAuthor

The build (macos-latest, stable) failure is the known reorg_test flake (tests/reorg_test.rs:170, "Unexpected balance state!") that #972 fixes.
The other two red jobs are fail-fast cancellations from it.

@Jolah1

Jolah1 commented Jul 16, 2026

Copy link
Copy Markdown
ContributorAuthor

Closing per the discussion on #918#983 covers the motivating case.
@tnull

@tnulltnull closed this Jul 17, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add configurable reconnection backoff for persisted peers

3 participants

@Jolah1@ldk-reviews-bot@tnull
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' connection: add per-peer exponential reconnection backoff by Jolah1 · Pull Request #951 · lightningdevkit/ldk-node · GitHub
Skip to content

connection: add per-peer exponential reconnection backoff - #951

Closed
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff
Closed

connection: add per-peer exponential reconnection backoff#951
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff

Conversation

@Jolah1

Copy link
Copy Markdown
Contributor

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

  • Introduces PeerReconnectState in connection.rs, tracked per peer in a Mutex<HashMap<PublicKey, PeerReconnectState>> on ConnectionManager.
  • Each consecutive failure schedules the next retry at the current backoff and doubles it, capped at PEER_RECONNECTION_MAX_INTERVAL (30 min).
  • The reconnect loop gates each peer on is_reconnect_due rather than attempting every peer every tick.
  • Backoff state is cleared on a successful connect (do_connect_peer), on explicit disconnect(), and when a peer is removed after its last channel closes.
  • Already-connected peers have their state cleared each tick, so an inbound
    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 on Config. Happy to wire them into Config as 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.rsChannelClosed handler. Once #895 lands I
will rebase and move the clear_reconnect_state call to sit alongside the
relocated removal (and pick up the now-async remove_peer.awaits from #919). Will un-draft after #895 is merged.

@ldk-reviews-bot

ldk-reviews-bot commented Jun 26, 2026

Copy link
Copy Markdown

👋 Hi! This PR is now in draft status.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@Jolah1
Jolah1 marked this pull request as draft June 26, 2026 08:01
@Jolah1
Jolah1 marked this pull request as ready for review July 1, 2026 13:26
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from d29aa81 to fec6507CompareJuly 1, 2026 14:39
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from fec6507 to 1172194CompareJuly 9, 2026 13:04
@Jolah1

Copy link
Copy Markdown
ContributorAuthor

Un-drafting now that #895 has landed. Changes since the initial draft:

Rebased on main. As promised above, the backoff clear that lived in
close_channel_internal moved to #895's relocated peer removal in the
ChannelClosed event handler, so backoff state is dropped exactly where the
peer leaves the store.

After another self-review pass:

  • Backoff bookkeeping is now anchored to the reconnection loop's tick instant
    instead of each attempt's completion time. Previously a first failure was
    scheduled at attempt_end + 60s, which always slipped past the next 60s
    tick and effectively doubled the first retry delay.
  • The loop now skips a peer that already has a connection attempt in flight
    (has_pending_connection) rather than subscribing to it. This avoids
    recording someone else's failure — possibly for a different, user-supplied
    address
    — as a failure of the persisted address.
  • Backoff state is pruned against the persisted peer store each tick. This
    guards a race where a failed in-flight attempt could resurrect state for a
    peer that was concurrently removed (leaking the entry), and makes cleanup
    self-healing if future code adds remove_peer call sites.
  • connect_peer_if_necessary clears backoff on its already-connected early
    return, since the peer is demonstrably reachable.
  • Simplified record_reconnect_attempt(&Result) into
    record_reconnect_failure called only on Err; do_connect_peer is now
    the single owner of clear-on-success.

One known limitation, documented in a comment: a peer that connects inbound
and drops again entirely within one tick keeps its accumulated backoff, since
connectivity is only observed at tick time. Clearing eagerly on inbound
connects would require hooking the LDK peer_connected handlers, which felt
out of scope here .

cc @tnull

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.
Closeslightningdevkit#918.
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from 1172194 to a9d0d13CompareJuly 10, 2026 04:40
@Jolah1

Jolah1 commented Jul 10, 2026

Copy link
Copy Markdown
ContributorAuthor

The build (macos-latest, stable) failure is the known reorg_test flake (tests/reorg_test.rs:170, "Unexpected balance state!") that #972 fixes.
The other two red jobs are fail-fast cancellations from it.

@Jolah1

Jolah1 commented Jul 16, 2026

Copy link
Copy Markdown
ContributorAuthor

Closing per the discussion on #918#983 covers the motivating case.
@tnull

@tnulltnull closed this Jul 17, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add configurable reconnection backoff for persisted peers

3 participants

@Jolah1@ldk-reviews-bot@tnull
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' connection: add per-peer exponential reconnection backoff by Jolah1 · Pull Request #951 · lightningdevkit/ldk-node · GitHub
Skip to content

connection: add per-peer exponential reconnection backoff - #951

Closed
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff
Closed

connection: add per-peer exponential reconnection backoff#951
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff

Conversation

@Jolah1

Copy link
Copy Markdown
Contributor

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

  • Introduces PeerReconnectState in connection.rs, tracked per peer in a Mutex<HashMap<PublicKey, PeerReconnectState>> on ConnectionManager.
  • Each consecutive failure schedules the next retry at the current backoff and doubles it, capped at PEER_RECONNECTION_MAX_INTERVAL (30 min).
  • The reconnect loop gates each peer on is_reconnect_due rather than attempting every peer every tick.
  • Backoff state is cleared on a successful connect (do_connect_peer), on explicit disconnect(), and when a peer is removed after its last channel closes.
  • Already-connected peers have their state cleared each tick, so an inbound
    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 on Config. Happy to wire them into Config as 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.rsChannelClosed handler. Once #895 lands I
will rebase and move the clear_reconnect_state call to sit alongside the
relocated removal (and pick up the now-async remove_peer.awaits from #919). Will un-draft after #895 is merged.

@ldk-reviews-bot

ldk-reviews-bot commented Jun 26, 2026

Copy link
Copy Markdown

👋 Hi! This PR is now in draft status.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@Jolah1
Jolah1 marked this pull request as draft June 26, 2026 08:01
@Jolah1
Jolah1 marked this pull request as ready for review July 1, 2026 13:26
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from d29aa81 to fec6507CompareJuly 1, 2026 14:39
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from fec6507 to 1172194CompareJuly 9, 2026 13:04
@Jolah1

Copy link
Copy Markdown
ContributorAuthor

Un-drafting now that #895 has landed. Changes since the initial draft:

Rebased on main. As promised above, the backoff clear that lived in
close_channel_internal moved to #895's relocated peer removal in the
ChannelClosed event handler, so backoff state is dropped exactly where the
peer leaves the store.

After another self-review pass:

  • Backoff bookkeeping is now anchored to the reconnection loop's tick instant
    instead of each attempt's completion time. Previously a first failure was
    scheduled at attempt_end + 60s, which always slipped past the next 60s
    tick and effectively doubled the first retry delay.
  • The loop now skips a peer that already has a connection attempt in flight
    (has_pending_connection) rather than subscribing to it. This avoids
    recording someone else's failure — possibly for a different, user-supplied
    address
    — as a failure of the persisted address.
  • Backoff state is pruned against the persisted peer store each tick. This
    guards a race where a failed in-flight attempt could resurrect state for a
    peer that was concurrently removed (leaking the entry), and makes cleanup
    self-healing if future code adds remove_peer call sites.
  • connect_peer_if_necessary clears backoff on its already-connected early
    return, since the peer is demonstrably reachable.
  • Simplified record_reconnect_attempt(&Result) into
    record_reconnect_failure called only on Err; do_connect_peer is now
    the single owner of clear-on-success.

One known limitation, documented in a comment: a peer that connects inbound
and drops again entirely within one tick keeps its accumulated backoff, since
connectivity is only observed at tick time. Clearing eagerly on inbound
connects would require hooking the LDK peer_connected handlers, which felt
out of scope here .

cc @tnull

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.
Closeslightningdevkit#918.
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from 1172194 to a9d0d13CompareJuly 10, 2026 04:40
@Jolah1

Jolah1 commented Jul 10, 2026

Copy link
Copy Markdown
ContributorAuthor

The build (macos-latest, stable) failure is the known reorg_test flake (tests/reorg_test.rs:170, "Unexpected balance state!") that #972 fixes.
The other two red jobs are fail-fast cancellations from it.

@Jolah1

Jolah1 commented Jul 16, 2026

Copy link
Copy Markdown
ContributorAuthor

Closing per the discussion on #918#983 covers the motivating case.
@tnull

@tnulltnull closed this Jul 17, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add configurable reconnection backoff for persisted peers

3 participants

@Jolah1@ldk-reviews-bot@tnull
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); connection: add per-peer exponential reconnection backoff by Jolah1 · Pull Request #951 · lightningdevkit/ldk-node · GitHub
Skip to content

connection: add per-peer exponential reconnection backoff - #951

Closed
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff
Closed

connection: add per-peer exponential reconnection backoff#951
Jolah1 wants to merge 1 commit into
lightningdevkit:mainfrom
Jolah1:peer-reconnection-backoff

Conversation

@Jolah1

Copy link
Copy Markdown
Contributor

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

  • Introduces PeerReconnectState in connection.rs, tracked per peer in a Mutex<HashMap<PublicKey, PeerReconnectState>> on ConnectionManager.
  • Each consecutive failure schedules the next retry at the current backoff and doubles it, capped at PEER_RECONNECTION_MAX_INTERVAL (30 min).
  • The reconnect loop gates each peer on is_reconnect_due rather than attempting every peer every tick.
  • Backoff state is cleared on a successful connect (do_connect_peer), on explicit disconnect(), and when a peer is removed after its last channel closes.
  • Already-connected peers have their state cleared each tick, so an inbound
    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 on Config. Happy to wire them into Config as 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.rsChannelClosed handler. Once #895 lands I
will rebase and move the clear_reconnect_state call to sit alongside the
relocated removal (and pick up the now-async remove_peer.awaits from #919). Will un-draft after #895 is merged.

@ldk-reviews-bot

ldk-reviews-bot commented Jun 26, 2026

Copy link
Copy Markdown

👋 Hi! This PR is now in draft status.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@Jolah1
Jolah1 marked this pull request as draft June 26, 2026 08:01
@Jolah1
Jolah1 marked this pull request as ready for review July 1, 2026 13:26
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from d29aa81 to fec6507CompareJuly 1, 2026 14:39
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from fec6507 to 1172194CompareJuly 9, 2026 13:04
@Jolah1

Copy link
Copy Markdown
ContributorAuthor

Un-drafting now that #895 has landed. Changes since the initial draft:

Rebased on main. As promised above, the backoff clear that lived in
close_channel_internal moved to #895's relocated peer removal in the
ChannelClosed event handler, so backoff state is dropped exactly where the
peer leaves the store.

After another self-review pass:

  • Backoff bookkeeping is now anchored to the reconnection loop's tick instant
    instead of each attempt's completion time. Previously a first failure was
    scheduled at attempt_end + 60s, which always slipped past the next 60s
    tick and effectively doubled the first retry delay.
  • The loop now skips a peer that already has a connection attempt in flight
    (has_pending_connection) rather than subscribing to it. This avoids
    recording someone else's failure — possibly for a different, user-supplied
    address
    — as a failure of the persisted address.
  • Backoff state is pruned against the persisted peer store each tick. This
    guards a race where a failed in-flight attempt could resurrect state for a
    peer that was concurrently removed (leaking the entry), and makes cleanup
    self-healing if future code adds remove_peer call sites.
  • connect_peer_if_necessary clears backoff on its already-connected early
    return, since the peer is demonstrably reachable.
  • Simplified record_reconnect_attempt(&Result) into
    record_reconnect_failure called only on Err; do_connect_peer is now
    the single owner of clear-on-success.

One known limitation, documented in a comment: a peer that connects inbound
and drops again entirely within one tick keeps its accumulated backoff, since
connectivity is only observed at tick time. Clearing eagerly on inbound
connects would require hooking the LDK peer_connected handlers, which felt
out of scope here .

cc @tnull

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.
Closeslightningdevkit#918.
@Jolah1
Jolah1force-pushed the peer-reconnection-backoff branch from 1172194 to a9d0d13CompareJuly 10, 2026 04:40
@Jolah1

Jolah1 commented Jul 10, 2026

Copy link
Copy Markdown
ContributorAuthor

The build (macos-latest, stable) failure is the known reorg_test flake (tests/reorg_test.rs:170, "Unexpected balance state!") that #972 fixes.
The other two red jobs are fail-fast cancellations from it.

@Jolah1

Jolah1 commented Jul 16, 2026

Copy link
Copy Markdown
ContributorAuthor

Closing per the discussion on #918#983 covers the motivating case.
@tnull

@tnulltnull closed this Jul 17, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add configurable reconnection backoff for persisted peers

3 participants

@Jolah1@ldk-reviews-bot@tnull