Merge pull request #4748 from RickiNano/Vote-wait

Simplify vote generator

nano/core_test/toml.cpp

@@ -191,7 +191,6 @@ TEST (toml, daemon_config_deserialize_defaults)
 	ASSERT_EQ (conf.node.unchecked_cutoff_time, defaults.node.unchecked_cutoff_time);
 	ASSERT_EQ (conf.node.use_memory_pools, defaults.node.use_memory_pools);
 	ASSERT_EQ (conf.node.vote_generator_delay, defaults.node.vote_generator_delay);
-	ASSERT_EQ (conf.node.vote_generator_threshold, defaults.node.vote_generator_threshold);
 	ASSERT_EQ (conf.node.vote_minimum, defaults.node.vote_minimum);
 	ASSERT_EQ (conf.node.work_peers, defaults.node.work_peers);
 	ASSERT_EQ (conf.node.work_threads, defaults.node.work_threads);
@@ -456,7 +455,6 @@ TEST (toml, daemon_config_deserialize_no_defaults)
 	unchecked_cutoff_time = 999
 	use_memory_pools = false
 	vote_generator_delay = 999
-	vote_generator_threshold = 9
 	vote_minimum = "999"
 	work_peers = ["dev.org:999"]
 	work_threads = 999
@@ -700,7 +698,6 @@ TEST (toml, daemon_config_deserialize_no_defaults)
 	ASSERT_NE (conf.node.unchecked_cutoff_time, defaults.node.unchecked_cutoff_time);
 	ASSERT_NE (conf.node.use_memory_pools, defaults.node.use_memory_pools);
 	ASSERT_NE (conf.node.vote_generator_delay, defaults.node.vote_generator_delay);
-	ASSERT_NE (conf.node.vote_generator_threshold, defaults.node.vote_generator_threshold);
 	ASSERT_NE (conf.node.vote_minimum, defaults.node.vote_minimum);
 	ASSERT_NE (conf.node.work_peers, defaults.node.work_peers);
 	ASSERT_NE (conf.node.work_threads, defaults.node.work_threads);

nano/node/nodeconfig.cpp

@@ -120,7 +120,6 @@ nano::error nano::node_config::serialize_toml (nano::tomlconfig & toml) const
 	toml.put ("allow_local_peers", allow_local_peers, "Enable or disable local host peering.\ntype:bool");
 	toml.put ("vote_minimum", vote_minimum.to_string_dec (), "Local representatives do not vote if the delegated weight is under this threshold. Saves on system resources.\ntype:string,amount,raw");
 	toml.put ("vote_generator_delay", vote_generator_delay.count (), "Delay before votes are sent to allow for efficient bundling of hashes in votes.\ntype:milliseconds");
-	toml.put ("vote_generator_threshold", vote_generator_threshold, "Number of bundled hashes required for an additional generator delay.\ntype:uint64,[1..11]");
 	toml.put ("unchecked_cutoff_time", unchecked_cutoff_time.count (), "Number of seconds before deleting an unchecked entry.\nWarning: lower values (e.g., 3600 seconds, or 1 hour) may result in unsuccessful bootstraps, especially a bootstrap from scratch.\ntype:seconds");
 	toml.put ("tcp_io_timeout", tcp_io_timeout.count (), "Timeout for TCP connect-, read- and write operations.\nWarning: a low value (e.g., below 5 seconds) may result in TCP connections failing.\ntype:seconds");
 	toml.put ("pow_sleep_interval", pow_sleep_interval.count (), "Time to sleep between batch work generation attempts. Reduces max CPU usage at the expense of a longer generation time.\ntype:nanoseconds");
@@ -484,8 +483,6 @@ nano::error nano::node_config::deserialize_toml (nano::tomlconfig & toml)
 		toml.get ("vote_generator_delay", delay_l);
 		vote_generator_delay = std::chrono::milliseconds (delay_l);
 
-		toml.get<unsigned> ("vote_generator_threshold", vote_generator_threshold);
-
 		auto block_processor_batch_max_time_l = block_processor_batch_max_time.count ();
 		toml.get ("block_processor_batch_max_time", block_processor_batch_max_time_l);
 		block_processor_batch_max_time = std::chrono::milliseconds (block_processor_batch_max_time_l);
@@ -600,10 +597,6 @@ nano::error nano::node_config::deserialize_toml (nano::tomlconfig & toml)
 		{
 			toml.get_error ().set ("bandwidth_limit unbounded = 0, default = 10485760, max = 18446744073709551615");
 		}
-		if (vote_generator_threshold < 1 || vote_generator_threshold > 11)
-		{
-			toml.get_error ().set ("vote_generator_threshold must be a number between 1 and 11");
-		}
 		if (max_work_generate_multiplier < 1)
 		{
 			toml.get_error ().set ("max_work_generate_multiplier must be greater than or equal to 1");

nano/node/nodeconfig.hpp

@@ -73,7 +73,6 @@ class node_config
 	nano::amount vote_minimum{ nano::Knano_ratio }; // 1000 nano
 	nano::amount rep_crawler_weight_minimum{ "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF" };
 	std::chrono::milliseconds vote_generator_delay{ std::chrono::milliseconds (100) };
-	unsigned vote_generator_threshold{ 3 };
 	nano::amount online_weight_minimum{ 60000 * nano::Knano_ratio }; // 60 million nano
 	/*
 	 * The minimum vote weight that a representative must have for its vote to be counted.

nano/node/vote_generator.cpp

@@ -287,31 +287,36 @@ void nano::vote_generator::run ()
 	nano::unique_lock<nano::mutex> lock{ mutex };
 	while (!stopped)
 	{
-		if (candidates.size () >= nano::network::confirm_ack_hashes_max)
+		condition.wait_for (lock, config.vote_generator_delay, [this] () { return broadcast_predicate () || !requests.empty (); });
+
+		if (broadcast_predicate ())
 		{
 			broadcast (lock);
+			next_broadcast = std::chrono::steady_clock::now () + std::chrono::milliseconds (config.vote_generator_delay);
 		}
-		else if (!requests.empty ())
+
+		if (!requests.empty ())
 		{
 			auto request (requests.front ());
 			requests.pop_front ();
 			reply (lock, std::move (request));
 		}
-		else
-		{
-			condition.wait_for (lock, config.vote_generator_delay, [this] () { return this->candidates.size () >= nano::network::confirm_ack_hashes_max; });
-			if (candidates.size () >= config.vote_generator_threshold && candidates.size () < nano::network::confirm_ack_hashes_max)
-			{
-				condition.wait_for (lock, config.vote_generator_delay, [this] () { return this->candidates.size () >= nano::network::confirm_ack_hashes_max; });
-			}
-			if (!candidates.empty ())
-			{
-				broadcast (lock);
-			}
-		}
 	}
 }
 
+bool nano::vote_generator::broadcast_predicate () const
+{
+	if (candidates.size () >= nano::network::confirm_ack_hashes_max)
+	{
+		return true;
+	}
+	if (candidates.size () > 0 && std::chrono::steady_clock::now () > next_broadcast)
+	{
+		return true;
+	}
+	return false;
+}
+
 nano::container_info nano::vote_generator::container_info () const
 {
 	nano::lock_guard<nano::mutex> guard{ mutex };

nano/node/vote_generator.hpp

@@ -30,6 +30,7 @@ class vote_generator final
 	using candidate_t = std::pair<nano::root, nano::block_hash>;
 	using request_t = std::pair<std::vector<candidate_t>, std::shared_ptr<nano::transport::channel>>;
 	using queue_entry_t = std::pair<nano::root, nano::block_hash>;
+	std::chrono::steady_clock::time_point next_broadcast = { std::chrono::steady_clock::now () };
 
 public:
 	vote_generator (nano::node_config const &, nano::node &, nano::ledger &, nano::wallets &, nano::vote_processor &, nano::local_vote_history &, nano::network &, nano::stats &, nano::logger &, bool is_final);
@@ -56,6 +57,7 @@ class vote_generator final
 	void broadcast_action (std::shared_ptr<nano::vote> const &) const;
 	void process_batch (std::deque<queue_entry_t> & batch);
 	bool should_vote (transaction_variant_t const &, nano::root const &, nano::block_hash const &) const;
+	bool broadcast_predicate () const;
 
 private:
 	std::function<void (std::shared_ptr<nano::vote> const &, std::shared_ptr<nano::transport::channel> &)> reply_action; // must be set only during initialization by using set_reply_action