mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-30 19:46:15 +00:00
Make auto-connect peers retry indefinitely on initial connection failure
Previously, static peers configured with AutoConnect gave up after 6 attempts (1 initial + 5 retries). If the remote peer was offline at startup, the node permanently abandoned the connection. The reconnect path (after MMP link-dead) already retried indefinitely but only activated after a peer had been previously connected. Remove the reconnect parameter from schedule_retry() and always set reconnect=true when creating retry entries, since only auto-connect peers reach this code path. The 300s backoff cap prevents resource waste. The max_retries=0 config still works as an explicit kill switch.
This commit is contained in:
@@ -51,7 +51,7 @@ impl Node {
|
||||
if conn.is_outbound()
|
||||
&& let Some(identity) = conn.expected_identity()
|
||||
{
|
||||
self.schedule_retry(*identity.node_addr(), now_ms, false);
|
||||
self.schedule_retry(*identity.node_addr(), now_ms);
|
||||
}
|
||||
}
|
||||
self.cleanup_stale_connection(link_id, now_ms);
|
||||
|
||||
@@ -475,7 +475,7 @@ impl Node {
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_millis() as u64)
|
||||
.unwrap_or(0);
|
||||
self.schedule_retry(*pending.peer_identity.node_addr(), now_ms, false);
|
||||
self.schedule_retry(*pending.peer_identity.node_addr(), now_ms);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+3
-5
@@ -59,11 +59,10 @@ impl Node {
|
||||
&mut self,
|
||||
node_addr: NodeAddr,
|
||||
now_ms: u64,
|
||||
reconnect: bool,
|
||||
) {
|
||||
let retry_cfg = &self.config.node.retry;
|
||||
let max_retries = retry_cfg.max_retries;
|
||||
if max_retries == 0 && !reconnect {
|
||||
if max_retries == 0 {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -112,12 +111,11 @@ impl Node {
|
||||
if let Some(pc) = peer_config {
|
||||
let mut state = RetryState::new(pc);
|
||||
state.retry_count = 1;
|
||||
state.reconnect = reconnect;
|
||||
state.reconnect = true;
|
||||
let delay = state.backoff_ms(base_interval_ms, max_backoff_ms);
|
||||
state.retry_after_ms = now_ms + delay;
|
||||
debug!(
|
||||
peer = %self.peer_display_name(&node_addr),
|
||||
reconnect = reconnect,
|
||||
delay_secs = delay / 1000,
|
||||
"First connection attempt failed, scheduling retry"
|
||||
);
|
||||
@@ -237,7 +235,7 @@ impl Node {
|
||||
);
|
||||
// Immediate failure counts as an attempt — schedule next retry
|
||||
// (reconnect flag is preserved on existing retry_pending entry)
|
||||
self.schedule_retry(node_addr, now_ms, false);
|
||||
self.schedule_retry(node_addr, now_ms);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+17
-15
@@ -568,11 +568,12 @@ fn test_schedule_retry_creates_entry() {
|
||||
|
||||
assert!(node.retry_pending.is_empty());
|
||||
|
||||
node.schedule_retry(peer_node_addr, 1000, false);
|
||||
node.schedule_retry(peer_node_addr, 1000);
|
||||
|
||||
assert_eq!(node.retry_pending.len(), 1);
|
||||
let state = node.retry_pending.get(&peer_node_addr).unwrap();
|
||||
assert_eq!(state.retry_count, 1);
|
||||
assert!(state.reconnect, "Auto-connect peers always get reconnect=true");
|
||||
// Default base = 5s, 2^1 = 10s, but first retry is 2^0... let me check:
|
||||
// retry_count is set to 1, backoff_ms(5000) = 5000 * 2^1 = 10000
|
||||
assert_eq!(state.retry_after_ms, 1000 + 10_000);
|
||||
@@ -595,20 +596,20 @@ fn test_schedule_retry_increments() {
|
||||
let mut node = Node::new(config).unwrap();
|
||||
|
||||
// First failure
|
||||
node.schedule_retry(peer_node_addr, 1000, false);
|
||||
node.schedule_retry(peer_node_addr, 1000);
|
||||
assert_eq!(node.retry_pending.get(&peer_node_addr).unwrap().retry_count, 1);
|
||||
|
||||
// Second failure
|
||||
node.schedule_retry(peer_node_addr, 11_000, false);
|
||||
node.schedule_retry(peer_node_addr, 11_000);
|
||||
let state = node.retry_pending.get(&peer_node_addr).unwrap();
|
||||
assert_eq!(state.retry_count, 2);
|
||||
// backoff_ms(5000) with retry_count=2 = 5000 * 4 = 20000
|
||||
assert_eq!(state.retry_after_ms, 11_000 + 20_000);
|
||||
}
|
||||
|
||||
/// Test that schedule_retry gives up after max_retries.
|
||||
/// Test that auto-connect peers retry indefinitely (never exhaust).
|
||||
#[test]
|
||||
fn test_schedule_retry_max_retries_exhausted() {
|
||||
fn test_schedule_retry_auto_connect_never_exhausts() {
|
||||
let peer_identity = Identity::generate();
|
||||
let peer_npub = peer_identity.npub();
|
||||
let peer_node_addr = *PeerIdentity::from_npub(&peer_npub).unwrap().node_addr();
|
||||
@@ -623,19 +624,20 @@ fn test_schedule_retry_max_retries_exhausted() {
|
||||
|
||||
let mut node = Node::new(config).unwrap();
|
||||
|
||||
// Attempts 1 and 2 should schedule retries
|
||||
node.schedule_retry(peer_node_addr, 1000, false);
|
||||
// All attempts should keep the entry alive despite max_retries=2
|
||||
node.schedule_retry(peer_node_addr, 1000);
|
||||
assert!(node.retry_pending.contains_key(&peer_node_addr));
|
||||
|
||||
node.schedule_retry(peer_node_addr, 2000, false);
|
||||
node.schedule_retry(peer_node_addr, 2000);
|
||||
assert!(node.retry_pending.contains_key(&peer_node_addr));
|
||||
|
||||
// Attempt 3 exceeds max_retries=2, should remove entry
|
||||
node.schedule_retry(peer_node_addr, 3000, false);
|
||||
// Attempt 3 would have exhausted before, but now retries indefinitely
|
||||
node.schedule_retry(peer_node_addr, 3000);
|
||||
assert!(
|
||||
!node.retry_pending.contains_key(&peer_node_addr),
|
||||
"Should be removed after max retries exhausted"
|
||||
node.retry_pending.contains_key(&peer_node_addr),
|
||||
"Auto-connect peers should never exhaust retries"
|
||||
);
|
||||
assert_eq!(node.retry_pending.get(&peer_node_addr).unwrap().retry_count, 3);
|
||||
}
|
||||
|
||||
/// Test that schedule_retry does nothing when max_retries is 0.
|
||||
@@ -655,7 +657,7 @@ fn test_schedule_retry_disabled() {
|
||||
|
||||
let mut node = Node::new(config).unwrap();
|
||||
|
||||
node.schedule_retry(peer_node_addr, 1000, false);
|
||||
node.schedule_retry(peer_node_addr, 1000);
|
||||
assert!(
|
||||
node.retry_pending.is_empty(),
|
||||
"No retry should be scheduled when max_retries=0"
|
||||
@@ -671,7 +673,7 @@ fn test_schedule_retry_ignores_non_autoconnect() {
|
||||
// No peers configured at all
|
||||
let mut node = make_node();
|
||||
|
||||
node.schedule_retry(peer_node_addr, 1000, false);
|
||||
node.schedule_retry(peer_node_addr, 1000);
|
||||
assert!(
|
||||
node.retry_pending.is_empty(),
|
||||
"No retry for unconfigured peer"
|
||||
@@ -693,7 +695,7 @@ fn test_schedule_retry_skips_connected_peer() {
|
||||
assert_eq!(node.peer_count(), 1);
|
||||
|
||||
// Scheduling a retry for an already-connected peer should be a no-op
|
||||
node.schedule_retry(node_addr, 3000, false);
|
||||
node.schedule_retry(node_addr, 3000);
|
||||
assert!(
|
||||
node.retry_pending.is_empty(),
|
||||
"No retry for already-connected peer"
|
||||
|
||||
Reference in New Issue
Block a user