From 69a528f83118bda724b5428d4ed901e01ef0454d Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Mon, 20 Oct 2025 17:52:43 +0200 Subject: [PATCH] fix: topology manager uses actual connection count instead of filtered count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes bug #1962 where the topology manager's `adjust_topology()` function was using the filtered connection count (neighbor_locations.len()) instead of the actual connection count when making topology decisions. **Root Cause:** The `adjust_topology()` function received `neighbor_locations` which had been filtered to exclude connections younger than 5 seconds. During network startup, all connections are < 5 seconds old, so `neighbor_locations.len() = 0` even when the node has connections. This caused the manager to constantly try adding more connections. **Changes:** 1. Added `current_connections` parameter to `adjust_topology()` to pass the actual connection count 2. Updated min_connections check to use actual count instead of filtered count 3. Added early exit for small networks (< 5 connections) to prevent resource-based topology adjustments from destabilizing them 4. Updated all test calls to pass the connection count parameter 5. Updated logging to show both actual and filtered connection counts **Testing:** - All 8 topology module tests pass - Test coverage includes min_connections behavior, resource-based adjustments, and the no-duplicate-connections scenario Fixes #1962 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- crates/core/src/ring/mod.rs | 1 + crates/core/src/topology/mod.rs | 59 +++++++++++++++++++++++---------- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/crates/core/src/ring/mod.rs b/crates/core/src/ring/mod.rs index b750c951c..2b2eaf8b0 100644 --- a/crates/core/src/ring/mod.rs +++ b/crates/core/src/ring/mod.rs @@ -513,6 +513,7 @@ impl Ring { &neighbor_locations, &self.connection_manager.own_location().location, Instant::now(), + current_connections, ); tracing::info!( diff --git a/crates/core/src/topology/mod.rs b/crates/core/src/topology/mod.rs index 8e1316309..977f5f515 100644 --- a/crates/core/src/topology/mod.rs +++ b/crates/core/src/topology/mod.rs @@ -283,6 +283,7 @@ impl TopologyManager { neighbor_locations: &BTreeMap>, my_location: &Option, at_time: Instant, + current_connections: usize, ) -> TopologyAdjustment { #[cfg(debug_assertions)] { @@ -294,8 +295,9 @@ impl TopologyManager { { LAST_LOG.with(|last_log| { tracing::trace!( - "Adjusting topology at {:?}. Current neighbors: {:?}", + "Adjusting topology at {:?}. Current connections: {}, Filtered neighbors: {}", at_time, + current_connections, neighbor_locations.len() ); *last_log.borrow_mut() = Instant::now(); @@ -303,12 +305,12 @@ impl TopologyManager { } } - if neighbor_locations.len() < self.limits.min_connections { + if current_connections < self.limits.min_connections { let mut locations = Vec::new(); - let below_threshold = self.limits.min_connections - neighbor_locations.len(); + let below_threshold = self.limits.min_connections - current_connections; if below_threshold > 0 { // If we have no connections at all, bootstrap by targeting own location - if neighbor_locations.is_empty() { + if current_connections == 0 { match my_location { Some(location) => { // The first connect message should target the peer's own @@ -331,7 +333,7 @@ impl TopologyManager { LAST_LOG.with(|last_log| { tracing::trace!( minimum_num_peers_hard_limit = self.limits.min_connections, - num_peers = neighbor_locations.len(), + num_peers = current_connections, to_add = below_threshold, "Bootstrap: adding first connection at own location" ); @@ -341,7 +343,7 @@ impl TopologyManager { } } // If we have 1-4 connections, use random locations for diversity - else if neighbor_locations.len() < 5 { + else if current_connections < 5 { for _i in 0..below_threshold { locations.push(Location::random()); } @@ -356,7 +358,7 @@ impl TopologyManager { LAST_LOG.with(|last_log| { tracing::trace!( minimum_num_peers_hard_limit = self.limits.min_connections, - num_peers = neighbor_locations.len(), + num_peers = current_connections, to_add = below_threshold, "Early stage: adding connections at random locations for diversity" ); @@ -381,6 +383,16 @@ impl TopologyManager { return TopologyAdjustment::AddConnections(locations); } + // Skip resource-based removal in very small networks to avoid destabilizing them + // During startup or in small test networks, we need stability more than optimization + if current_connections < 5 { + debug!( + "Skipping resource-based topology adjustment for small network (connections: {})", + current_connections + ); + return TopologyAdjustment::NoChange; + } + let increase_usage_if_below: RateProportion = RateProportion::new(MINIMUM_DESIRED_RESOURCE_USAGE_PROPORTION); let decrease_usage_if_above: RateProportion = @@ -392,11 +404,10 @@ impl TopologyManager { debug!(?usage_proportion, "Resource usage information"); let adjustment: anyhow::Result = - if neighbor_locations.len() > self.limits.max_connections { + if current_connections > self.limits.max_connections { debug!( - "Number of neighbors ({:?}) is above maximum ({:?}), removing connections", - neighbor_locations.len(), - self.limits.max_connections + "Number of connections ({:?}) is above maximum ({:?}), removing connections", + current_connections, self.limits.max_connections ); self.update_connection_acquisition_strategy(ConnectionAcquisitionStrategy::Slow); @@ -721,8 +732,12 @@ mod tests { neighbor_locations.insert(peer.location.unwrap(), vec![]); } - let adjustment = - resource_manager.adjust_topology(&neighbor_locations, &None, Instant::now()); + let adjustment = resource_manager.adjust_topology( + &neighbor_locations, + &None, + Instant::now(), + peers.len(), + ); match adjustment { TopologyAdjustment::RemoveConnections(peers) => { assert_eq!(peers.len(), 1); @@ -766,8 +781,12 @@ mod tests { neighbor_locations.insert(peer.location.unwrap(), vec![]); } - let adjustment = - resource_manager.adjust_topology(&neighbor_locations, &None, Instant::now()); + let adjustment = resource_manager.adjust_topology( + &neighbor_locations, + &None, + Instant::now(), + peers.len(), + ); match adjustment { TopologyAdjustment::AddConnections(locations) => { @@ -807,8 +826,12 @@ mod tests { neighbor_locations.insert(peer.location.unwrap(), vec![]); } - let adjustment = - resource_manager.adjust_topology(&neighbor_locations, &None, report_time); + let adjustment = resource_manager.adjust_topology( + &neighbor_locations, + &None, + report_time, + peers.len(), + ); match adjustment { TopologyAdjustment::NoChange => {} @@ -848,6 +871,7 @@ mod tests { &neighbor_locations, &Some(my_location), report_time, + peers.len(), ); match adjustment { @@ -992,6 +1016,7 @@ mod tests { &neighbor_locations, &Some(Location::new(0.5)), Instant::now(), + 1, // 1 current connection ); match adjustment {