Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add a test for lots of nodes connecting at the same time #6247

Merged
merged 2 commits into from
Jun 9, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions client/network/src/service/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,3 +272,73 @@ fn notifications_state_consistent() {
}
});
}

#[test]
fn lots_of_incoming_peers_works() {
let listen_addr = config::build_multiaddr![Memory(rand::random::<u64>())];

let (main_node, _) = build_test_full_node(config::NetworkConfiguration {
notifications_protocols: vec![(ENGINE_ID, From::from(&b"/foo"[..]))],
listen_addresses: vec![listen_addr.clone()],
in_peers: u32::max_value(),
transport: config::TransportConfig::MemoryOnly,
.. config::NetworkConfiguration::new_local()
});

let main_node_peer_id = main_node.local_peer_id().clone();

// We spawn background tasks and push them in this `Vec`. They will all be waited upon before
// this test ends.
let mut background_tasks_to_wait = Vec::new();

for _ in 0..256 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned in another conversation already, the test often executes very slowly on my machine. Even if I change this line to for _ in 0..3 { I sometimes (but not always) get execution times like this:

> time cargo test lots_of_incoming_peers_works
    Finished test [unoptimized + debuginfo] target(s) in 0.28s
     Running /home/user/workspace/substrate/target/debug/deps/sc_network-53c6b71fe56316d8

running 1 test
test service::tests::lots_of_incoming_peers_works ... test service::tests::lots_of_incoming_peers_works has been running for over 60 seconds
test service::tests::lots_of_incoming_peers_works ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 42 filtered out


________________________________________________________
Executed in   67.74 secs   fish           external
   usr time  1298.09 millis  288.00 micros  1297.81 millis
   sys time  193.63 millis  147.00 micros  193.49 millis

It makes me wonder if some task is not properly woken up at some point.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@twittner Just curious since I haven't tried running it at all, but which version of async-std are you running this with? I'm asking because the dev-dependencies of this crate state async-std = "1.5" which allows 1.6 which seems to have some issues (e.g. the libp2p noise smoke tests stalled completely with that version).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async-std version 1.5.0, cf. Cargo.lock.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think there's anything wrong in the test itself?
I don't see any problem and I'd rather suspect an issue in the networking code as a whole.
If you agree, I suggest to go on with this PR and open an issue?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the test is probably not to blame for this. Since CI seems to cope this is not a blocker.

let main_node_peer_id = main_node_peer_id.clone();

let (_dialing_node, event_stream) = build_test_full_node(config::NetworkConfiguration {
notifications_protocols: vec![(ENGINE_ID, From::from(&b"/foo"[..]))],
listen_addresses: vec![],
reserved_nodes: vec![config::MultiaddrWithPeerId {
multiaddr: listen_addr.clone(),
peer_id: main_node_peer_id.clone(),
}],
transport: config::TransportConfig::MemoryOnly,
.. config::NetworkConfiguration::new_local()
});

background_tasks_to_wait.push(async_std::task::spawn(async move {
// Create a dummy timer that will "never" fire, and that will be overwritten when we
// actually need the timer. Using an Option would be technically cleaner, but it would
// make the code below way more complicated.
let mut timer = futures_timer::Delay::new(Duration::from_secs(3600 * 24 * 7)).fuse();

let mut event_stream = event_stream.fuse();
loop {
futures::select! {
_ = timer => {
// Test succeeds when timer fires.
return;
}
ev = event_stream.next() => {
match ev.unwrap() {
Event::NotificationStreamOpened { remote, .. } => {
assert_eq!(remote, main_node_peer_id);
// Test succeeds after 5 seconds. This timer is here in order to
// detect a potential problem after opening.
timer = futures_timer::Delay::new(Duration::from_secs(5)).fuse();
}
Event::NotificationStreamClosed { .. } => {
// Test failed.
panic!();
}
_ => {}
}
}
}
}
}));
}

futures::executor::block_on(async move {
future::join_all(background_tasks_to_wait).await
});
}