Remove active_elections::trim

Each scheduler checks its own limits with calls to active_elections::vacancy.
Trim is problematic as it indiscriminately cancels elections without consideration to why it was scheduled or its priority.

nano/core_test/active_elections.cpp

@@ -1341,89 +1341,6 @@ TEST (active_elections, vacancy)
 	}
 }
 
-// Ensure transactions in excess of capacity are removed in fifo order
-TEST (active_elections, fifo)
-{
-	nano::test::system system{};
-
-	nano::node_config config = system.default_config ();
-	config.active_elections.size = 1;
-	config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
-
-	auto & node = *system.add_node (config);
-	auto latest_hash = nano::dev::genesis->hash ();
-	nano::keypair key0{};
-	nano::state_block_builder builder{};
-
-	// Construct two pending entries that can be received simultaneously
-	auto send1 = builder.make_block ()
-				 .previous (latest_hash)
-				 .account (nano::dev::genesis_key.pub)
-				 .representative (nano::dev::genesis_key.pub)
-				 .link (key0.pub)
-				 .balance (nano::dev::constants.genesis_amount - 1)
-				 .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
-				 .work (*system.work.generate (latest_hash))
-				 .build ();
-	ASSERT_EQ (nano::block_status::progress, node.process (send1));
-	node.process_confirmed (nano::election_status{ send1 });
-	ASSERT_TIMELY (5s, node.block_confirmed (send1->hash ()));
-
-	nano::keypair key1{};
-	latest_hash = send1->hash ();
-	auto send2 = builder.make_block ()
-				 .previous (latest_hash)
-				 .account (nano::dev::genesis_key.pub)
-				 .representative (nano::dev::genesis_key.pub)
-				 .link (key1.pub)
-				 .balance (nano::dev::constants.genesis_amount - 2)
-				 .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
-				 .work (*system.work.generate (latest_hash))
-				 .build ();
-	ASSERT_EQ (nano::block_status::progress, node.process (send2));
-	node.process_confirmed (nano::election_status{ send2 });
-	ASSERT_TIMELY (5s, node.block_confirmed (send2->hash ()));
-
-	auto receive1 = builder.make_block ()
-					.previous (0)
-					.account (key0.pub)
-					.representative (nano::dev::genesis_key.pub)
-					.link (send1->hash ())
-					.balance (1)
-					.sign (key0.prv, key0.pub)
-					.work (*system.work.generate (key0.pub))
-					.build ();
-	ASSERT_EQ (nano::block_status::progress, node.process (receive1));
-
-	auto receive2 = builder.make_block ()
-					.previous (0)
-					.account (key1.pub)
-					.representative (nano::dev::genesis_key.pub)
-					.link (send2->hash ())
-					.balance (1)
-					.sign (key1.prv, key1.pub)
-					.work (*system.work.generate (key1.pub))
-					.build ();
-	ASSERT_EQ (nano::block_status::progress, node.process (receive2));
-
-	// Ensure first transaction becomes active
-	node.scheduler.manual.push (receive1);
-	ASSERT_TIMELY (5s, node.active.election (receive1->qualified_root ()) != nullptr);
-
-	// Ensure second transaction becomes active
-	node.scheduler.manual.push (receive2);
-	ASSERT_TIMELY (5s, node.active.election (receive2->qualified_root ()) != nullptr);
-
-	// Ensure excess transactions get trimmed
-	ASSERT_TIMELY_EQ (5s, node.active.size (), 1);
-
-	// Ensure overflow stats have been incremented
-	ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::manual));
-
-	// Ensure the surviving transaction is the least recently inserted
-	ASSERT_TIMELY (1s, node.active.election (receive2->qualified_root ()) != nullptr);
-}
-
 /*
  * Ensures we limit the number of vote hinted elections in AEC
  */

nano/node/active_elections.cpp

@@ -365,20 +365,6 @@ void nano::active_elections::request_loop ()
 	}
 }
 
-void nano::active_elections::trim ()
-{
-	/*
-	 * Both normal and hinted election schedulers are well-behaved, meaning they first check for AEC vacancy before inserting new elections.
-	 * However, it is possible that AEC will be temporarily overfilled in case it's running at full capacity and election hinting or manual queue kicks in.
-	 * That case will lead to unwanted churning of elections, so this allows for AEC to be overfilled to 125% until erasing of elections happens.
-	 */
-	while (vacancy (nano::election_behavior::priority) < -(limit (nano::election_behavior::priority) / 4))
-	{
-		node.stats.inc (nano::stat::type::active, nano::stat::detail::erase_oldest);
-		erase_oldest ();
-	}
-}
-
 nano::election_insertion_result nano::active_elections::insert (std::shared_ptr<nano::block> const & block_a, nano::election_behavior election_behavior_a)
 {
 	debug_assert (block_a);
@@ -449,8 +435,6 @@ nano::election_insertion_result nano::active_elections::insert (std::shared_ptr<
 		result.election->broadcast_vote ();
 	}
 
-	trim ();
-
 	return result;
 }
 
@@ -495,16 +479,6 @@ bool nano::active_elections::erase (nano::qualified_root const & root_a)
 	return false;
 }
 
-void nano::active_elections::erase_oldest ()
-{
-	nano::unique_lock<nano::mutex> lock{ mutex };
-	if (!roots.empty ())
-	{
-		auto item = roots.get<tag_sequenced> ().front ();
-		cleanup_election (lock, item.election);
-	}
-}
-
 bool nano::active_elections::empty () const
 {
 	nano::lock_guard<nano::mutex> lock{ mutex };

nano/node/active_elections.hpp

@@ -117,7 +117,6 @@ class active_elections final
 	std::vector<std::shared_ptr<nano::election>> list_active (std::size_t = std::numeric_limits<std::size_t>::max ());
 	bool erase (nano::block const &);
 	bool erase (nano::qualified_root const &);
-	void erase_oldest ();
 	bool empty () const;
 	std::size_t size () const;
 	bool publish (std::shared_ptr<nano::block> const &);
@@ -140,8 +139,6 @@ class active_elections final
 	std::shared_ptr<nano::election> remove_election_winner_details (nano::block_hash const &);
 
 private:
-	// Erase elections if we're over capacity
-	void trim ();
 	void request_loop ();
 	void request_confirm (nano::unique_lock<nano::mutex> &);
 	// Erase all blocks from active and, if not confirmed, clear digests from network filters