Skip to content

Commit

Permalink
Merge #2378: Fix for network discovery state machine spin loop
Browse files Browse the repository at this point in the history
If the node is unable to obtain many peers it continues to aggresively
ask for peers. On a small localhost network this can cause high CPU
usage as it repeatedly queries for peers in a state loop. This fix adds
checks in the Ready state that allow a transition to Idle if: there
are no sync peers, there are no sync peers other than the ones used last
time, or there have been 10 rounds in a row where the node has not been
able to obtain "enough" peers to enter "on connect" mode.
  • Loading branch information
stringhandler committed Oct 23, 2020
2 parents c9a94b9 + 6a8f7eb commit a495c98
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 18 deletions.
46 changes: 34 additions & 12 deletions comms/dht/src/network_discovery/ready.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,17 @@ impl DiscoveryReady {

// We don't have many peers - let's aggressively probe for them
if num_peers < self.context.config.network_discovery.min_desired_peers {
if self.context.num_rounds() >= self.config().network_discovery.idle_after_num_rounds {
warn!(
target: LOG_TARGET,
"Still unable to obtain at minimum desired peers ({}) after {} rounds. Idling...",
self.config().network_discovery.min_desired_peers,
self.context.num_rounds(),
);
self.context.reset_num_rounds();
return Ok(StateEvent::Idle);
}

let peers = self
.context
.peer_manager
Expand All @@ -68,10 +79,21 @@ impl DiscoveryReady {
self.previous_sync_peers(),
)
.await?;
let peers = peers.into_iter().map(|p| p.node_id).collect::<Vec<_>>();

if peers.is_empty() {
debug!(
target: LOG_TARGET,
"No more sync peers after round #{}. Idling...",
self.context.num_rounds()
);
return Ok(StateEvent::Idle);
}

return Ok(StateEvent::BeginDiscovery(DiscoveryParams {
// All peers
num_peers_to_request: None,
peers: peers.into_iter().map(|p| p.node_id).collect(),
peers,
}));
}

Expand Down Expand Up @@ -124,7 +146,7 @@ impl DiscoveryReady {
.await?
.into_iter()
.map(|p| p.node_id)
.collect()
.collect::<Vec<_>>()
} else {
debug!(
target: LOG_TARGET,
Expand All @@ -148,19 +170,19 @@ impl DiscoveryReady {
.await?
.into_iter()
.map(|p| p.node_id)
.collect()
.collect::<Vec<_>>()
},
};

// let max_accept_closer_peers = cmp::max(
// // Want to get to the min_desired_peers as quickly as possible
// self.config()
// .network_discovery
// .min_desired_peers
// .saturating_sub(num_peers),
// // Otherwise we want to be a bit more 'cautious' about accepting neighbouring peers
// self.config().num_neighbouring_nodes,
// );
if peers.is_empty() {
debug!(
target: LOG_TARGET,
"No more sync peers after round #{}. Idling...",
self.context.num_rounds()
);
return Ok(StateEvent::Idle);
}

Ok(StateEvent::BeginDiscovery(DiscoveryParams {
// Request all peers
num_peers_to_request: None,
Expand Down
19 changes: 13 additions & 6 deletions comms/dht/src/network_discovery/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ mod discovery_ready {
state_machine::{NetworkDiscoveryContext, StateEvent},
DhtNetworkDiscoveryRoundInfo,
};
use tari_comms::test_utils::mocks::ConnectivityManagerMock;
use tari_comms::test_utils::{mocks::ConnectivityManagerMock, node_identity::build_many_node_identities};

fn setup(
config: NetworkDiscoveryConfig,
Expand Down Expand Up @@ -201,16 +201,23 @@ mod discovery_ready {

#[tokio_macros::test_basic]
async fn it_begins_aggressive_discovery() {
let config = NetworkDiscoveryConfig {
min_desired_peers: 10,
..Default::default()
};
let (_, _, _, mut ready, _) = setup(config);
let (_, pm, _, mut ready, _) = setup(Default::default());
let peers = build_many_node_identities(1, PeerFeatures::COMMUNICATION_NODE);
for peer in peers {
pm.add_peer(peer.to_peer()).await.unwrap();
}
let state_event = ready.next_event().await;
unpack_enum!(StateEvent::BeginDiscovery(params) = state_event);
assert!(params.num_peers_to_request.is_none());
}

#[tokio_macros::test_basic]
async fn it_idles_if_no_sync_peers() {
let (_, _, _, mut ready, _) = setup(Default::default());
let state_event = ready.next_event().await;
unpack_enum!(StateEvent::Idle = state_event);
}

#[tokio_macros::test_basic]
async fn it_idles_if_num_rounds_reached() {
let config = NetworkDiscoveryConfig {
Expand Down

0 comments on commit a495c98

Please sign in to comment.