Squash merge PR nanocurrency#4626: Bounded election buckets with dyna…

…mic reprioritization

nano/core_test/active_elections.cpp

@@ -157,7 +157,8 @@ TEST (active_elections, confirm_frontier)
 	ASSERT_GT (election2->confirmation_request_count, 0u);
 }
 
-TEST (active_elections, keep_local)
+// TODO: Adjust for new behaviour of bounded buckets
+TEST (active_elections, DISABLED_keep_local)
 {
 	nano::test::system system{};
 
@@ -1390,101 +1391,3 @@ TEST (active_elections, limit_vote_hinted_elections)
 	// Ensure there was no overflow of elections
 	ASSERT_EQ (0, node.stats.count (nano::stat::type::active_elections_dropped, nano::stat::detail::priority));
 }
-
-/*
- * Tests that when AEC is running at capacity from normal elections, it is still possible to schedule a limited number of hinted elections
- */
-TEST (active_elections, allow_limited_overflow)
-{
-	nano::test::system system;
-	nano::node_config config = system.default_config ();
-	const int aec_limit = 20;
-	config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
-	config.active_elections.size = aec_limit;
-	config.active_elections.hinted_limit_percentage = 20; // Should give us a limit of 4 hinted elections
-	auto & node = *system.add_node (config);
-
-	auto blocks = nano::test::setup_independent_blocks (system, node, aec_limit * 4);
-
-	// Split blocks in two halves
-	std::vector<std::shared_ptr<nano::block>> blocks1 (blocks.begin (), blocks.begin () + blocks.size () / 2);
-	std::vector<std::shared_ptr<nano::block>> blocks2 (blocks.begin () + blocks.size () / 2, blocks.end ());
-
-	// Even though automatic frontier confirmation is disabled, AEC is doing funny stuff and inserting elections, clear that
-	WAIT (1s);
-	node.active.clear ();
-	ASSERT_TRUE (node.active.empty ());
-
-	// Insert the first part of the blocks into normal election scheduler
-	for (auto const & block : blocks1)
-	{
-		node.scheduler.priority.activate (node.ledger.tx_begin_read (), block->account ());
-	}
-
-	// Ensure number of active elections reaches AEC limit and there is no overfill
-	ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit (nano::election_behavior::priority));
-	// And it stays that way without increasing
-	ASSERT_ALWAYS (1s, node.active.size () == node.active.limit (nano::election_behavior::priority));
-
-	// Insert votes for the second part of the blocks, so that those are scheduled as hinted elections
-	for (auto const & block : blocks2)
-	{
-		// Non-final vote, so it stays in the AEC without getting confirmed
-		auto vote = nano::test::make_vote (nano::dev::genesis_key, { block });
-		node.vote_cache.insert (vote);
-	}
-
-	// Ensure active elections overfill AEC only up to normal + hinted limit
-	ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit (nano::election_behavior::priority) + node.active.limit (nano::election_behavior::hinted));
-	// And it stays that way without increasing
-	ASSERT_ALWAYS (1s, node.active.size () == node.active.limit (nano::election_behavior::priority) + node.active.limit (nano::election_behavior::hinted));
-}
-
-/*
- * Tests that when hinted elections are present in the AEC, normal scheduler adapts not to exceed the limit of all elections
- */
-TEST (active_elections, allow_limited_overflow_adapt)
-{
-	nano::test::system system;
-	nano::node_config config = system.default_config ();
-	const int aec_limit = 20;
-	config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
-	config.active_elections.size = aec_limit;
-	config.active_elections.hinted_limit_percentage = 20; // Should give us a limit of 4 hinted elections
-	auto & node = *system.add_node (config);
-
-	auto blocks = nano::test::setup_independent_blocks (system, node, aec_limit * 4);
-
-	// Split blocks in two halves
-	std::vector<std::shared_ptr<nano::block>> blocks1 (blocks.begin (), blocks.begin () + blocks.size () / 2);
-	std::vector<std::shared_ptr<nano::block>> blocks2 (blocks.begin () + blocks.size () / 2, blocks.end ());
-
-	// Even though automatic frontier confirmation is disabled, AEC is doing funny stuff and inserting elections, clear that
-	WAIT (1s);
-	node.active.clear ();
-	ASSERT_TRUE (node.active.empty ());
-
-	// Insert votes for the second part of the blocks, so that those are scheduled as hinted elections
-	for (auto const & block : blocks2)
-	{
-		// Non-final vote, so it stays in the AEC without getting confirmed
-		auto vote = nano::test::make_vote (nano::dev::genesis_key, { block });
-		node.vote_cache.insert (vote);
-	}
-
-	// Ensure hinted election amount is bounded by hinted limit
-	ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit (nano::election_behavior::hinted));
-	// And it stays that way without increasing
-	ASSERT_ALWAYS (1s, node.active.size () == node.active.limit (nano::election_behavior::hinted));
-
-	// Insert the first part of the blocks into normal election scheduler
-	for (auto const & block : blocks1)
-	{
-		node.scheduler.priority.activate (node.ledger.tx_begin_read (), block->account ());
-	}
-
-	// Ensure number of active elections reaches AEC limit and there is no overfill
-	ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit (nano::election_behavior::priority));
-	// And it stays that way without increasing
-	ASSERT_ALWAYS (1s, node.active.size () == node.active.limit (nano::election_behavior::priority));
-}

nano/core_test/scheduler_buckets.cpp

@@ -1,5 +1,4 @@
 #include <nano/lib/blocks.hpp>
-#include <nano/node/scheduler/buckets.hpp>
 #include <nano/secure/common.hpp>
 
 #include <gtest/gtest.h>
@@ -107,6 +106,7 @@ std::shared_ptr<nano::state_block> & block3 ()
 	return result;
 }
 
+/*
 TEST (buckets, construction)
 {
 	nano::scheduler::buckets buckets;
@@ -240,3 +240,4 @@ TEST (buckets, trim_even)
 	buckets.pop ();
 	ASSERT_EQ (block1 (), buckets.top ());
 }
+*/

nano/lib/stats_enums.hpp

@@ -27,6 +27,7 @@ enum class type
 	vote_processor_tier,
 	vote_processor_overfill,
 	election,
+	election_cleanup,
 	election_vote,
 	http_callback,
 	ipc,
@@ -68,10 +69,12 @@ enum class type
 	active_elections_confirmed,
 	active_elections_dropped,
 	active_elections_timeout,
+	active_elections_cancelled,
 	active_elections_cemented,
 	backlog,
 	unchecked,
 	election_scheduler,
+	election_bucket,
 	optimistic_scheduler,
 	handshake,
 	rep_crawler,
@@ -388,6 +391,7 @@ enum class detail
 	// active
 	insert,
 	insert_failed,
+	election_cleanup,
 
 	// active_elections
 	started,
@@ -470,6 +474,7 @@ enum class detail
 	active,
 	expired_confirmed,
 	expired_unconfirmed,
+	cancelled,
 
 	// election_status_type
 	ongoing,
@@ -486,6 +491,10 @@ enum class detail
 	rep_update,
 	update_online,
 
+	// election bucket
+	activate_success,
+	cancel_lowest,
+
 	_last // Must be the last enum
 };
 

nano/node/CMakeLists.txt

@@ -141,8 +141,6 @@ add_library(
   request_aggregator.cpp
   scheduler/bucket.cpp
   scheduler/bucket.hpp
-  scheduler/buckets.cpp
-  scheduler/buckets.hpp
   scheduler/component.hpp
   scheduler/component.cpp
   scheduler/hinted.hpp

nano/node/active_elections.cpp

@@ -309,7 +309,11 @@ void nano::active_elections::cleanup_election (nano::unique_lock<nano::mutex> &
 	auto blocks_l = election->blocks ();
 	node.vote_router.disconnect (*election);
 
-	roots.get<tag_root> ().erase (roots.get<tag_root> ().find (election->qualified_root));
+	// Erase root info
+	auto it = roots.get<tag_root> ().find (election->qualified_root);
+	release_assert (it != roots.get<tag_root> ().end ());
+	entry entry = *it;
+	roots.get<tag_root> ().erase (it);
 
 	node.stats.inc (nano::stat::type::active_elections, nano::stat::detail::stopped);
 	node.stats.inc (nano::stat::type::active_elections, election->confirmed () ? nano::stat::detail::confirmed : nano::stat::detail::unconfirmed);
@@ -328,6 +332,11 @@ void nano::active_elections::cleanup_election (nano::unique_lock<nano::mutex> &
 	// Track election duration
 	node.stats.sample (nano::stat::sample::active_election_duration, election->duration ().count (), { 0, 1000 * 60 * 10 /* 0-10 minutes range */ });
 
+	// Notify observers without holding the lock
+	if (entry.erased_callback)
+	{
+		entry.erased_callback (election);
+	}
 	vacancy_update ();
 
 	for (auto const & [hash, block] : blocks_l)
@@ -388,7 +397,7 @@ void nano::active_elections::request_loop ()
 	}
 }
 
-nano::election_insertion_result nano::active_elections::insert (std::shared_ptr<nano::block> const & block_a, nano::election_behavior election_behavior_a)
+nano::election_insertion_result nano::active_elections::insert (std::shared_ptr<nano::block> const & block_a, nano::election_behavior election_behavior_a, erased_callback_t erased_callback_a)
 {
 	debug_assert (block_a);
 	debug_assert (block_a->has_sideband ());
@@ -416,7 +425,7 @@ nano::election_insertion_result nano::active_elections::insert (std::shared_ptr<
 				node.online_reps.observe (rep_a);
 			};
 			result.election = nano::make_shared<nano::election> (node, block_a, nullptr, observe_rep_cb, election_behavior_a);
-			roots.get<tag_root> ().emplace (nano::active_elections::conflict_info{ root, result.election });
+			roots.get<tag_root> ().emplace (entry{ root, result.election, std::move (erased_callback_a) });
 			node.vote_router.connect (hash, result.election);
 
 			// Keep track of election count by election type
@@ -550,10 +559,12 @@ std::size_t nano::active_elections::election_winner_details_size ()
 
 void nano::active_elections::clear ()
 {
+	// TODO: Call erased_callback for each election
 	{
 		nano::lock_guard<nano::mutex> guard{ mutex };
 		roots.clear ();
 	}
+
 	vacancy_update ();
 }
 
@@ -623,16 +634,14 @@ nano::stat::type nano::to_stat_type (nano::election_state state)
 		case election_state::expired_unconfirmed:
 			return nano::stat::type::active_elections_timeout;
 			break;
+		case election_state::cancelled:
+			return nano::stat::type::active_elections_cancelled;
+			break;
 	}
 	debug_assert (false);
 	return {};
 }
 
-nano::stat::detail nano::to_stat_detail (nano::election_state state)
-{
-	return nano::enum_util::cast<nano::stat::detail> (state);
-}
-
 nano::stat::detail nano::to_stat_detail (nano::election_status_type type)
 {
 	return nano::enum_util::cast<nano::stat::detail> (type);

nano/node/active_elections.hpp

@@ -72,12 +72,16 @@ class active_elections_config final
  */
 class active_elections final
 {
+public:
+	using erased_callback_t = std::function<void (std::shared_ptr<nano::election>)>;
+
 private: // Elections
-	class conflict_info final
+	class entry final
 	{
 	public:
 		nano::qualified_root root;
 		std::shared_ptr<nano::election> election;
+		erased_callback_t erased_callback;
 	};
 
 	friend class nano::election;
@@ -90,11 +94,11 @@ class active_elections final
 	class tag_arrival {};
 	class tag_hash {};
 
-	using ordered_roots = boost::multi_index_container<conflict_info,
+	using ordered_roots = boost::multi_index_container<entry,
 	mi::indexed_by<
 		mi::sequenced<mi::tag<tag_sequenced>>,
 		mi::hashed_unique<mi::tag<tag_root>,
-			mi::member<conflict_info, nano::qualified_root, &conflict_info::root>>
+			mi::member<entry, nano::qualified_root, &entry::root>>
 	>>;
 	// clang-format on
 	ordered_roots roots;
@@ -109,7 +113,7 @@ class active_elections final
 	/**
 	 * Starts new election with a specified behavior type
 	 */
-	nano::election_insertion_result insert (std::shared_ptr<nano::block> const &, nano::election_behavior = nano::election_behavior::priority);
+	nano::election_insertion_result insert (std::shared_ptr<nano::block> const &, nano::election_behavior = nano::election_behavior::priority, erased_callback_t = nullptr);
 	// Is the root of this block in the roots container
 	bool active (nano::block const &) const;
 	bool active (nano::qualified_root const &) const;

nano/node/election.cpp

@@ -81,7 +81,6 @@ void nano::election::confirm_once (nano::unique_lock<nano::mutex> & lock_a)
 
 bool nano::election::valid_change (nano::election_state expected_a, nano::election_state desired_a) const
 {
-	bool result = false;
 	switch (expected_a)
 	{
 		case nano::election_state::passive:
@@ -90,8 +89,8 @@ bool nano::election::valid_change (nano::election_state expected_a, nano::electi
 				case nano::election_state::active:
 				case nano::election_state::confirmed:
 				case nano::election_state::expired_unconfirmed:
-					result = true;
-					break;
+				case nano::election_state::cancelled:
+					return true; // Valid
 				default:
 					break;
 			}
@@ -101,8 +100,8 @@ bool nano::election::valid_change (nano::election_state expected_a, nano::electi
 			{
 				case nano::election_state::confirmed:
 				case nano::election_state::expired_unconfirmed:
-					result = true;
-					break;
+				case nano::election_state::cancelled:
+					return true; // Valid
 				default:
 					break;
 			}
@@ -111,17 +110,18 @@ bool nano::election::valid_change (nano::election_state expected_a, nano::electi
 			switch (desired_a)
 			{
 				case nano::election_state::expired_confirmed:
-					result = true;
-					break;
+					return true; // Valid
 				default:
 					break;
 			}
 			break;
 		case nano::election_state::expired_unconfirmed:
 		case nano::election_state::expired_confirmed:
+		case nano::election_state::cancelled:
+			// No transitions are valid from these states
 			break;
 	}
-	return result;
+	return false;
 }
 
 bool nano::election::state_change (nano::election_state expected_a, nano::election_state desired_a)
@@ -168,10 +168,16 @@ void nano::election::send_confirm_req (nano::confirmation_solicitor & solicitor_
 
 void nano::election::transition_active ()
 {
-	nano::unique_lock<nano::mutex> lock{ mutex };
+	nano::lock_guard<nano::mutex> guard{ mutex };
 	state_change (nano::election_state::passive, nano::election_state::active);
 }
 
+void nano::election::cancel ()
+{
+	nano::lock_guard<nano::mutex> guard{ mutex };
+	state_change (state_m, nano::election_state::cancelled);
+}
+
 bool nano::election::confirmed_locked () const
 {
 	debug_assert (!mutex.try_lock ());
@@ -273,6 +279,8 @@ bool nano::election::transition_time (nano::confirmation_solicitor & solicitor_a
 		case nano::election_state::expired_confirmed:
 			debug_assert (false);
 			break;
+		case nano::election_state::cancelled:
+			return true; // Clean up cancelled elections immediately
 	}
 
 	if (!confirmed_locked () && time_to_live () < std::chrono::steady_clock::now () - election_start)
@@ -822,3 +830,8 @@ std::string_view nano::to_string (nano::election_state state)
 {
 	return nano::enum_util::name (state);
 }
+
+nano::stat::detail nano::to_stat_detail (nano::election_state state)
+{
+	return nano::enum_util::cast<nano::stat::detail> (state);
+}