Skip to content

Commit

Permalink
Remove special semantics of stat::detail::all
Browse files Browse the repository at this point in the history
  • Loading branch information
pwojcikdev committed Apr 24, 2024
1 parent c4b97a4 commit c6f88b7
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 53 deletions.
2 changes: 1 addition & 1 deletion nano/core_test/confirming_set.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 ());
Expand Down
2 changes: 1 addition & 1 deletion nano/core_test/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()));
Expand Down
12 changes: 8 additions & 4 deletions nano/core_test/stats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
33 changes: 18 additions & 15 deletions nano/lib/stats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand All @@ -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<counter_entry> ());
auto [it_all, inserted_all] = counters.emplace (all_key, std::make_unique<counter_entry> ());

updater (*it->second);

if (key != all_key)
{
updater (*it_all->second); // Also update the `all` counter
}
}
};

Expand All @@ -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 ())
Expand All @@ -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<sampler_value_t, sampler_value_t> expected_min_max, nano::stats::sampler_value_t value)
{
// Updates need to happen while holding the mutex
Expand All @@ -137,6 +139,7 @@ void nano::stats::sample (stat::sample sample, std::pair<sampler_value_t, sample
if (auto it = samplers.find (key); it != samplers.end ())
{
updater (*it->second);

return;
}
}
Expand Down
21 changes: 3 additions & 18 deletions nano/lib/stats.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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<sampler_value_t, sampler_value_t> expected_min_max, sampler_value_t value);

Expand Down
9 changes: 6 additions & 3 deletions nano/lib/stats_enums.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions nano/node/transport/socket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ void nano::transport::socket::async_read (std::shared_ptr<std::vector<uint8_t>>
}
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 ();
}
Expand Down Expand Up @@ -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 ();
}

Expand Down
6 changes: 3 additions & 3 deletions nano/rpc_test/rpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> ("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);
}

Expand Down
12 changes: 6 additions & 6 deletions nano/slow_test/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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 ());
}
Expand All @@ -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 ());
}
Expand Down

0 comments on commit c6f88b7

Please sign in to comment.