From 6f0fbbff5360f03b5cef7f74848fe8452ad0b3bf Mon Sep 17 00:00:00 2001 From: Guilherme Lawless Date: Wed, 29 Apr 2020 14:46:53 +0100 Subject: [PATCH] Fix minor test-specific intermittent failures (#2748) * Fix intermittent failures in websocket.active_difficulty test * Fix intermittent failures in websocket.bootstrap_exited test due to attempt finishing immediately * Fix work_watcher.propagate by allowing higher max work generation difficulty * Apply similar fix to node_telemetry.remove_peer_different_genesis_udp as the tcp version in https://github.com/nanocurrency/nano-node/pull/2738 * Move node_telemetry.all_peers_use_single_request_cache to slow_tests as it takes too long * Fix intermittent failure in active_transactions.activate_dependencies due to the election being removed before full cementing (still in conf height processor) * Don't check network size, peers are added for exclusion but they can still reconnect via UDP; explicitly request telemetry to make the test faster (UDP does not request telemetry on connection --- nano/core_test/active_transactions.cpp | 4 +- nano/core_test/node_telemetry.cpp | 102 ++++++------------------- nano/core_test/wallet.cpp | 1 + nano/core_test/websocket.cpp | 23 ++++-- nano/slow_test/node.cpp | 71 +++++++++++++++++ 5 files changed, 114 insertions(+), 87 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index b704f40d9..f053087f2 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -668,8 +668,8 @@ TEST (active_transactions, activate_dependencies) { ASSERT_NO_ERROR (system.poll ()); } - ASSERT_TRUE (node1->ledger.block_confirmed (node1->store.tx_begin_read (), block2->hash ())); - ASSERT_TRUE (node2->ledger.block_confirmed (node2->store.tx_begin_read (), block2->hash ())); + ASSERT_TRUE (node1->block_confirmed_or_being_confirmed (node1->store.tx_begin_read (), block2->hash ())); + ASSERT_TRUE (node2->block_confirmed_or_being_confirmed (node2->store.tx_begin_read (), block2->hash ())); } namespace nano diff --git a/nano/core_test/node_telemetry.cpp b/nano/core_test/node_telemetry.cpp index 081a0384c..0ee89e494 100644 --- a/nano/core_test/node_telemetry.cpp +++ b/nano/core_test/node_telemetry.cpp @@ -511,77 +511,6 @@ TEST (node_telemetry, disconnects) } } -TEST (node_telemetry, all_peers_use_single_request_cache) -{ - nano::system system; - nano::node_flags node_flags; - node_flags.disable_ongoing_telemetry_requests = true; - node_flags.disable_initial_telemetry_requests = true; - auto node_client = system.add_node (node_flags); - auto node_server = system.add_node (node_flags); - - wait_peer_connections (system); - - // Request telemetry metrics - nano::telemetry_data telemetry_data; - { - std::atomic done{ false }; - auto channel = node_client->network.find_channel (node_server->network.endpoint ()); - node_client->telemetry->get_metrics_single_peer_async (channel, [&done, &telemetry_data](nano::telemetry_data_response const & response_a) { - telemetry_data = response_a.telemetry_data; - done = true; - }); - - system.deadline_set (10s); - while (!done) - { - ASSERT_NO_ERROR (system.poll ()); - } - } - - auto responses = node_client->telemetry->get_metrics (); - ASSERT_EQ (telemetry_data, responses.begin ()->second); - - // Confirm only 1 request was made - ASSERT_EQ (1, node_client->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_ack, nano::stat::dir::in)); - ASSERT_EQ (0, node_client->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::in)); - ASSERT_EQ (1, node_client->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::out)); - ASSERT_EQ (0, node_server->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_ack, nano::stat::dir::in)); - ASSERT_EQ (1, node_server->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::in)); - ASSERT_EQ (0, node_server->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::out)); - - std::this_thread::sleep_for (node_server->telemetry->cache_plus_buffer_cutoff_time ()); - - // Should be empty - responses = node_client->telemetry->get_metrics (); - ASSERT_TRUE (responses.empty ()); - - { - std::atomic done{ false }; - auto channel = node_client->network.find_channel (node_server->network.endpoint ()); - node_client->telemetry->get_metrics_single_peer_async (channel, [&done, &telemetry_data](nano::telemetry_data_response const & response_a) { - telemetry_data = response_a.telemetry_data; - done = true; - }); - - system.deadline_set (10s); - while (!done) - { - ASSERT_NO_ERROR (system.poll ()); - } - } - - responses = node_client->telemetry->get_metrics (); - ASSERT_EQ (telemetry_data, responses.begin ()->second); - - ASSERT_EQ (2, node_client->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_ack, nano::stat::dir::in)); - ASSERT_EQ (0, node_client->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::in)); - ASSERT_EQ (2, node_client->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::out)); - ASSERT_EQ (0, node_server->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_ack, nano::stat::dir::in)); - ASSERT_EQ (2, node_server->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::in)); - ASSERT_EQ (0, node_server->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::out)); -} - TEST (node_telemetry, dos_tcp) { // Confirm that telemetry_reqs are not processed @@ -753,11 +682,13 @@ TEST (node_telemetry, remove_peer_different_genesis) ASSERT_EQ (1, node1->network.excluded_peers.peers.get ().count (node0->network.endpoint ().address ())); } +// Peer exclusion is only fully supported for TCP-only nodes; peers can still reconnect through UDP TEST (node_telemetry, remove_peer_different_genesis_udp) { nano::node_flags node_flags; node_flags.disable_udp = false; node_flags.disable_tcp_realtime = true; + node_flags.disable_ongoing_telemetry_requests = true; nano::system system (1, nano::transport::transport_type::udp, node_flags); auto node0 (system.nodes[0]); ASSERT_EQ (0, node0->network.size ()); @@ -765,21 +696,34 @@ TEST (node_telemetry, remove_peer_different_genesis_udp) node1->network_params.ledger.genesis_hash = nano::block_hash ("0"); node1->start (); system.nodes.push_back (node1); - node0->network.send_keepalive (std::make_shared (node0->network.udp_channels, node1->network.endpoint (), node1->network_params.protocol.protocol_version)); - node1->network.send_keepalive (std::make_shared (node1->network.udp_channels, node0->network.endpoint (), node0->network_params.protocol.protocol_version)); + auto channel0 (std::make_shared (node1->network.udp_channels, node0->network.endpoint (), node0->network_params.protocol.protocol_version)); + auto channel1 (std::make_shared (node0->network.udp_channels, node1->network.endpoint (), node1->network_params.protocol.protocol_version)); + node0->network.send_keepalive (channel1); + node1->network.send_keepalive (channel0); ASSERT_TIMELY (10s, node0->network.udp_channels.size () != 0 && node1->network.udp_channels.size () != 0); ASSERT_EQ (node0->network.tcp_channels.size (), 0); ASSERT_EQ (node1->network.tcp_channels.size (), 0); - ASSERT_TIMELY (10s, node0->stats.count (nano::stat::type::telemetry, nano::stat::detail::different_genesis_hash) != 0 && node1->stats.count (nano::stat::type::telemetry, nano::stat::detail::different_genesis_hash) != 0); + std::atomic done0{ false }; + std::atomic done1{ false }; - ASSERT_EQ (0, node0->network.size ()); - ASSERT_EQ (0, node1->network.size ()); - system.poll_until_true (3s, [&node0, address = node1->network.endpoint ().address ()]() -> bool { - nano::lock_guard guard (node0->network.excluded_peers.mutex); - return node0->network.excluded_peers.peers.get ().count (address); + node0->telemetry->get_metrics_single_peer_async (channel1, [&done0](nano::telemetry_data_response const & response_a) { + done0 = true; }); + + node1->telemetry->get_metrics_single_peer_async (channel0, [&done1](nano::telemetry_data_response const & response_a) { + done1 = true; + }); + + ASSERT_TIMELY (10s, done0 && done1); + + ASSERT_EQ (node0->network.tcp_channels.size (), 0); + ASSERT_EQ (node1->network.tcp_channels.size (), 0); + + nano::lock_guard guard (node0->network.excluded_peers.mutex); + ASSERT_EQ (1, node0->network.excluded_peers.peers.get ().count (node1->network.endpoint ().address ())); + ASSERT_EQ (1, node1->network.excluded_peers.peers.get ().count (node0->network.endpoint ().address ())); } TEST (node_telemetry, remove_peer_invalid_signature) diff --git a/nano/core_test/wallet.cpp b/nano/core_test/wallet.cpp index ba2eb3dc1..79e05e436 100644 --- a/nano/core_test/wallet.cpp +++ b/nano/core_test/wallet.cpp @@ -1212,6 +1212,7 @@ TEST (work_watcher, propagate) nano::node_config node_config (nano::get_available_port (), system.logging); node_config.enable_voting = false; node_config.work_watcher_period = 1s; + node_config.max_work_generate_multiplier = 1e6; nano::node_flags node_flags; node_flags.disable_request_loop = true; auto & node = *system.add_node (node_config, node_flags); diff --git a/nano/core_test/websocket.cpp b/nano/core_test/websocket.cpp index 5d19c4f0d..f35f406c3 100644 --- a/nano/core_test/websocket.cpp +++ b/nano/core_test/websocket.cpp @@ -59,7 +59,11 @@ TEST (websocket, active_difficulty) nano::node_config config (nano::get_available_port (), system.logging); config.websocket_config.enabled = true; config.websocket_config.port = nano::get_available_port (); - auto node1 (system.add_node (config)); + nano::node_flags node_flags; + // Disable auto-updating active difficulty (multiplier) to prevent intermittent failures + node_flags.disable_request_loop = true; + auto node1 (system.add_node (config, node_flags)); + // "Start" epoch 2 node1->ledger.cache.epoch_2_started = true; ASSERT_EQ (node1->default_difficulty (nano::work_version::work_1), node1->network_params.network.publish_thresholds.epoch_2); @@ -83,10 +87,11 @@ TEST (websocket, active_difficulty) ASSERT_NO_ERROR (system.poll ()); } - // Fake history records to force trended_active_multiplier change + // Fake history records and force a trended_active_multiplier change { nano::unique_lock lock (node1->active.mutex); node1->active.multipliers_cb.push_front (10.); + node1->active.update_active_multiplier (lock); } system.deadline_set (5s); @@ -775,10 +780,16 @@ TEST (websocket, bootstrap_exited) // Start bootstrap, exit after subscription std::atomic bootstrap_started{ false }; nano::util::counted_completion subscribed_completion (1); - std::thread bootstrap_thread ([node1, &bootstrap_started, &subscribed_completion]() { - node1->bootstrap_initiator.bootstrap (true, "123abc"); - auto attempt (node1->bootstrap_initiator.current_attempt ()); - EXPECT_NE (nullptr, attempt); + std::thread bootstrap_thread ([node1, &system, &bootstrap_started, &subscribed_completion]() { + std::shared_ptr attempt; + system.deadline_set (5s); + while (attempt == nullptr) + { + ASSERT_NO_ERROR (system.poll ()); + node1->bootstrap_initiator.bootstrap (true, "123abc"); + attempt = node1->bootstrap_initiator.current_attempt (); + } + ASSERT_NE (nullptr, attempt); bootstrap_started = true; EXPECT_FALSE (subscribed_completion.await_count_for (5s)); }); diff --git a/nano/slow_test/node.cpp b/nano/slow_test/node.cpp index 4c69f300d..071dec660 100644 --- a/nano/slow_test/node.cpp +++ b/nano/slow_test/node.cpp @@ -1322,3 +1322,74 @@ TEST (node, mass_epoch_upgrader) perform_test (42); perform_test (std::numeric_limits::max ()); } + +TEST (node_telemetry, all_peers_use_single_request_cache) +{ + nano::system system; + nano::node_flags node_flags; + node_flags.disable_ongoing_telemetry_requests = true; + node_flags.disable_initial_telemetry_requests = true; + auto node_client = system.add_node (node_flags); + auto node_server = system.add_node (node_flags); + + wait_peer_connections (system); + + // Request telemetry metrics + nano::telemetry_data telemetry_data; + { + std::atomic done{ false }; + auto channel = node_client->network.find_channel (node_server->network.endpoint ()); + node_client->telemetry->get_metrics_single_peer_async (channel, [&done, &telemetry_data](nano::telemetry_data_response const & response_a) { + telemetry_data = response_a.telemetry_data; + done = true; + }); + + system.deadline_set (10s); + while (!done) + { + ASSERT_NO_ERROR (system.poll ()); + } + } + + auto responses = node_client->telemetry->get_metrics (); + ASSERT_EQ (telemetry_data, responses.begin ()->second); + + // Confirm only 1 request was made + ASSERT_EQ (1, node_client->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_ack, nano::stat::dir::in)); + ASSERT_EQ (0, node_client->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::in)); + ASSERT_EQ (1, node_client->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::out)); + ASSERT_EQ (0, node_server->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_ack, nano::stat::dir::in)); + ASSERT_EQ (1, node_server->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::in)); + ASSERT_EQ (0, node_server->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::out)); + + std::this_thread::sleep_for (node_server->telemetry->cache_plus_buffer_cutoff_time ()); + + // Should be empty + responses = node_client->telemetry->get_metrics (); + ASSERT_TRUE (responses.empty ()); + + { + std::atomic done{ false }; + auto channel = node_client->network.find_channel (node_server->network.endpoint ()); + node_client->telemetry->get_metrics_single_peer_async (channel, [&done, &telemetry_data](nano::telemetry_data_response const & response_a) { + telemetry_data = response_a.telemetry_data; + done = true; + }); + + system.deadline_set (10s); + while (!done) + { + ASSERT_NO_ERROR (system.poll ()); + } + } + + responses = node_client->telemetry->get_metrics (); + ASSERT_EQ (telemetry_data, responses.begin ()->second); + + ASSERT_EQ (2, node_client->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_ack, nano::stat::dir::in)); + ASSERT_EQ (0, node_client->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::in)); + ASSERT_EQ (2, node_client->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::out)); + ASSERT_EQ (0, node_server->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_ack, nano::stat::dir::in)); + ASSERT_EQ (2, node_server->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::in)); + ASSERT_EQ (0, node_server->stats.count (nano::stat::type::message, nano::stat::detail::telemetry_req, nano::stat::dir::out)); +}