Process cemented elections with exclusive lock to avoid races
This commit is contained in:
		
					parent
					
						
							
								b6efeb699a
							
						
					
				
			
			
				commit
				
					
						82d9d2d323
					
				
			
		
					 2 changed files with 38 additions and 16 deletions
				
			
		| 
						 | 
				
			
			@ -31,11 +31,24 @@ nano::active_elections::active_elections (nano::node & node_a, nano::confirming_
 | 
			
		|||
 | 
			
		||||
	// Cementing blocks might implicitly confirm dependent elections
 | 
			
		||||
	confirming_set.batch_cemented.add ([this] (auto const & cemented) {
 | 
			
		||||
		auto transaction = node.ledger.tx_begin_read ();
 | 
			
		||||
		for (auto const & [block, confirmation_root, source_election] : cemented)
 | 
			
		||||
		std::deque<block_cemented_result> results;
 | 
			
		||||
		{
 | 
			
		||||
			transaction.refresh_if_needed ();
 | 
			
		||||
			block_cemented (transaction, block, confirmation_root, source_election);
 | 
			
		||||
			// Process all cemented blocks while holding the lock to avoid races where an election for a block that is already cemented is inserted
 | 
			
		||||
			nano::lock_guard<nano::mutex> guard{ mutex };
 | 
			
		||||
			for (auto const & [block, confirmation_root, source_election] : cemented)
 | 
			
		||||
			{
 | 
			
		||||
				auto result = block_cemented (block, confirmation_root, source_election);
 | 
			
		||||
				results.push_back (result);
 | 
			
		||||
			}
 | 
			
		||||
		}
 | 
			
		||||
		{
 | 
			
		||||
			// TODO: This could be offloaded to a separate notification worker, profiling is needed
 | 
			
		||||
			auto transaction = node.ledger.tx_begin_read ();
 | 
			
		||||
			for (auto const & [status, votes] : results)
 | 
			
		||||
			{
 | 
			
		||||
				transaction.refresh_if_needed ();
 | 
			
		||||
				notify_observers (transaction, status, votes);
 | 
			
		||||
			}
 | 
			
		||||
		}
 | 
			
		||||
	});
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -83,16 +96,17 @@ void nano::active_elections::stop ()
 | 
			
		|||
	clear ();
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
void nano::active_elections::block_cemented (nano::secure::transaction const & transaction, std::shared_ptr<nano::block> const & block, nano::block_hash const & confirmation_root, std::shared_ptr<nano::election> const & source_election)
 | 
			
		||||
auto nano::active_elections::block_cemented (std::shared_ptr<nano::block> const & block, nano::block_hash const & confirmation_root, std::shared_ptr<nano::election> const & source_election) -> block_cemented_result
 | 
			
		||||
{
 | 
			
		||||
	debug_assert (!mutex.try_lock ());
 | 
			
		||||
	debug_assert (node.block_confirmed (block->hash ()));
 | 
			
		||||
 | 
			
		||||
	// Dependent elections are implicitly confirmed when their block is cemented
 | 
			
		||||
	auto dependend_election = election (block->qualified_root ());
 | 
			
		||||
	auto dependend_election = election_impl (block->qualified_root ());
 | 
			
		||||
	if (dependend_election)
 | 
			
		||||
	{
 | 
			
		||||
		node.stats.inc (nano::stat::type::active_elections, nano::stat::detail::confirm_dependent);
 | 
			
		||||
		dependend_election->try_confirm (block->hash ());
 | 
			
		||||
		dependend_election->try_confirm (block->hash ()); // TODO: This should either confirm or cancel the election
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	nano::election_status status;
 | 
			
		||||
| 
						 | 
				
			
			@ -126,7 +140,7 @@ void nano::active_elections::block_cemented (nano::secure::transaction const & t
 | 
			
		|||
	nano::log::arg{ "confirmation_root", confirmation_root },
 | 
			
		||||
	nano::log::arg{ "source_election", source_election });
 | 
			
		||||
 | 
			
		||||
	notify_observers (transaction, status, votes);
 | 
			
		||||
	return { status, votes };
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
void nano::active_elections::notify_observers (nano::secure::transaction const & transaction, nano::election_status const & status, std::vector<nano::vote_with_weight_info> const & votes) const
 | 
			
		||||
| 
						 | 
				
			
			@ -443,11 +457,17 @@ bool nano::active_elections::active (nano::block const & block_a) const
 | 
			
		|||
	return roots.get<tag_root> ().find (block_a.qualified_root ()) != roots.get<tag_root> ().end ();
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
std::shared_ptr<nano::election> nano::active_elections::election (nano::qualified_root const & root_a) const
 | 
			
		||||
std::shared_ptr<nano::election> nano::active_elections::election (nano::qualified_root const & root) const
 | 
			
		||||
{
 | 
			
		||||
	std::shared_ptr<nano::election> result;
 | 
			
		||||
	nano::lock_guard<nano::mutex> lock{ mutex };
 | 
			
		||||
	auto existing = roots.get<tag_root> ().find (root_a);
 | 
			
		||||
	return election_impl (root);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
std::shared_ptr<nano::election> nano::active_elections::election_impl (nano::qualified_root const & root) const
 | 
			
		||||
{
 | 
			
		||||
	debug_assert (!mutex.try_lock ());
 | 
			
		||||
	std::shared_ptr<nano::election> result;
 | 
			
		||||
	auto existing = roots.get<tag_root> ().find (root);
 | 
			
		||||
	if (existing != roots.get<tag_root> ().end ())
 | 
			
		||||
	{
 | 
			
		||||
		result = existing->election;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -105,7 +105,7 @@ public:
 | 
			
		|||
	bool active (nano::qualified_root const &) const;
 | 
			
		||||
	std::shared_ptr<nano::election> election (nano::qualified_root const &) const;
 | 
			
		||||
	// Returns a list of elections sorted by difficulty
 | 
			
		||||
	std::vector<std::shared_ptr<nano::election>> list_active (std::size_t = std::numeric_limits<std::size_t>::max ());
 | 
			
		||||
	std::vector<std::shared_ptr<nano::election>> list_active (std::size_t max_count = std::numeric_limits<std::size_t>::max ());
 | 
			
		||||
	bool erase (nano::block const &);
 | 
			
		||||
	bool erase (nano::qualified_root const &);
 | 
			
		||||
	bool empty () const;
 | 
			
		||||
| 
						 | 
				
			
			@ -133,11 +133,13 @@ private:
 | 
			
		|||
	void request_confirm (nano::unique_lock<nano::mutex> &);
 | 
			
		||||
	// 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;
 | 
			
		||||
 | 
			
		||||
	using block_cemented_result = std::pair<nano::election_status, std::vector<nano::vote_with_weight_info>>;
 | 
			
		||||
	block_cemented_result block_cemented (std::shared_ptr<nano::block> const & block, nano::block_hash const & confirmation_root, std::shared_ptr<nano::election> const & source_election);
 | 
			
		||||
	void notify_observers (nano::secure::transaction const &, nano::election_status const & status, std::vector<nano::vote_with_weight_info> const & votes) const;
 | 
			
		||||
	void block_cemented (nano::secure::transaction const &, std::shared_ptr<nano::block> const & block, nano::block_hash const & confirmation_root, std::shared_ptr<nano::election> const & source_election);
 | 
			
		||||
 | 
			
		||||
	std::shared_ptr<nano::election> election_impl (nano::qualified_root const &) const;
 | 
			
		||||
	std::vector<std::shared_ptr<nano::election>> list_active_impl (std::size_t max_count) const;
 | 
			
		||||
 | 
			
		||||
private: // Dependencies
 | 
			
		||||
	active_elections_config const & config;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue