Merge pull request #4334 from pwojcikdev/hinted-fixes

Fixes for hinted scheduler & vote cache

nano/core_test/toml.cpp

@@ -275,6 +275,7 @@ TEST (toml, daemon_config_deserialize_defaults)
 	ASSERT_EQ (conf.node.hinted_scheduler.hinting_threshold_percent, defaults.node.hinted_scheduler.hinting_threshold_percent);
 	ASSERT_EQ (conf.node.hinted_scheduler.check_interval.count (), defaults.node.hinted_scheduler.check_interval.count ());
 	ASSERT_EQ (conf.node.hinted_scheduler.block_cooldown.count (), defaults.node.hinted_scheduler.block_cooldown.count ());
+	ASSERT_EQ (conf.node.hinted_scheduler.vacancy_threshold_percent, defaults.node.hinted_scheduler.vacancy_threshold_percent);
 
 	ASSERT_EQ (conf.node.vote_cache.max_size, defaults.node.vote_cache.max_size);
 	ASSERT_EQ (conf.node.vote_cache.max_voters, defaults.node.vote_cache.max_voters);
@@ -543,6 +544,7 @@ TEST (toml, daemon_config_deserialize_no_defaults)
 	hinting_threshold = 99
 	check_interval = 999
 	block_cooldown = 999
+	vacancy_threshold = 99
 
 	[node.rocksdb]
 	enable = true
@@ -720,6 +722,7 @@ TEST (toml, daemon_config_deserialize_no_defaults)
 	ASSERT_NE (conf.node.hinted_scheduler.hinting_threshold_percent, defaults.node.hinted_scheduler.hinting_threshold_percent);
 	ASSERT_NE (conf.node.hinted_scheduler.check_interval.count (), defaults.node.hinted_scheduler.check_interval.count ());
 	ASSERT_NE (conf.node.hinted_scheduler.block_cooldown.count (), defaults.node.hinted_scheduler.block_cooldown.count ());
+	ASSERT_NE (conf.node.hinted_scheduler.vacancy_threshold_percent, defaults.node.hinted_scheduler.vacancy_threshold_percent);
 
 	ASSERT_NE (conf.node.vote_cache.max_size, defaults.node.vote_cache.max_size);
 	ASSERT_NE (conf.node.vote_cache.max_voters, defaults.node.vote_cache.max_voters);

nano/core_test/vote_cache.cpp

@@ -36,8 +36,9 @@ nano::keypair create_rep (nano::uint128_t weight)
 
 TEST (vote_cache, construction)
 {
+	nano::test::system system;
 	nano::vote_cache_config cfg;
-	nano::vote_cache vote_cache{ cfg };
+	nano::vote_cache vote_cache{ cfg, system.stats };
 	ASSERT_EQ (0, vote_cache.size ());
 	ASSERT_TRUE (vote_cache.empty ());
 	auto hash1 = nano::test::random_hash ();
@@ -49,8 +50,9 @@ TEST (vote_cache, construction)
  */
 TEST (vote_cache, insert_one_hash)
 {
+	nano::test::system system;
 	nano::vote_cache_config cfg;
-	nano::vote_cache vote_cache{ cfg };
+	nano::vote_cache vote_cache{ cfg, system.stats };
 	vote_cache.rep_weight_query = rep_weight_query ();
 	auto rep1 = create_rep (7);
 	auto hash1 = nano::test::random_hash ();
@@ -79,8 +81,9 @@ TEST (vote_cache, insert_one_hash)
  */
 TEST (vote_cache, insert_one_hash_many_votes)
 {
+	nano::test::system system;
 	nano::vote_cache_config cfg;
-	nano::vote_cache vote_cache{ cfg };
+	nano::vote_cache vote_cache{ cfg, system.stats };
 	vote_cache.rep_weight_query = rep_weight_query ();
 	auto hash1 = nano::test::random_hash ();
 	auto rep1 = create_rep (7);
@@ -114,8 +117,9 @@ TEST (vote_cache, insert_one_hash_many_votes)
  */
 TEST (vote_cache, insert_many_hashes_many_votes)
 {
+	nano::test::system system;
 	nano::vote_cache_config cfg;
-	nano::vote_cache vote_cache{ cfg };
+	nano::vote_cache vote_cache{ cfg, system.stats };
 	vote_cache.rep_weight_query = rep_weight_query ();
 	// There will be 3 random hashes to vote for
 	auto hash1 = nano::test::random_hash ();
@@ -194,8 +198,9 @@ TEST (vote_cache, insert_many_hashes_many_votes)
  */
 TEST (vote_cache, insert_duplicate)
 {
+	nano::test::system system;
 	nano::vote_cache_config cfg;
-	nano::vote_cache vote_cache{ cfg };
+	nano::vote_cache vote_cache{ cfg, system.stats };
 	vote_cache.rep_weight_query = rep_weight_query ();
 	auto hash1 = nano::test::random_hash ();
 	auto rep1 = create_rep (9);
@@ -211,8 +216,9 @@ TEST (vote_cache, insert_duplicate)
  */
 TEST (vote_cache, insert_newer)
 {
+	nano::test::system system;
 	nano::vote_cache_config cfg;
-	nano::vote_cache vote_cache{ cfg };
+	nano::vote_cache vote_cache{ cfg, system.stats };
 	vote_cache.rep_weight_query = rep_weight_query ();
 	auto hash1 = nano::test::random_hash ();
 	auto rep1 = create_rep (9);
@@ -236,8 +242,9 @@ TEST (vote_cache, insert_newer)
  */
 TEST (vote_cache, insert_older)
 {
+	nano::test::system system;
 	nano::vote_cache_config cfg;
-	nano::vote_cache vote_cache{ cfg };
+	nano::vote_cache vote_cache{ cfg, system.stats };
 	vote_cache.rep_weight_query = rep_weight_query ();
 	auto hash1 = nano::test::random_hash ();
 	auto rep1 = create_rep (9);
@@ -259,8 +266,9 @@ TEST (vote_cache, insert_older)
  */
 TEST (vote_cache, erase)
 {
+	nano::test::system system;
 	nano::vote_cache_config cfg;
-	nano::vote_cache vote_cache{ cfg };
+	nano::vote_cache vote_cache{ cfg, system.stats };
 	vote_cache.rep_weight_query = rep_weight_query ();
 	auto hash1 = nano::test::random_hash ();
 	auto hash2 = nano::test::random_hash ();
@@ -298,10 +306,11 @@ TEST (vote_cache, erase)
  */
 TEST (vote_cache, overfill)
 {
+	nano::test::system system;
 	// Create a vote cache with max size set to 1024
 	nano::vote_cache_config cfg;
 	cfg.max_size = 1024;
-	nano::vote_cache vote_cache{ cfg };
+	nano::vote_cache vote_cache{ cfg, system.stats };
 	vote_cache.rep_weight_query = rep_weight_query ();
 	const int count = 16 * 1024;
 	for (int n = 0; n < count; ++n)
@@ -324,8 +333,9 @@ TEST (vote_cache, overfill)
  */
 TEST (vote_cache, overfill_entry)
 {
+	nano::test::system system;
 	nano::vote_cache_config cfg;
-	nano::vote_cache vote_cache{ cfg };
+	nano::vote_cache vote_cache{ cfg, system.stats };
 	vote_cache.rep_weight_query = rep_weight_query ();
 	const int count = 1024;
 	auto hash1 = nano::test::random_hash ();
@@ -336,4 +346,40 @@ TEST (vote_cache, overfill_entry)
 		vote_cache.vote (vote1->hashes.front (), vote1);
 	}
 	ASSERT_EQ (1, vote_cache.size ());
+}
+
+TEST (vote_cache, age_cutoff)
+{
+	nano::test::system system;
+	nano::vote_cache_config cfg;
+	cfg.age_cutoff = std::chrono::seconds{ 3 };
+	nano::vote_cache vote_cache{ cfg, system.stats };
+	vote_cache.rep_weight_query = rep_weight_query ();
+
+	auto hash1 = nano::test::random_hash ();
+	auto rep1 = create_rep (9);
+	auto vote1 = nano::test::make_vote (rep1, { hash1 }, 3);
+	vote_cache.vote (vote1->hashes.front (), vote1);
+	ASSERT_EQ (1, vote_cache.size ());
+	ASSERT_TRUE (vote_cache.find (hash1));
+
+	auto tops1 = vote_cache.top (0);
+	ASSERT_EQ (tops1.size (), 1);
+	ASSERT_EQ (tops1[0].hash, hash1);
+	ASSERT_EQ (system.stats.count (nano::stat::type::vote_cache, nano::stat::detail::cleanup), 0);
+
+	// Wait for first cleanup
+	auto check = [&] () {
+		// Cleanup is performed periodically when calling `top ()`
+		vote_cache.top (0);
+		return system.stats.count (nano::stat::type::vote_cache, nano::stat::detail::cleanup);
+	};
+	ASSERT_TIMELY_EQ (5s, 1, check ());
+
+	// After first cleanup the entry should still be there
+	auto tops2 = vote_cache.top (0);
+	ASSERT_EQ (tops2.size (), 1);
+
+	// After 3 seconds the entry should be removed
+	ASSERT_TIMELY (5s, vote_cache.top (0).empty ());
 }

nano/lib/CMakeLists.txt

@@ -38,6 +38,7 @@ add_library(
   errors.hpp
   errors.cpp
   id_dispenser.hpp
+  interval.hpp
   ipc.hpp
   ipc.cpp
   ipc_client.hpp

nano/lib/interval.hpp

@@ -0,0 +1,30 @@
+#pragma once
+
+#include <chrono>
+
+namespace nano
+{
+class interval
+{
+public:
+	explicit interval (std::chrono::milliseconds target) :
+		target{ target }
+	{
+	}
+
+	bool elapsed ()
+	{
+		auto const now = std::chrono::steady_clock::now ();
+		if (now - last >= target)
+		{
+			last = now;
+			return true;
+		}
+		return false;
+	}
+
+private:
+	std::chrono::milliseconds const target;
+	std::chrono::steady_clock::time_point last{ std::chrono::steady_clock::now () };
+};
+}

nano/lib/stats_enums.hpp

@@ -63,6 +63,8 @@ enum class detail : uint8_t
 	update,
 	request,
 	broadcast,
+	cleanup,
+	top,
 
 	// processing queue
 	queue,

nano/node/node.cpp

@@ -182,7 +182,7 @@ nano::node::node (boost::asio::io_context & io_ctx_a, std::filesystem::path cons
 	history{ config.network_params.voting },
 	vote_uniquer (block_uniquer),
 	confirmation_height_processor (ledger, write_database_queue, config.conf_height_processor_batch_min_time, config.logging, logger, node_initialized_latch, flags.confirmation_height_processor_mode),
-	vote_cache{ config.vote_cache },
+	vote_cache{ config.vote_cache, stats },
 	generator{ config, ledger, wallets, vote_processor, history, network, stats, /* non-final */ false },
 	final_generator{ config, ledger, wallets, vote_processor, history, network, stats, /* final */ true },
 	active (*this, confirmation_height_processor),

nano/node/scheduler/hinted.cpp

@@ -47,7 +47,7 @@ void nano::scheduler::hinted::notify ()
 {
 	// Avoid notifying when there is very little space inside AEC
 	auto const limit = active.limit (nano::election_behavior::hinted);
-	if (active.vacancy (nano::election_behavior::hinted) >= (limit / 5))
+	if (active.vacancy (nano::election_behavior::hinted) >= (limit * config.vacancy_threshold_percent / 100))
 	{
 		condition.notify_all ();
 	}
@@ -107,7 +107,8 @@ void nano::scheduler::hinted::activate (const nano::store::read_transaction & tr
 		else
 		{
 			stats.inc (nano::stat::type::hinting, nano::stat::detail::missing_block);
-			node.bootstrap_block (current_hash);
+
+			// TODO: Block is missing, bootstrap it
 		}
 	}
 }
@@ -244,6 +245,7 @@ nano::error nano::scheduler::hinted_config::serialize (nano::tomlconfig & toml)
 	toml.put ("hinting_threshold", hinting_threshold_percent, "Percentage of online weight needed to start a hinted election. \ntype:uint32,[0,100]");
 	toml.put ("check_interval", check_interval.count (), "Interval between scans of the vote cache for possible hinted elections. \ntype:milliseconds");
 	toml.put ("block_cooldown", block_cooldown.count (), "Cooldown period for blocks that failed to start an election. \ntype:milliseconds");
+	toml.put ("vacancy_threshold", vacancy_threshold_percent, "Percentage of available space in the active elections container needed to trigger a scan for hinted elections (before the check interval elapses). \ntype:uint32,[0,100]");
 
 	return toml.get_error ();
 }
@@ -260,10 +262,16 @@ nano::error nano::scheduler::hinted_config::deserialize (nano::tomlconfig & toml
 	toml.get ("block_cooldown", block_cooldown_l);
 	block_cooldown = std::chrono::milliseconds{ block_cooldown_l };
 
+	toml.get ("vacancy_threshold", vacancy_threshold_percent);
+
 	if (hinting_threshold_percent > 100)
 	{
 		toml.get_error ().set ("hinting_threshold must be a number between 0 and 100");
 	}
+	if (vacancy_threshold_percent > 100)
+	{
+		toml.get_error ().set ("vacancy_threshold must be a number between 0 and 100");
+	}
 
 	return toml.get_error ();
 }

nano/node/scheduler/hinted.hpp

@@ -37,8 +37,9 @@ class hinted_config final
 
 public:
 	std::chrono::milliseconds check_interval{ 1000 };
-	std::chrono::milliseconds block_cooldown{ 5000 };
+	std::chrono::milliseconds block_cooldown{ 10000 };
 	unsigned hinting_threshold_percent{ 10 };
+	unsigned vacancy_threshold_percent{ 20 };
 };
 
 /*