Remove election_winner_details

This commit is contained in:
Piotr Wójcik 2024-10-15 17:30:57 +02:00
commit e456b2282b
8 changed files with 37 additions and 148 deletions

View file

@ -111,7 +111,6 @@ TEST (confirmation_callback, observer_callbacks)
ASSERT_EQ (2, node->stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in));
ASSERT_EQ (3, node->ledger.cemented_count ());
ASSERT_EQ (0, node->active.election_winner_details_size ());
}
// The callback and confirmation history should only be updated after confirmation height is set (and not just after voting)
@ -186,7 +185,6 @@ TEST (confirmation_callback, confirmed_history)
ASSERT_TIMELY_EQ (5s, 1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::inactive_conf_height, nano::stat::dir::out));
ASSERT_TIMELY_EQ (5s, 2, node->stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in));
ASSERT_EQ (3, node->ledger.cemented_count ());
ASSERT_EQ (0, node->active.election_winner_details_size ());
}
TEST (confirmation_callback, dependent_election)
@ -248,29 +246,4 @@ TEST (confirmation_callback, dependent_election)
ASSERT_TIMELY_EQ (5s, 1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::active_conf_height, nano::stat::dir::out));
ASSERT_TIMELY_EQ (5s, 1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::inactive_conf_height, nano::stat::dir::out));
ASSERT_EQ (4, node->ledger.cemented_count ());
ASSERT_EQ (0, node->active.election_winner_details_size ());
}
TEST (confirmation_callback, election_winner_details_clearing_node_process_confirmed)
{
// Make sure election_winner_details is also cleared if the block never enters the confirmation height processor from node::process_confirmed
nano::test::system system (1);
auto node = system.nodes.front ();
nano::block_builder builder;
auto send = builder
.send ()
.previous (nano::dev::genesis->hash ())
.destination (nano::dev::genesis_key.pub)
.balance (nano::dev::constants.genesis_amount - nano::Knano_ratio)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (*system.work.generate (nano::dev::genesis->hash ()))
.build ();
// Add to election_winner_details. Use an unrealistic iteration so that it should fall into the else case and do a cleanup
node->active.add_election_winner_details (send->hash (), nullptr);
nano::election_status election;
election.winner = send;
node->process_confirmed (election, 1000000);
ASSERT_EQ (0, node->active.election_winner_details_size ());
}

View file

@ -752,29 +752,6 @@ TEST (ledger_confirm, observers)
ASSERT_EQ (2, node1->ledger.cemented_count ());
}
TEST (ledger_confirm, election_winner_details_clearing_node_process_confirmed)
{
// Make sure election_winner_details is also cleared if the block never enters the confirmation height processor from node::process_confirmed
nano::test::system system (1);
auto node = system.nodes.front ();
nano::block_builder builder;
auto send = builder
.send ()
.previous (nano::dev::genesis->hash ())
.destination (nano::dev::genesis_key.pub)
.balance (nano::dev::constants.genesis_amount - nano::Knano_ratio)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (*system.work.generate (nano::dev::genesis->hash ()))
.build ();
// Add to election_winner_details. Use an unrealistic iteration so that it should fall into the else case and do a cleanup
node->active.add_election_winner_details (send->hash (), nullptr);
nano::election_status election;
election.winner = send;
node->process_confirmed (election, 1000000);
ASSERT_EQ (0, node->active.election_winner_details_size ());
}
TEST (ledger_confirm, pruned_source)
{
nano::test::system system;

View file

@ -412,6 +412,7 @@ enum class detail
// active_elections
started,
stopped,
confirm_dependent,
// unchecked
put,

View file

@ -25,24 +25,16 @@ nano::active_elections::active_elections (nano::node & node_a, nano::confirming_
confirming_set{ confirming_set_a },
block_processor{ block_processor_a },
recently_confirmed{ config.confirmation_cache },
recently_cemented{ config.confirmation_history_size },
election_time_to_live{ node_a.network_params.network.is_dev_network () ? 0s : 2s }
recently_cemented{ config.confirmation_history_size }
{
count_by_behavior.fill (0); // Zero initialize array
confirming_set.batch_cemented.add ([this] (auto const & cemented) {
auto transaction = node.ledger.tx_begin_read ();
for (auto const & [block, confirmation_root, election] : cemented)
for (auto const & [block, confirmation_root, source_election] : cemented)
{
transaction.refresh_if_needed ();
block_cemented_callback (transaction, block, confirmation_root);
}
});
confirming_set.already_cemented.add ([this] (auto const & already_cemented) {
for (auto const & hash : already_cemented)
{
block_already_cemented_callback (hash);
block_cemented (transaction, block, confirmation_root, source_election);
}
});
@ -91,28 +83,31 @@ void nano::active_elections::stop ()
clear ();
}
void nano::active_elections::block_cemented_callback (nano::secure::transaction const & transaction, std::shared_ptr<nano::block> const & block, nano::block_hash const & confirmation_root)
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)
{
debug_assert (node.block_confirmed (block->hash ()));
if (auto election_l = election (block->qualified_root ()))
// Dependent elections are implicitly confirmed when their block is cemented
auto dependend_election = election (block->qualified_root ());
if (dependend_election)
{
election_l->try_confirm (block->hash ());
node.stats.inc (nano::stat::type::active_elections, nano::stat::detail::confirm_dependent);
dependend_election->try_confirm (block->hash ());
}
auto election = remove_election_winner_details (block->hash ());
nano::election_status status;
std::vector<nano::vote_with_weight_info> votes;
status.winner = block;
if (election)
{
status = election->get_status ();
votes = election->votes_with_weight ();
}
if (block->hash () == confirmation_root)
// Check if the currently cemented block was part of an election that triggered the confirmation
if (source_election && source_election->qualified_root == block->qualified_root ())
{
status = source_election->get_status ();
debug_assert (status.winner->hash () == block->hash ());
votes = source_election->votes_with_weight ();
status.type = nano::election_status_type::active_confirmed_quorum;
}
else if (election)
else if (dependend_election)
{
status.type = nano::election_status_type::active_confirmation_height;
}
@ -120,12 +115,16 @@ void nano::active_elections::block_cemented_callback (nano::secure::transaction
{
status.type = nano::election_status_type::inactive_confirmation_height;
}
recently_cemented.put (status);
node.stats.inc (nano::stat::type::active_elections, nano::stat::detail::cemented);
node.stats.inc (nano::stat::type::active_elections_cemented, to_stat_detail (status.type));
node.logger.trace (nano::log::type::active_elections, nano::log::detail::active_cemented, nano::log::arg{ "election", election });
node.logger.trace (nano::log::type::active_elections, nano::log::detail::active_cemented,
nano::log::arg{ "block", block },
nano::log::arg{ "confirmation_root", confirmation_root },
nano::log::arg{ "source_election", source_election });
notify_observers (transaction, status, votes);
@ -185,39 +184,6 @@ void nano::active_elections::activate_successors (nano::secure::transaction cons
}
}
void nano::active_elections::add_election_winner_details (nano::block_hash const & hash_a, std::shared_ptr<nano::election> const & election_a)
{
nano::lock_guard<nano::mutex> guard{ election_winner_details_mutex };
election_winner_details.emplace (hash_a, election_a);
}
std::shared_ptr<nano::election> nano::active_elections::remove_election_winner_details (nano::block_hash const & hash_a)
{
std::shared_ptr<nano::election> result;
{
nano::lock_guard<nano::mutex> guard{ election_winner_details_mutex };
auto existing = election_winner_details.find (hash_a);
if (existing != election_winner_details.end ())
{
result = existing->second;
election_winner_details.erase (existing);
}
}
vacancy_update ();
return result;
}
void nano::active_elections::block_already_cemented_callback (nano::block_hash const & hash_a)
{
// Depending on timing there is a situation where the election_winner_details is not reset.
// This can happen when a block wins an election, and the block is confirmed + observer
// called before the block hash gets added to election_winner_details. If the block is confirmed
// callbacks have already been done, so we can safely just remove it.
remove_election_winner_details (hash_a);
}
int64_t nano::active_elections::limit (nano::election_behavior behavior) const
{
switch (behavior)
@ -265,7 +231,7 @@ int64_t nano::active_elections::vacancy (nano::election_behavior behavior) const
};
auto election_winners_vacancy = [this] () -> int64_t {
return static_cast<int64_t> (config.max_election_winners) - static_cast<int64_t> (election_winner_details_size ());
return static_cast<int64_t> (config.max_election_winners) - static_cast<int64_t> (confirming_set.size ());
};
return std::min (election_vacancy (behavior), election_winners_vacancy ());
@ -572,12 +538,6 @@ bool nano::active_elections::publish (std::shared_ptr<nano::block> const & block
return result;
}
std::size_t nano::active_elections::election_winner_details_size () const
{
nano::lock_guard<nano::mutex> guard{ election_winner_details_mutex };
return election_winner_details.size ();
}
void nano::active_elections::clear ()
{
// TODO: Call erased_callback for each election
@ -595,7 +555,6 @@ nano::container_info nano::active_elections::container_info () const
nano::container_info info;
info.put ("roots", roots.size ());
info.put ("election_winner_details", election_winner_details_size ());
info.put ("normal", static_cast<std::size_t> (count_by_behavior[nano::election_behavior::priority]));
info.put ("hinted", static_cast<std::size_t> (count_by_behavior[nano::election_behavior::hinted]));
info.put ("optimistic", static_cast<std::size_t> (count_by_behavior[nano::election_behavior::optimistic]));

View file

@ -123,10 +123,6 @@ public:
int64_t vacancy (nano::election_behavior behavior) const;
std::function<void ()> vacancy_update{ [] () {} };
std::size_t election_winner_details_size () const;
void add_election_winner_details (nano::block_hash const &, std::shared_ptr<nano::election> const &);
std::shared_ptr<nano::election> remove_election_winner_details (nano::block_hash const &);
nano::container_info container_info () const;
private:
@ -139,8 +135,7 @@ private:
std::vector<std::shared_ptr<nano::election>> list_active_impl (std::size_t) const;
void activate_successors (nano::secure::transaction const &, std::shared_ptr<nano::block> const & block);
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_callback (nano::secure::transaction const &, std::shared_ptr<nano::block> const & block, nano::block_hash const & confirmation_root);
void block_already_cemented_callback (nano::block_hash const & hash);
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);
private: // Dependencies
active_elections_config const & config;
@ -157,12 +152,6 @@ public:
mutable nano::mutex mutex{ mutex_identifier (mutexes::active) };
private:
mutable nano::mutex election_winner_details_mutex{ mutex_identifier (mutexes::election_winner_details) };
std::unordered_map<nano::block_hash, std::shared_ptr<nano::election>> election_winner_details;
// Maximum time an election can be kept active if it is extending the container
std::chrono::seconds const election_time_to_live;
/** Keeps track of number of elections by election behavior (normal, hinted, optimistic) */
nano::enum_array<nano::election_behavior, int64_t> count_by_behavior{};

View file

@ -35,18 +35,16 @@ nano::election::election (nano::node & node_a, std::shared_ptr<nano::block> cons
last_blocks.emplace (block_a->hash (), block_a);
}
void nano::election::confirm_once (nano::unique_lock<nano::mutex> & lock_a)
void nano::election::confirm_once (nano::unique_lock<nano::mutex> & lock)
{
debug_assert (lock_a.owns_lock ());
debug_assert (lock.owns_lock ());
debug_assert (!mutex.try_lock ());
// This must be kept above the setting of election state, as dependent confirmed elections require up to date changes to election_winner_details
nano::unique_lock<nano::mutex> election_winners_lk{ node.active.election_winner_details_mutex };
auto just_confirmed = state_m != nano::election_state::confirmed;
bool just_confirmed = state_m != nano::election_state::confirmed;
state_m = nano::election_state::confirmed;
if (just_confirmed && (node.active.election_winner_details.count (status.winner->hash ()) == 0))
if (just_confirmed)
{
node.active.election_winner_details.emplace (status.winner->hash (), shared_from_this ());
election_winners_lk.unlock ();
status.election_end = std::chrono::duration_cast<std::chrono::milliseconds> (std::chrono::system_clock::now ().time_since_epoch ());
status.election_duration = std::chrono::duration_cast<std::chrono::milliseconds> (std::chrono::steady_clock::now () - election_start);
status.confirmation_request_count = confirmation_request_count;
@ -62,11 +60,11 @@ void nano::election::confirm_once (nano::unique_lock<nano::mutex> & lock_a)
nano::log::arg{ "qualified_root", qualified_root },
nano::log::arg{ "status", current_status_locked () });
lock_a.unlock ();
node.confirming_set.add (status_l.winner->hash (), shared_from_this ());
node.election_workers.push_task ([node_l = node.shared (), status_l, confirmation_action_l = confirmation_action] () {
node_l->process_confirmed (status_l);
lock.unlock ();
node.election_workers.push_task ([status_l, confirmation_action_l = confirmation_action] () {
if (confirmation_action_l)
{
confirmation_action_l (status_l.winner);
@ -76,7 +74,7 @@ void nano::election::confirm_once (nano::unique_lock<nano::mutex> & lock_a)
else
{
node.stats.inc (nano::stat::type::election, nano::stat::detail::confirm_once_failed);
lock_a.unlock ();
lock.unlock ();
}
}
@ -416,6 +414,7 @@ void nano::election::confirm_if_quorum (nano::unique_lock<nano::mutex> & lock_a)
if (final_weight >= node.online_reps.delta ())
{
confirm_once (lock_a);
debug_assert (!lock_a.owns_lock ());
}
}
}
@ -429,6 +428,7 @@ void nano::election::try_confirm (nano::block_hash const & hash)
if (!confirmed_locked ())
{
confirm_once (election_lock);
debug_assert (!election_lock.owns_lock ());
}
}
}

View file

@ -1172,7 +1172,6 @@ void nano::node::process_confirmed (nano::election_status const & status_a, uint
stats.inc (nano::stat::type::process_confirmed, nano::stat::detail::timeout);
// Do some cleanup due to this block never being processed by confirmation height processor
active.remove_election_winner_details (hash);
}
}

View file

@ -717,7 +717,6 @@ TEST (confirmation_height, many_accounts_single_confirmation)
ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed_unbounded, nano::stat::dir::in), 0);
ASSERT_TIMELY_EQ (40s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::dir::out));
ASSERT_TIMELY_EQ (10s, node->active.election_winner_details_size (), 0);
}
TEST (confirmation_height, many_accounts_many_confirmations)
@ -792,8 +791,6 @@ TEST (confirmation_height, many_accounts_many_confirmations)
ASSERT_EQ (cemented_count, node->ledger.cemented_count ());
ASSERT_TIMELY_EQ (20s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::dir::out));
ASSERT_TIMELY_EQ (10s, node->active.election_winner_details_size (), 0);
}
TEST (confirmation_height, long_chains)
@ -939,7 +936,6 @@ TEST (confirmation_height, long_chains)
ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed_unbounded, nano::stat::dir::in), 0);
ASSERT_TIMELY_EQ (40s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::dir::out));
ASSERT_TIMELY_EQ (10s, node->active.election_winner_details_size (), 0);
}
TEST (confirmation_height, dynamic_algorithm)
@ -987,7 +983,6 @@ TEST (confirmation_height, dynamic_algorithm)
ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in), num_blocks);
ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed_bounded, nano::stat::dir::in), 1);
ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed_unbounded, nano::stat::dir::in), num_blocks - 1);
ASSERT_TIMELY_EQ (10s, node->active.election_winner_details_size (), 0);
}
TEST (confirmation_height, many_accounts_send_receive_self)
@ -1118,10 +1113,6 @@ TEST (confirmation_height, many_accounts_send_receive_self)
}
system.deadline_set (60s);
while (node->active.election_winner_details_size () > 0)
{
ASSERT_NO_ERROR (system.poll ());
}
}
// Same as the many_accounts_send_receive_self test, except works on the confirmation height processor directly