Cleaning up stat counters for active_transactions. (#4149)

The stat type active tracks:
  - active_started for each behavior
  - active_confirmed/active_timeout/active_dropped for each behavior.

nano/core_test/active_transactions.cpp

@@ -646,7 +646,7 @@ TEST (active_transactions, dropped_cleanup)
 	ASSERT_FALSE (node.network.publish_filter.apply (block_bytes.data (), block_bytes.size ()));
 
 	// An election was recently dropped
-	ASSERT_EQ (1, node.stats.count (nano::stat::type::election, nano::stat::detail::election_drop_all));
+	ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::normal));
 
 	// Block cleared from active
 	ASSERT_FALSE (node.active.active (nano::dev::genesis->hash ()));
@@ -664,7 +664,7 @@ TEST (active_transactions, dropped_cleanup)
 	ASSERT_TRUE (node.network.publish_filter.apply (block_bytes.data (), block_bytes.size ()));
 
 	// Not dropped
-	ASSERT_EQ (1, node.stats.count (nano::stat::type::election, nano::stat::detail::election_drop_all));
+	ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::normal));
 
 	// Block cleared from active
 	ASSERT_FALSE (node.active.active (nano::dev::genesis->hash ()));
@@ -1403,7 +1403,7 @@ TEST (active_transactions, fifo)
 	ASSERT_TIMELY (5s, node.active.size () == 1);
 
 	// Ensure overflow stats have been incremented
-	ASSERT_EQ (1, node.stats.count (nano::stat::type::election, nano::stat::detail::election_drop_overflow));
+	ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::normal));
 
 	// Ensure the surviving transaction is the least recently inserted
 	ASSERT_TIMELY (1s, node.active.election (receive2->qualified_root ()) != nullptr);
@@ -1471,7 +1471,7 @@ TEST (active_transactions, limit_vote_hinted_elections)
 	ASSERT_TIMELY (5s, nano::test::active (node, { open1 }));
 
 	// Ensure there was no overflow of elections
-	ASSERT_EQ (0, node.stats.count (nano::stat::type::election, nano::stat::detail::election_drop_overflow));
+	ASSERT_EQ (0, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::normal));
 }
 
 /*

nano/lib/stats_enums.hpp

@@ -35,6 +35,10 @@ enum class type : uint8_t
 	blockprocessor,
 	bootstrap_server,
 	active,
+	active_started,
+	active_confirmed,
+	active_dropped,
+	active_timeout,
 	backlog,
 	unchecked,
 
@@ -142,24 +146,20 @@ enum class detail : uint8_t
 	vote_cached,
 	late_block,
 	late_block_seconds,
-	election_start,
-	election_confirmed_all,
 	election_block_conflict,
-	election_difficulty_update,
-	election_drop_expired,
-	election_drop_overflow,
-	election_drop_all,
 	election_restart,
-	election_confirmed,
 	election_not_confirmed,
 	election_hinted_overflow,
-	election_hinted_started,
 	election_hinted_confirmed,
 	election_hinted_drop,
 	generate_vote,
 	generate_vote_normal,
 	generate_vote_final,
 
+	// election types
+	normal,
+	hinted,
+
 	// received messages
 	invalid_header,
 	invalid_message_type,
@@ -240,7 +240,6 @@ enum class detail : uint8_t
 	generator_spacing,
 
 	// hinting
-	hinted,
 	insert_failed,
 	missing_block,
 

nano/node/active_transactions.cpp

@@ -235,11 +235,6 @@ void nano::active_transactions::request_confirm (nano::unique_lock<nano::mutex>
 
 		if (election_l->transition_time (solicitor))
 		{
-			// Locks active mutex, cleans up the election and erases it from the main container
-			if (!confirmed_l)
-			{
-				node.stats.inc (nano::stat::type::election, nano::stat::detail::election_drop_expired);
-			}
 			erase (election_l->qualified_root);
 		}
 	}
@@ -257,23 +252,7 @@ void nano::active_transactions::cleanup_election (nano::unique_lock<nano::mutex>
 {
 	debug_assert (lock_a.owns_lock ());
 
-	if (!election->confirmed ())
+	node.stats.inc (completion_type (*election), nano::to_stat_detail (election->behavior ()));
-	{
-		node.stats.inc (nano::stat::type::election, nano::stat::detail::election_drop_all);
-		if (election->behavior () == election_behavior::hinted)
-		{
-			node.stats.inc (nano::stat::type::election, nano::stat::detail::election_hinted_drop);
-		}
-	}
-	else
-	{
-		node.stats.inc (nano::stat::type::election, nano::stat::detail::election_confirmed_all);
-		if (election->behavior () == election_behavior::hinted)
-		{
-			node.stats.inc (nano::stat::type::election, nano::stat::detail::election_hinted_confirmed);
-		}
-	}
-
 	if (election->behavior () == election_behavior::hinted)
 	{
 		--active_hinted_elections_count;
@@ -306,13 +285,25 @@ void nano::active_transactions::cleanup_election (nano::unique_lock<nano::mutex>
 		}
 	}
 
-	node.stats.inc (nano::stat::type::election, election->confirmed () ? nano::stat::detail::election_confirmed : nano::stat::detail::election_not_confirmed);
 	if (node.config.logging.election_result_logging ())
 	{
 		node.logger.try_log (boost::str (boost::format ("Election erased for root %1%, confirmed: %2$b") % election->qualified_root.to_string () % election->confirmed ()));
 	}
 }
 
+nano::stat::type nano::active_transactions::completion_type (nano::election const & election) const
+{
+	if (election.confirmed ())
+	{
+		return nano::stat::type::active_confirmed;
+	}
+	if (election.failed ())
+	{
+		return nano::stat::type::active_timeout;
+	}
+	return nano::stat::type::active_dropped;
+}
+
 std::vector<std::shared_ptr<nano::election>> nano::active_transactions::list_active (std::size_t max_a)
 {
 	nano::lock_guard<nano::mutex> guard{ mutex };
@@ -388,8 +379,8 @@ nano::election_insertion_result nano::active_transactions::insert_impl (nano::un
 				{
 					cache->fill (result.election);
 				}
+				node.stats.inc (nano::stat::type::active_started, nano::to_stat_detail (election_behavior_a));
 				node.observers.active_started.notify (hash);
-				node.stats.inc (nano::stat::type::election, nano::stat::detail::election_start);
 				vacancy_update ();
 			}
 		}
@@ -420,10 +411,6 @@ nano::election_insertion_result nano::active_transactions::insert_hinted (std::s
 	nano::unique_lock<nano::mutex> lock{ mutex };
 
 	auto result = insert_impl (lock, block_a, nano::election_behavior::hinted);
-	if (result.inserted)
-	{
-		node.stats.inc (nano::stat::type::election, nano::stat::detail::election_hinted_started);
-	}
 	return result;
 }
 
@@ -563,7 +550,6 @@ void nano::active_transactions::erase_oldest ()
 	nano::unique_lock<nano::mutex> lock{ mutex };
 	if (!roots.empty ())
 	{
-		node.stats.inc (nano::stat::type::election, nano::stat::detail::election_drop_overflow);
 		auto item = roots.get<tag_sequenced> ().front ();
 		cleanup_election (lock, item.election);
 	}

nano/node/active_transactions.hpp

@@ -202,6 +202,7 @@ private:
 	void erase (nano::qualified_root const &);
 	// Erase all blocks from active and, if not confirmed, clear digests from network filters
 	void cleanup_election (nano::unique_lock<nano::mutex> & lock_a, std::shared_ptr<nano::election>);
+	nano::stat::type completion_type (nano::election const & election) const;
 	// Returns a list of elections sorted by difficulty, mutex must be locked
 	std::vector<std::shared_ptr<nano::election>> list_active_impl (std::size_t) const;
 	/**

nano/node/election.cpp

@@ -634,6 +634,24 @@ std::vector<nano::vote_with_weight_info> nano::election::votes_with_weight () co
 	return result;
 }
 
+nano::stat::detail nano::to_stat_detail (nano::election_behavior behavior)
+{
+	switch (behavior)
+	{
+		case nano::election_behavior::normal:
+		{
+			return nano::stat::detail::normal;
+		}
+		case nano::election_behavior::hinted:
+		{
+			return nano::stat::detail::hinted;
+		}
+	}
+
+	debug_assert (false, "unknown election behavior");
+	return {};
+}
+
 nano::election_behavior nano::election::behavior () const
 {
 	return behavior_m;

nano/node/election.hpp

@@ -49,6 +49,8 @@ enum class election_behavior
 	hinted
 };
 
+nano::stat::detail to_stat_detail (nano::election_behavior);
+
 struct election_extended_status final
 {
 	nano::election_status status;