From c6f88b7162f9cbd54fe5e8b70ed46a73e1fc0d96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Wed, 24 Apr 2024 15:33:19 +0200 Subject: [PATCH] Remove special semantics of `stat::detail::all` --- nano/core_test/confirming_set.cpp | 2 +- nano/core_test/node.cpp | 2 +- nano/core_test/stats.cpp | 12 +++++++---- nano/lib/stats.cpp | 33 +++++++++++++++++-------------- nano/lib/stats.hpp | 21 +++----------------- nano/lib/stats_enums.hpp | 9 ++++++--- nano/node/transport/socket.cpp | 4 ++-- nano/rpc_test/rpc.cpp | 6 +++--- nano/slow_test/node.cpp | 12 +++++------ 9 files changed, 48 insertions(+), 53 deletions(-) diff --git a/nano/core_test/confirming_set.cpp b/nano/core_test/confirming_set.cpp index 6fe35bebfb..7b49769767 100644 --- a/nano/core_test/confirming_set.cpp +++ b/nano/core_test/confirming_set.cpp @@ -102,7 +102,7 @@ TEST (confirmation_callback, observer_callbacks) node->confirming_set.add (send1->hash ()); // Callback is performed for all blocks that are confirmed - ASSERT_TIMELY_EQ (5s, 2, node->ledger.stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::all, nano::stat::dir::out)); + ASSERT_TIMELY_EQ (5s, 2, node->ledger.stats.count (nano::stat::type::confirmation_observer, nano::stat::dir::out)); ASSERT_EQ (2, node->stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in)); ASSERT_EQ (3, node->ledger.cemented_count ()); diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index e83a34cb6b..8853ec1581 100644 --- a/nano/core_test/node.cpp +++ b/nano/core_test/node.cpp @@ -2418,7 +2418,7 @@ TEST (node, DISABLED_fork_invalid_block_signature) // Send the vote with the corrupt block signature node2.network.flood_vote (vote_corrupt, 1.0f); // Wait for the rollback - ASSERT_TIMELY (5s, node1.stats.count (nano::stat::type::rollback, nano::stat::detail::all)); + ASSERT_TIMELY (5s, node1.stats.count (nano::stat::type::rollback)); // Send the vote with the correct block node2.network.flood_vote (vote, 1.0f); ASSERT_TIMELY (10s, !node1.block (send1->hash ())); diff --git a/nano/core_test/stats.cpp b/nano/core_test/stats.cpp index 0cdef2cc14..b099300c20 100644 --- a/nano/core_test/stats.cpp +++ b/nano/core_test/stats.cpp @@ -10,15 +10,19 @@ TEST (stats, stat_counting) { nano::test::system system (1); auto & node1 (*system.nodes[0]); - node1.stats.add (nano::stat::type::ledger, nano::stat::dir::in, 1); - node1.stats.add (nano::stat::type::ledger, nano::stat::dir::in, 5); - node1.stats.inc (nano::stat::type::ledger, nano::stat::dir::in); + + node1.stats.add (nano::stat::type::ledger, nano::stat::detail::test, nano::stat::dir::in, 1); + node1.stats.add (nano::stat::type::ledger, nano::stat::detail::test, nano::stat::dir::in, 5); + node1.stats.inc (nano::stat::type::ledger, nano::stat::detail::test, nano::stat::dir::in); node1.stats.inc (nano::stat::type::ledger, nano::stat::detail::send, nano::stat::dir::in); node1.stats.inc (nano::stat::type::ledger, nano::stat::detail::send, nano::stat::dir::in); node1.stats.inc (nano::stat::type::ledger, nano::stat::detail::receive, nano::stat::dir::in); + ASSERT_EQ (10, node1.stats.count (nano::stat::type::ledger, nano::stat::dir::in)); ASSERT_EQ (2, node1.stats.count (nano::stat::type::ledger, nano::stat::detail::send, nano::stat::dir::in)); ASSERT_EQ (1, node1.stats.count (nano::stat::type::ledger, nano::stat::detail::receive, nano::stat::dir::in)); - node1.stats.add (nano::stat::type::ledger, nano::stat::dir::in, 0); + + node1.stats.add (nano::stat::type::ledger, nano::stat::detail::test, nano::stat::dir::in, 0); + ASSERT_EQ (10, node1.stats.count (nano::stat::type::ledger, nano::stat::dir::in)); } \ No newline at end of file diff --git a/nano/lib/stats.cpp b/nano/lib/stats.cpp index f375f4f7ff..5bd92b20da 100644 --- a/nano/lib/stats.cpp +++ b/nano/lib/stats.cpp @@ -84,13 +84,6 @@ void nano::stats::add (stat::type type, stat::detail detail, stat::dir dir, coun { updater (*it->second); - if (key != all_key) - { - auto it_all = counters.find (all_key); - release_assert (it_all != counters.end ()); // The `all` counter should always be created together - updater (*it_all->second); // Also update the `all` counter - } - return; } } @@ -100,14 +93,7 @@ void nano::stats::add (stat::type type, stat::detail detail, stat::dir dir, coun // Insertions will be ignored if the key already exists auto [it, inserted] = counters.emplace (key, std::make_unique ()); - auto [it_all, inserted_all] = counters.emplace (all_key, std::make_unique ()); - updater (*it->second); - - if (key != all_key) - { - updater (*it_all->second); // Also update the `all` counter - } } }; @@ -116,7 +102,7 @@ void nano::stats::add (stat::type type, stat::detail detail, stat::dir dir, coun }); } -auto nano::stats::count (stat::type type, stat::detail detail, stat::dir dir) const -> counter_value_t +nano::stats::counter_value_t nano::stats::count (stat::type type, stat::detail detail, stat::dir dir) const { std::shared_lock lock{ mutex }; if (auto it = counters.find (counter_key{ type, detail, dir }); it != counters.end ()) @@ -126,6 +112,22 @@ auto nano::stats::count (stat::type type, stat::detail detail, stat::dir dir) co return 0; } +nano::stats::counter_value_t nano::stats::count (stat::type type, stat::dir dir) const +{ + std::shared_lock lock{ mutex }; + counter_value_t result = 0; + auto it = counters.lower_bound (counter_key{ type, stat::detail::all, dir }); + while (it != counters.end () && it->first.type == type) + { + if (it->first.dir == dir) + { + result += it->second->value; + } + ++it; + } + return result; +} + void nano::stats::sample (stat::sample sample, std::pair expected_min_max, nano::stats::sampler_value_t value) { // Updates need to happen while holding the mutex @@ -137,6 +139,7 @@ void nano::stats::sample (stat::sample sample, std::pairsecond); + return; } } diff --git a/nano/lib/stats.hpp b/nano/lib/stats.hpp index 633c5c2b2a..cc791be752 100644 --- a/nano/lib/stats.hpp +++ b/nano/lib/stats.hpp @@ -80,12 +80,6 @@ class stats final /** Clear all stats */ void clear (); - /** Increments the given counter */ - void inc (stat::type type, stat::dir dir = stat::dir::in) - { - add (type, dir, 1); - } - /** Increments the given counter */ void inc (stat::type type, stat::detail detail, stat::dir dir = stat::dir::in) { @@ -98,24 +92,15 @@ class stats final add (type, detail, stat::dir::in, value); } - /** Adds \p value to the given counter */ - void add (stat::type type, stat::dir dir, counter_value_t value) - { - add (type, stat::detail::all, dir, value); - } - /** Adds \p value to the given counter */ void add (stat::type type, stat::detail detail, stat::dir dir, counter_value_t value); - /** Returns current value for the given counter at the type level */ - counter_value_t count (stat::type type, stat::dir dir = stat::dir::in) const - { - return count (type, stat::detail::all, dir); - } - /** Returns current value for the given counter at the detail level */ counter_value_t count (stat::type type, stat::detail detail, stat::dir dir = stat::dir::in) const; + /** Returns current value for the given counter at the type level (sum of all details) */ + counter_value_t count (stat::type type, stat::dir dir = stat::dir::in) const; + /** Adds a sample to the given sampler */ void sample (stat::sample sample, std::pair expected_min_max, sampler_value_t value); diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index 158bd5af79..ae37762946 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -10,6 +10,8 @@ namespace nano::stat /** Primary statistics type */ enum class type { + _invalid = 0, // Default value, should not be used + traffic_tcp, error, message, @@ -72,13 +74,14 @@ enum class type /** Optional detail type */ enum class detail { - all = 0, + _invalid = 0, // Default value, should not be used - // common + all, ok, + test, + total, loop, loop_cleanup, - total, process, processed, ignored, diff --git a/nano/node/transport/socket.cpp b/nano/node/transport/socket.cpp index ac9019818b..f703176058 100644 --- a/nano/node/transport/socket.cpp +++ b/nano/node/transport/socket.cpp @@ -121,7 +121,7 @@ void nano::transport::socket::async_read (std::shared_ptr> } else { - node_l->stats.add (nano::stat::type::traffic_tcp, nano::stat::dir::in, size_a); + node_l->stats.add (nano::stat::type::traffic_tcp, nano::stat::detail::all, nano::stat::dir::in, size_a); this_l->set_last_completion (); this_l->set_last_receive_time (); } @@ -214,7 +214,7 @@ void nano::transport::socket::write_queued_messages () } else { - node_l->stats.add (nano::stat::type::traffic_tcp, nano::stat::dir::out, size); + node_l->stats.add (nano::stat::type::traffic_tcp, nano::stat::detail::all, nano::stat::dir::out, size); this_l->set_last_completion (); } diff --git a/nano/rpc_test/rpc.cpp b/nano/rpc_test/rpc.cpp index d7d9be0da9..1039da4056 100644 --- a/nano/rpc_test/rpc.cpp +++ b/nano/rpc_test/rpc.cpp @@ -5350,14 +5350,14 @@ TEST (rpc, stats_clear) auto node = add_ipc_enabled_node (system); auto const rpc_ctx = add_rpc (system, node); nano::keypair key; - node->stats.inc (nano::stat::type::ledger, nano::stat::dir::in); - ASSERT_EQ (1, node->stats.count (nano::stat::type::ledger, nano::stat::dir::in)); + node->stats.inc (nano::stat::type::ledger, nano::stat::detail::test, nano::stat::dir::in); + ASSERT_EQ (1, node->stats.count (nano::stat::type::ledger, nano::stat::detail::test, nano::stat::dir::in)); boost::property_tree::ptree request; request.put ("action", "stats_clear"); auto response (wait_response (system, rpc_ctx, request)); std::string success (response.get ("success")); ASSERT_TRUE (success.empty ()); - ASSERT_EQ (0, node->stats.count (nano::stat::type::ledger, nano::stat::dir::in)); + ASSERT_EQ (0, node->stats.count (nano::stat::type::ledger, nano::stat::detail::test, nano::stat::dir::in)); ASSERT_LE (node->stats.last_reset ().count (), 5); } diff --git a/nano/slow_test/node.cpp b/nano/slow_test/node.cpp index fe22bb3868..59631f1c0d 100644 --- a/nano/slow_test/node.cpp +++ b/nano/slow_test/node.cpp @@ -713,7 +713,7 @@ TEST (confirmation_height, many_accounts_single_confirmation) ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed_bounded, nano::stat::dir::in), num_accounts * 2 - 2); ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed_unbounded, nano::stat::dir::in), 0); - ASSERT_TIMELY_EQ (40s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::all, nano::stat::dir::out)); + ASSERT_TIMELY_EQ (40s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::dir::out)); ASSERT_TIMELY_EQ (10s, node->active.election_winner_details_size (), 0); } @@ -776,7 +776,7 @@ TEST (confirmation_height, many_accounts_many_confirmations) ASSERT_GE (num_confirmed_bounded, nano::confirmation_height::unbounded_cutoff); ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed_unbounded, nano::stat::dir::in), num_blocks_to_confirm - num_confirmed_bounded); - ASSERT_TIMELY_EQ (60s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::all, nano::stat::dir::out)); + ASSERT_TIMELY_EQ (60s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::dir::out)); auto transaction = node->store.tx_begin_read (); size_t cemented_count = 0; @@ -788,7 +788,7 @@ TEST (confirmation_height, many_accounts_many_confirmations) ASSERT_EQ (num_blocks_to_confirm + 1, cemented_count); ASSERT_EQ (cemented_count, node->ledger.cemented_count ()); - ASSERT_TIMELY_EQ (20s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::all, nano::stat::dir::out)); + ASSERT_TIMELY_EQ (20s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::dir::out)); ASSERT_TIMELY_EQ (10s, node->active.election_winner_details_size (), 0); } @@ -935,7 +935,7 @@ TEST (confirmation_height, long_chains) ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed_bounded, nano::stat::dir::in), num_blocks * 2 + 2); ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed_unbounded, nano::stat::dir::in), 0); - ASSERT_TIMELY_EQ (40s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::all, nano::stat::dir::out)); + ASSERT_TIMELY_EQ (40s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::dir::out)); ASSERT_TIMELY_EQ (10s, node->active.election_winner_details_size (), 0); } @@ -1093,7 +1093,7 @@ TEST (confirmation_height, many_accounts_send_receive_self) } system.deadline_set (200s); - while ((node->ledger.cemented_count () - 1) != node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::all, nano::stat::dir::out)) + while ((node->ledger.cemented_count () - 1) != node->stats.count (nano::stat::type::confirmation_observer, nano::stat::dir::out)) { ASSERT_NO_ERROR (system.poll ()); } @@ -1109,7 +1109,7 @@ TEST (confirmation_height, many_accounts_send_receive_self) ASSERT_EQ (cemented_count, node->ledger.cemented_count ()); system.deadline_set (60s); - while ((node->ledger.cemented_count () - 1) != node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::all, nano::stat::dir::out)) + while ((node->ledger.cemented_count () - 1) != node->stats.count (nano::stat::type::confirmation_observer, nano::stat::dir::out)) { ASSERT_NO_ERROR (system.poll ()); }