From 20d213428e7ff2c81b041210f2a9d407c93a7ecf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 9 Oct 2023 14:50:53 +0200 Subject: [PATCH 01/13] Revert election status change This degrades performance, should be solved without additional coupling between election and conf height processor --- nano/node/active_transactions.cpp | 7 +++---- nano/node/election.cpp | 21 +++++---------------- nano/node/election.hpp | 9 +-------- 3 files changed, 9 insertions(+), 28 deletions(-) diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index bffa0b6243..9aab28cced 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -307,7 +307,7 @@ void nano::active_transactions::request_confirm (nano::unique_lock bool const confirmed_l (election_l->confirmed ()); unconfirmed_count_l += !confirmed_l; - if (confirmed_l || election_l->transition_time (solicitor)) + if (election_l->transition_time (solicitor)) { erase (election_l->qualified_root); } @@ -367,7 +367,7 @@ void nano::active_transactions::cleanup_election (nano::unique_lock nano::stat::type nano::active_transactions::completion_type (nano::election const & election) const { - if (election.status_confirmed ()) + if (election.confirmed ()) { return nano::stat::type::active_confirmed; } @@ -700,8 +700,7 @@ boost::optional nano::active_transactions::confirm_b nano::unique_lock election_lock{ election->mutex }; if (election->status.winner && election->status.winner->hash () == hash) { - // Determine if the block was confirmed explicitly via election confirmation or implicitly via confirmation height - if (!election->status_confirmed ()) + if (!election->confirmed ()) { election->confirm_once (election_lock, nano::election_status_type::active_confirmation_height); status_type = nano::election_status_type::active_confirmation_height; diff --git a/nano/node/election.cpp b/nano/node/election.cpp index 79243bfece..22c3f573b9 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -124,11 +124,6 @@ bool nano::election::state_change (nano::election::state_t expected_a, nano::ele return result; } -bool nano::election::confirmed (nano::unique_lock & lock) const -{ - return node.block_confirmed (status.winner->hash ()); -} - std::chrono::milliseconds nano::election::confirm_req_time () const { switch (behavior ()) @@ -162,12 +157,6 @@ void nano::election::transition_active () } bool nano::election::confirmed () const -{ - nano::unique_lock lock{ mutex }; - return confirmed (lock); -} - -bool nano::election::status_confirmed () const { return state_m == nano::election::state_t::confirmed || state_m == nano::election::state_t::expired_confirmed; } @@ -194,7 +183,7 @@ void nano::election::broadcast_vote () nano::unique_lock lock{ mutex }; if (last_vote + std::chrono::milliseconds (node.config.network_params.network.vote_broadcast_interval) < std::chrono::steady_clock::now ()) { - broadcast_vote_impl (lock); + broadcast_vote_impl (); last_vote = std::chrono::steady_clock::now (); } } @@ -442,7 +431,7 @@ nano::election_vote_result nano::election::vote (nano::account const & rep, uint node.stats.inc (nano::stat::type::election, vote_source_a == vote_source::live ? nano::stat::detail::vote_new : nano::stat::detail::vote_cached); - if (!confirmed (lock)) + if (!confirmed ()) { confirm_if_quorum (lock); } @@ -454,7 +443,7 @@ bool nano::election::publish (std::shared_ptr const & block_a) nano::unique_lock lock{ mutex }; // Do not insert new blocks if already confirmed - auto result = confirmed (lock); + auto result (confirmed ()); if (!result && last_blocks.size () >= max_blocks && last_blocks.find (block_a->hash ()) == last_blocks.end ()) { if (!replace_by_weight (lock, block_a->hash ())) @@ -507,7 +496,7 @@ std::shared_ptr nano::election::winner () const return status.winner; } -void nano::election::broadcast_vote_impl (nano::unique_lock & lock) +void nano::election::broadcast_vote_impl () { debug_assert (!mutex.try_lock ()); @@ -515,7 +504,7 @@ void nano::election::broadcast_vote_impl (nano::unique_lock & lock) { node.stats.inc (nano::stat::type::election, nano::stat::detail::generate_vote); - if (confirmed (lock) || have_quorum (tally_impl ())) + if (confirmed () || have_quorum (tally_impl ())) { node.stats.inc (nano::stat::type::election, nano::stat::detail::generate_vote_final); node.final_generator.add (root, status.winner->hash ()); // Broadcasts vote to the network diff --git a/nano/node/election.hpp b/nano/node/election.hpp index 721503850a..0ee0cd9a24 100644 --- a/nano/node/election.hpp +++ b/nano/node/election.hpp @@ -110,19 +110,12 @@ class election final : public std::enable_shared_from_this bool valid_change (nano::election::state_t, nano::election::state_t) const; bool state_change (nano::election::state_t, nano::election::state_t); - bool confirmed (nano::unique_lock & lock) const; public: // State transitions bool transition_time (nano::confirmation_solicitor &); void transition_active (); public: // Status - // Returns true when the election is confirmed in memory - // Elections will first confirm in memory once sufficient votes have been received - bool status_confirmed () const; - // Returns true when the winning block is durably confirmed in the ledger. - // Later once the confirmation height processor has updated the confirmation height it will be confirmed on disk - // It is possible for an election to be confirmed on disk but not in memory, for instance if implicitly confirmed via confirmation height bool confirmed () const; bool failed () const; nano::election_extended_status current_status () const; @@ -175,7 +168,7 @@ class election final : public std::enable_shared_from_this * Broadcast vote for current election winner. Generates final vote if reached quorum or already confirmed * Requires mutex lock */ - void broadcast_vote_impl (nano::unique_lock & lock); + void broadcast_vote_impl (); void remove_votes (nano::block_hash const &); void remove_block (nano::block_hash const &); bool replace_by_weight (nano::unique_lock & lock_a, nano::block_hash const &); From ec7f9df561b70c6193a8cf8b47e799cf41f565c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sun, 17 Sep 2023 23:58:30 +0200 Subject: [PATCH 02/13] Refactor `vote_cache::entry` --- nano/core_test/active_transactions.cpp | 6 +- nano/core_test/vote_cache.cpp | 50 ++++++++-------- nano/node/election.cpp | 2 +- nano/node/scheduler/hinted.cpp | 2 +- nano/node/vote_cache.cpp | 80 ++++++++++++++++++++------ nano/node/vote_cache.hpp | 34 ++++++++--- nano/secure/common.cpp | 5 ++ nano/secure/common.hpp | 4 ++ 8 files changed, 127 insertions(+), 56 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index 066fa04eae..d481c2c3de 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -359,7 +359,7 @@ TEST (active_transactions, inactive_votes_cache_existing_vote) node.inactive_vote_cache.vote (send->hash (), vote1); auto cache = node.inactive_vote_cache.find (send->hash ()); ASSERT_TRUE (cache); - ASSERT_EQ (1, cache->voters.size ()); + ASSERT_EQ (1, cache->voters ().size ()); cache->fill (election); // Check that election data is not changed ASSERT_EQ (2, election->votes ().size ()); @@ -417,7 +417,7 @@ TEST (active_transactions, inactive_votes_cache_multiple_votes) node.vote_processor.vote (vote2, std::make_shared (node, node)); ASSERT_TIMELY (5s, node.inactive_vote_cache.find (send1->hash ())); - ASSERT_TIMELY (5s, node.inactive_vote_cache.find (send1->hash ())->voters.size () == 2); + ASSERT_TIMELY (5s, node.inactive_vote_cache.find (send1->hash ())->voters ().size () == 2); ASSERT_EQ (1, node.inactive_vote_cache.cache_size ()); node.scheduler.priority.activate (nano::dev::genesis_key.pub, node.store.tx_begin_read ()); std::shared_ptr election; @@ -514,7 +514,7 @@ TEST (active_transactions, inactive_votes_cache_election_start) ASSERT_TRUE (node.active.empty ()); auto send4_cache (node.inactive_vote_cache.find (send4->hash ())); ASSERT_TRUE (send4_cache); - ASSERT_EQ (3, send4_cache->voters.size ()); + ASSERT_EQ (3, send4_cache->voters ().size ()); node.process_active (send3); // An election is started for send6 but does not ASSERT_FALSE (node.block_confirmed_or_being_confirmed (send3->hash ())); diff --git a/nano/core_test/vote_cache.cpp b/nano/core_test/vote_cache.cpp index 09a34be4d6..555be41123 100644 --- a/nano/core_test/vote_cache.cpp +++ b/nano/core_test/vote_cache.cpp @@ -77,11 +77,11 @@ TEST (vote_cache, insert_one_hash) ASSERT_TRUE (vote_cache.find (hash1)); auto peek1 = vote_cache.peek (); ASSERT_TRUE (peek1); - ASSERT_EQ (peek1->hash, hash1); - ASSERT_EQ (peek1->voters.size (), 1); - ASSERT_EQ (peek1->voters.front ().first, rep1.pub); // account - ASSERT_EQ (peek1->voters.front ().second, 1024 * 1024); // timestamp - ASSERT_EQ (peek1->tally, 7); + ASSERT_EQ (peek1->hash (), hash1); + ASSERT_EQ (peek1->voters ().size (), 1); + ASSERT_EQ (peek1->voters ().front ().representative, rep1.pub); // account + ASSERT_EQ (peek1->voters ().front ().timestamp, 1024 * 1024); // timestamp + ASSERT_EQ (peek1->tally (), 7); } /* @@ -106,9 +106,9 @@ TEST (vote_cache, insert_one_hash_many_votes) ASSERT_EQ (1, vote_cache.cache_size ()); auto peek1 = vote_cache.peek (); ASSERT_TRUE (peek1); - ASSERT_EQ (peek1->voters.size (), 3); + ASSERT_EQ (peek1->voters ().size (), 3); // Tally must be the sum of rep weights - ASSERT_EQ (peek1->tally, 7 + 9 + 11); + ASSERT_EQ (peek1->tally (), 7 + 9 + 11); } /* @@ -146,9 +146,9 @@ TEST (vote_cache, insert_many_hashes_many_votes) // Ensure that first entry in queue is the one for hash3 (rep3 has the highest weight of the first 3 reps) auto peek1 = vote_cache.peek (); ASSERT_TRUE (peek1); - ASSERT_EQ (peek1->voters.size (), 1); - ASSERT_EQ (peek1->tally, 11); - ASSERT_EQ (peek1->hash, hash3); + ASSERT_EQ (peek1->voters ().size (), 1); + ASSERT_EQ (peek1->tally (), 11); + ASSERT_EQ (peek1->hash (), hash3); // Now add a vote from rep4 with the highest voting weight vote_cache.vote (vote4->hashes.front (), vote4); @@ -156,23 +156,23 @@ TEST (vote_cache, insert_many_hashes_many_votes) // Ensure that the first entry in queue is now the one for hash1 (rep1 + rep4 tally weight) auto pop1 = vote_cache.pop (); ASSERT_TRUE (pop1); - ASSERT_EQ ((*pop1).voters.size (), 2); - ASSERT_EQ ((*pop1).tally, 7 + 13); - ASSERT_EQ ((*pop1).hash, hash1); + ASSERT_EQ ((*pop1).voters ().size (), 2); + ASSERT_EQ ((*pop1).tally (), 7 + 13); + ASSERT_EQ ((*pop1).hash (), hash1); ASSERT_TRUE (vote_cache.find (hash1)); // Only pop from queue, votes should still be stored in cache // After popping the previous entry, the next entry in queue should be hash3 (rep3 tally weight) auto pop2 = vote_cache.pop (); - ASSERT_EQ ((*pop2).voters.size (), 1); - ASSERT_EQ ((*pop2).tally, 11); - ASSERT_EQ ((*pop2).hash, hash3); + ASSERT_EQ ((*pop2).voters ().size (), 1); + ASSERT_EQ ((*pop2).tally (), 11); + ASSERT_EQ ((*pop2).hash (), hash3); ASSERT_TRUE (vote_cache.find (hash3)); // And last one should be hash2 with rep2 tally weight auto pop3 = vote_cache.pop (); - ASSERT_EQ ((*pop3).voters.size (), 1); - ASSERT_EQ ((*pop3).tally, 9); - ASSERT_EQ ((*pop3).hash, hash2); + ASSERT_EQ ((*pop3).voters ().size (), 1); + ASSERT_EQ ((*pop3).tally (), 9); + ASSERT_EQ ((*pop3).hash (), hash2); ASSERT_TRUE (vote_cache.find (hash2)); ASSERT_TRUE (vote_cache.queue_empty ()); @@ -212,10 +212,10 @@ TEST (vote_cache, insert_newer) auto peek2 = vote_cache.peek (); ASSERT_TRUE (peek2); ASSERT_EQ (1, vote_cache.cache_size ()); - ASSERT_EQ (1, peek2->voters.size ()); + ASSERT_EQ (1, peek2->voters ().size ()); // Second entry should have timestamp greater than the first one - ASSERT_GT (peek2->voters.front ().second, peek1->voters.front ().second); - ASSERT_EQ (peek2->voters.front ().second, std::numeric_limits::max ()); // final timestamp + ASSERT_GT (peek2->voters ().front ().timestamp, peek1->voters ().front ().timestamp); + ASSERT_EQ (peek2->voters ().front ().timestamp, std::numeric_limits::max ()); // final timestamp } /* @@ -236,8 +236,8 @@ TEST (vote_cache, insert_older) auto peek2 = vote_cache.peek (); ASSERT_TRUE (peek2); ASSERT_EQ (1, vote_cache.cache_size ()); - ASSERT_EQ (1, peek2->voters.size ()); - ASSERT_EQ (peek2->voters.front ().second, peek1->voters.front ().second); // timestamp2 == timestamp1 + ASSERT_EQ (1, peek2->voters ().size ()); + ASSERT_EQ (peek2->voters ().front ().timestamp, peek1->voters ().front ().timestamp); // timestamp2 == timestamp1 } /* @@ -298,7 +298,7 @@ TEST (vote_cache, overfill) auto peek1 = vote_cache.peek (); ASSERT_TRUE (peek1); // Check that oldest votes are dropped first - ASSERT_EQ (peek1->tally, 1024); + ASSERT_EQ (peek1->tally (), 1024); } /* diff --git a/nano/node/election.cpp b/nano/node/election.cpp index 22c3f573b9..bcfb576a5f 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -571,7 +571,7 @@ bool nano::election::replace_by_weight (nano::unique_lock & lock_a, std::sort (sorted.begin (), sorted.end (), [] (auto const & left, auto const & right) { return left.second < right.second; }); // Replace if lowest tally is below inactive cache new block weight auto inactive_existing = node.inactive_vote_cache.find (hash_a); - auto inactive_tally = inactive_existing ? inactive_existing->tally : 0; + auto inactive_tally = inactive_existing ? inactive_existing->tally () : 0; if (inactive_tally > 0 && sorted.size () < max_blocks) { // If count of tally items is less than 10, remove any block without tally diff --git a/nano/node/scheduler/hinted.cpp b/nano/node/scheduler/hinted.cpp index 6c5d4933bd..59321a79c3 100644 --- a/nano/node/scheduler/hinted.cpp +++ b/nano/node/scheduler/hinted.cpp @@ -66,7 +66,7 @@ bool nano::scheduler::hinted::run_one (nano::uint128_t const & minimum_tally) { if (auto top = inactive_vote_cache.pop (minimum_tally); top) { - const auto hash = top->hash; // Hash of block we want to hint + const auto hash = top->hash (); // Hash of block we want to hint // Check if block exists auto block = node.block (hash); diff --git a/nano/node/vote_cache.cpp b/nano/node/vote_cache.cpp index 1357ebad71..785e179e3f 100644 --- a/nano/node/vote_cache.cpp +++ b/nano/node/vote_cache.cpp @@ -1,32 +1,48 @@ #include #include +/* + * entry + */ + nano::vote_cache::entry::entry (const nano::block_hash & hash) : - hash{ hash } + hash_m{ hash } { } bool nano::vote_cache::entry::vote (const nano::account & representative, const uint64_t & timestamp, const nano::uint128_t & rep_weight) { - auto existing = std::find_if (voters.begin (), voters.end (), [&representative] (auto const & item) { return item.first == representative; }); - if (existing != voters.end ()) + auto existing = std::find_if (voters_m.begin (), voters_m.end (), [&representative] (auto const & item) { return item.representative == representative; }); + if (existing != voters_m.end ()) { // We already have a vote from this rep // Update timestamp if newer but tally remains unchanged as we already counted this rep weight // It is not essential to keep tally up to date if rep voting weight changes, elections do tally calculations independently, so in the worst case scenario only our queue ordering will be a bit off - if (timestamp > existing->second) + if (timestamp > existing->timestamp) + { + existing->timestamp = timestamp; + if (nano::vote::is_final_timestamp (timestamp)) + { + final_tally_m += rep_weight; + } + return true; + } + else { - existing->second = timestamp; + return false; } - return false; } else { // Vote from an unseen representative, add to list and update tally - if (voters.size () < max_voters) + if (voters_m.size () < max_voters) { - voters.emplace_back (representative, timestamp); - tally += rep_weight; + voters_m.push_back ({ representative, timestamp }); + tally_m += rep_weight; + if (nano::vote::is_final_timestamp (timestamp)) + { + final_tally_m += rep_weight; + } return true; } else @@ -36,12 +52,12 @@ bool nano::vote_cache::entry::vote (const nano::account & representative, const } } -std::size_t nano::vote_cache::entry::fill (std::shared_ptr election) const +std::size_t nano::vote_cache::entry::fill (std::shared_ptr const & election) const { std::size_t inserted = 0; - for (const auto & [rep, timestamp] : voters) + for (const auto & entry : voters_m) { - auto [is_replay, processed] = election->vote (rep, timestamp, hash, nano::election::vote_source::cache); + auto [is_replay, processed] = election->vote (entry.representative, entry.timestamp, hash_m, nano::election::vote_source::cache); if (processed) { inserted++; @@ -52,9 +68,33 @@ std::size_t nano::vote_cache::entry::fill (std::shared_ptr elect std::size_t nano::vote_cache::entry::size () const { - return voters.size (); + return voters_m.size (); } +nano::block_hash nano::vote_cache::entry::hash () const +{ + return hash_m; +} + +nano::uint128_t nano::vote_cache::entry::tally () const +{ + return tally_m; +} + +nano::uint128_t nano::vote_cache::entry::final_tally () const +{ + return final_tally_m; +} + +std::vector nano::vote_cache::entry::voters () const +{ + return voters_m; +} + +/* + * vote_cache + */ + nano::vote_cache::vote_cache (const config config_a) : max_size{ config_a.max_size } { @@ -86,7 +126,7 @@ void nano::vote_cache::vote_impl (const nano::block_hash & hash, const nano::acc if (auto queue_existing = queue_by_hash.find (hash); queue_existing != queue_by_hash.end ()) { queue_by_hash.modify (queue_existing, [&existing] (queue_entry & ent) { - ent.tally = existing->tally; + ent.tally = existing->tally (); }); } } @@ -103,7 +143,7 @@ void nano::vote_cache::vote_impl (const nano::block_hash & hash, const nano::acc { queue_by_hash.erase (queue_existing); } - queue_by_hash.insert ({ hash, cache_entry.tally }); + queue_by_hash.insert ({ hash, cache_entry.tally () }); trim_overflow_locked (); } @@ -142,6 +182,7 @@ std::optional nano::vote_cache::find (const nano::block bool nano::vote_cache::erase (const nano::block_hash & hash) { nano::lock_guard lock{ mutex }; + bool result = false; auto & cache_by_hash = cache.get (); if (auto existing = cache_by_hash.find (hash); existing != cache_by_hash.end ()) @@ -160,6 +201,7 @@ bool nano::vote_cache::erase (const nano::block_hash & hash) std::optional nano::vote_cache::pop (nano::uint128_t const & min_tally) { nano::lock_guard lock{ mutex }; + if (!queue.empty ()) { auto & queue_by_tally = queue.get (); @@ -168,7 +210,7 @@ std::optional nano::vote_cache::pop (nano::uint128_t co { // Here we check whether our best candidate passes the minimum vote tally threshold // If yes, erase it from the queue (but still keep the votes in cache) - if (maybe_cache_entry->tally >= min_tally) + if (maybe_cache_entry->tally () >= min_tally) { queue_by_tally.erase (top); return maybe_cache_entry.value (); @@ -181,13 +223,14 @@ std::optional nano::vote_cache::pop (nano::uint128_t co std::optional nano::vote_cache::peek (nano::uint128_t const & min_tally) const { nano::lock_guard lock{ mutex }; + if (!queue.empty ()) { auto & queue_by_tally = queue.get (); auto top = std::prev (queue_by_tally.end ()); // Iterator to element with the highest tally if (auto maybe_cache_entry = find_locked (top->hash); maybe_cache_entry) { - if (maybe_cache_entry->tally >= min_tally) + if (maybe_cache_entry->tally () >= min_tally) { return maybe_cache_entry.value (); } @@ -199,13 +242,14 @@ std::optional nano::vote_cache::peek (nano::uint128_t c void nano::vote_cache::trigger (const nano::block_hash & hash) { nano::lock_guard lock{ mutex }; + auto & queue_by_hash = queue.get (); // Only reinsert to queue if it is not already in queue and there are votes in passive cache if (auto existing_queue = queue_by_hash.find (hash); existing_queue == queue_by_hash.end ()) { if (auto maybe_cache_entry = find_locked (hash); maybe_cache_entry) { - queue_by_hash.insert ({ hash, maybe_cache_entry->tally }); + queue_by_hash.insert ({ hash, maybe_cache_entry->tally () }); trim_overflow_locked (); } diff --git a/nano/node/vote_cache.hpp b/nano/node/vote_cache.hpp index a29443b0ee..48bfd2fd53 100644 --- a/nano/node/vote_cache.hpp +++ b/nano/node/vote_cache.hpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -51,13 +52,16 @@ class vote_cache final class entry final { public: - constexpr static int max_voters = 40; + struct voter_entry + { + nano::account representative; + uint64_t timestamp; + }; - explicit entry (nano::block_hash const & hash); + public: + constexpr static int max_voters = 80; - nano::block_hash hash; - std::vector> voters; // pair - nano::uint128_t tally{ 0 }; + explicit entry (nano::block_hash const & hash); /** * Adds a vote into a list, checks for duplicates and updates timestamp if new one is greater @@ -67,11 +71,23 @@ class vote_cache final /** * Inserts votes stored in this entry into an election */ - std::size_t fill (std::shared_ptr election) const; + std::size_t fill (std::shared_ptr const & election) const; /* * Size of this entry */ std::size_t size () const; + + nano::block_hash hash () const; + nano::uint128_t tally () const; + nano::uint128_t final_tally () const; + std::vector voters () const; + + private: + nano::block_hash const hash_m; + std::vector voters_m; + + nano::uint128_t tally_m{ 0 }; + nano::uint128_t final_tally_m{ 0 }; }; private: @@ -147,7 +163,8 @@ class vote_cache final mi::indexed_by< mi::sequenced>, mi::hashed_unique, - mi::member>>>; + mi::const_mem_fun> + >>; // clang-format on ordered_cache cache; @@ -158,7 +175,8 @@ class vote_cache final mi::ordered_non_unique, mi::member>, mi::hashed_unique, - mi::member>>>; + mi::member> + >>; // clang-format on ordered_queue queue; diff --git a/nano/secure/common.cpp b/nano/secure/common.cpp index c26b32f893..09e37ea965 100644 --- a/nano/secure/common.cpp +++ b/nano/secure/common.cpp @@ -605,6 +605,11 @@ uint64_t nano::vote::packed_timestamp (uint64_t timestamp, uint8_t duration) con return (timestamp & timestamp_mask) | duration; } +bool nano::vote::is_final_timestamp (uint64_t timestamp) +{ + return timestamp == std::numeric_limits::max (); +} + nano::block_hash nano::iterate_vote_blocks_as_hash::operator() (nano::block_hash const & item) const { return item; diff --git a/nano/secure/common.hpp b/nano/secure/common.hpp index f1d52ff7cf..cd8cfe7802 100644 --- a/nano/secure/common.hpp +++ b/nano/secure/common.hpp @@ -276,11 +276,15 @@ class vote final uint64_t timestamp () const; uint8_t duration_bits () const; std::chrono::milliseconds duration () const; + static uint64_t constexpr timestamp_mask = { 0xffff'ffff'ffff'fff0ULL }; static nano::seconds_t constexpr timestamp_max = { 0xffff'ffff'ffff'fff0ULL }; static uint64_t constexpr timestamp_min = { 0x0000'0000'0000'0010ULL }; static uint8_t constexpr duration_max = { 0x0fu }; + /* Check if timestamp represents a final vote */ + static bool is_final_timestamp (uint64_t timestamp); + private: // Vote timestamp uint64_t timestamp_m; From 46adf29c5983e79cd54def76bd3b933b927e2531 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Tue, 19 Sep 2023 22:45:37 +0200 Subject: [PATCH 03/13] Rename `inactive_vote_cache` to `vote_cache` --- nano/core_test/active_transactions.cpp | 30 +++++++++++++------------- nano/node/active_transactions.cpp | 12 +++++------ nano/node/active_transactions.hpp | 2 +- nano/node/election.cpp | 2 +- nano/node/node.cpp | 10 ++++----- nano/node/node.hpp | 2 +- nano/node/process_live_dispatcher.cpp | 6 +++--- nano/node/process_live_dispatcher.hpp | 4 ++-- nano/node/scheduler/component.cpp | 2 +- nano/node/scheduler/hinted.cpp | 8 +++---- nano/node/scheduler/hinted.hpp | 2 +- nano/slow_test/vote_cache.cpp | 4 ++-- 12 files changed, 42 insertions(+), 42 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index d481c2c3de..a3212bf9fa 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -238,7 +238,7 @@ TEST (active_transactions, inactive_votes_cache) .build_shared (); auto vote (std::make_shared (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::vote::timestamp_max, nano::vote::duration_max, std::vector (1, send->hash ()))); node.vote_processor.vote (vote, std::make_shared (node, node)); - ASSERT_TIMELY (5s, node.inactive_vote_cache.cache_size () == 1); + ASSERT_TIMELY (5s, node.vote_cache.cache_size () == 1); node.process_active (send); node.block_processor.flush (); ASSERT_TIMELY (5s, node.ledger.block_confirmed (node.store.tx_begin_read (), send->hash ())); @@ -264,7 +264,7 @@ TEST (active_transactions, inactive_votes_cache_non_final) // Non-final vote auto vote = std::make_shared (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, 0, 0, std::vector (1, send->hash ())); node.vote_processor.vote (vote, std::make_shared (node, node)); - ASSERT_TIMELY (5s, node.inactive_vote_cache.cache_size () == 1); + ASSERT_TIMELY (5s, node.vote_cache.cache_size () == 1); node.process_active (send); std::shared_ptr election; @@ -301,7 +301,7 @@ TEST (active_transactions, inactive_votes_cache_fork) auto const vote = std::make_shared (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::vote::timestamp_max, nano::vote::duration_max, std::vector (1, send1->hash ())); node.vote_processor.vote (vote, std::make_shared (node, node)); - ASSERT_TIMELY (5s, node.inactive_vote_cache.cache_size () == 1); + ASSERT_TIMELY (5s, node.vote_cache.cache_size () == 1); node.process_active (send2); @@ -356,8 +356,8 @@ TEST (active_transactions, inactive_votes_cache_existing_vote) ASSERT_EQ (nano::vote::timestamp_min * 1, last_vote1.timestamp); // Attempt to change vote with inactive_votes_cache nano::unique_lock active_lock (node.active.mutex); - node.inactive_vote_cache.vote (send->hash (), vote1); - auto cache = node.inactive_vote_cache.find (send->hash ()); + node.vote_cache.vote (send->hash (), vote1); + auto cache = node.vote_cache.find (send->hash ()); ASSERT_TRUE (cache); ASSERT_EQ (1, cache->voters ().size ()); cache->fill (election); @@ -416,9 +416,9 @@ TEST (active_transactions, inactive_votes_cache_multiple_votes) auto vote2 (std::make_shared (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, 0, 0, std::vector (1, send1->hash ()))); node.vote_processor.vote (vote2, std::make_shared (node, node)); - ASSERT_TIMELY (5s, node.inactive_vote_cache.find (send1->hash ())); - ASSERT_TIMELY (5s, node.inactive_vote_cache.find (send1->hash ())->voters ().size () == 2); - ASSERT_EQ (1, node.inactive_vote_cache.cache_size ()); + ASSERT_TIMELY (5s, node.vote_cache.find (send1->hash ())); + ASSERT_TIMELY (5s, node.vote_cache.find (send1->hash ())->voters ().size () == 2); + ASSERT_EQ (1, node.vote_cache.cache_size ()); node.scheduler.priority.activate (nano::dev::genesis_key.pub, node.store.tx_begin_read ()); std::shared_ptr election; ASSERT_TIMELY (5s, election = node.active.election (send1->qualified_root ())); @@ -497,7 +497,7 @@ TEST (active_transactions, inactive_votes_cache_election_start) std::vector hashes{ open1->hash (), open2->hash (), send4->hash () }; auto vote1 (std::make_shared (key1.pub, key1.prv, 0, 0, hashes)); node.vote_processor.vote (vote1, std::make_shared (node, node)); - ASSERT_TIMELY (5s, node.inactive_vote_cache.cache_size () == 3); + ASSERT_TIMELY (5s, node.vote_cache.cache_size () == 3); ASSERT_TRUE (node.active.empty ()); ASSERT_EQ (1, node.ledger.cache.cemented_count); // 2 votes are required to start election (dev network) @@ -512,7 +512,7 @@ TEST (active_transactions, inactive_votes_cache_election_start) ASSERT_TIMELY (5s, 5 == node.ledger.cache.cemented_count); // A late block arrival also checks the inactive votes cache ASSERT_TRUE (node.active.empty ()); - auto send4_cache (node.inactive_vote_cache.find (send4->hash ())); + auto send4_cache (node.vote_cache.find (send4->hash ())); ASSERT_TRUE (send4_cache); ASSERT_EQ (3, send4_cache->voters ().size ()); node.process_active (send3); @@ -960,8 +960,8 @@ TEST (active_transactions, fork_replacement_tally) node1.vote_processor.vote (vote, std::make_shared (node1, node1)); node1.vote_processor.flush (); // ensure vote arrives before the block - ASSERT_TIMELY (5s, node1.inactive_vote_cache.find (send_last->hash ())); - ASSERT_TIMELY (5s, 1 == node1.inactive_vote_cache.find (send_last->hash ())->size ()); + ASSERT_TIMELY (5s, node1.vote_cache.find (send_last->hash ())); + ASSERT_TIMELY (5s, 1 == node1.vote_cache.find (send_last->hash ())->size ()); node1.network.publish_filter.clear (); node2.network.flood_block (send_last); ASSERT_TIMELY (5s, node1.stats.count (nano::stat::type::message, nano::stat::detail::publish, nano::stat::dir::in) > 1); @@ -1444,7 +1444,7 @@ TEST (active_transactions, limit_vote_hinted_elections) auto vote1 = nano::test::make_vote (rep1, { open0, open1 }); node.vote_processor.vote (vote1, nano::test::fake_channel (node)); // Ensure new inactive vote cache entries were created - ASSERT_TIMELY (5s, node.inactive_vote_cache.cache_size () == 2); + ASSERT_TIMELY (5s, node.vote_cache.cache_size () == 2); // And no elections are getting started yet ASSERT_ALWAYS (1s, node.active.empty ()); // And nothing got confirmed yet @@ -1517,7 +1517,7 @@ TEST (active_transactions, allow_limited_overflow) { // Non-final vote, so it stays in the AEC without getting confirmed auto vote = nano::test::make_vote (nano::dev::genesis_key, { block }); - node.inactive_vote_cache.vote (block->hash (), vote); + node.vote_cache.vote (block->hash (), vote); } // Ensure active elections overfill AEC only up to normal + hinted limit @@ -1555,7 +1555,7 @@ TEST (active_transactions, allow_limited_overflow_adapt) { // Non-final vote, so it stays in the AEC without getting confirmed auto vote = nano::test::make_vote (nano::dev::genesis_key, { block }); - node.inactive_vote_cache.vote (block->hash (), vote); + node.vote_cache.vote (block->hash (), vote); } // Ensure hinted election amount is bounded by hinted limit diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 9aab28cced..db8a831085 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -338,7 +338,7 @@ void nano::active_transactions::cleanup_election (nano::unique_lock auto erased (blocks.erase (hash)); (void)erased; debug_assert (erased == 1); - node.inactive_vote_cache.erase (hash); + node.vote_cache.erase (hash); } roots.get ().erase (roots.get ().find (election->qualified_root)); @@ -473,7 +473,7 @@ nano::election_insertion_result nano::active_transactions::insert_impl (nano::un count_by_behavior[result.election->behavior ()]++; lock_a.unlock (); - if (auto const cache = node.inactive_vote_cache.find (hash); cache) + if (auto const cache = node.vote_cache.find (hash); cache) { cache->fill (result.election); } @@ -535,7 +535,7 @@ nano::vote_code nano::active_transactions::vote (std::shared_ptr con // Process inactive votes outside of the critical section for (auto & hash : inactive) { - add_inactive_vote_cache (hash, vote_a); + add_vote_cache (hash, vote_a); } if (!process.empty ()) @@ -670,7 +670,7 @@ bool nano::active_transactions::publish (std::shared_ptr const & bl lock.lock (); blocks.emplace (block_a->hash (), election); lock.unlock (); - if (auto const cache = node.inactive_vote_cache.find (block_a->hash ()); cache) + if (auto const cache = node.vote_cache.find (block_a->hash ()); cache) { cache->fill (election); } @@ -727,11 +727,11 @@ boost::optional nano::active_transactions::confirm_b return status_type; } -void nano::active_transactions::add_inactive_vote_cache (nano::block_hash const & hash, std::shared_ptr const vote) +void nano::active_transactions::add_vote_cache (nano::block_hash const & hash, std::shared_ptr const vote) { if (node.ledger.weight (vote->account) > node.minimum_principal_weight ()) { - node.inactive_vote_cache.vote (hash, vote); + node.vote_cache.vote (hash, vote); } } diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index d62e8631fb..ff8988c498 100644 --- a/nano/node/active_transactions.hpp +++ b/nano/node/active_transactions.hpp @@ -196,7 +196,7 @@ class active_transactions final * Checks if vote passes minimum representative weight threshold and adds it to inactive vote cache * TODO: Should be moved to `vote_cache` class */ - void add_inactive_vote_cache (nano::block_hash const & hash, std::shared_ptr vote); + void add_vote_cache (nano::block_hash const & hash, std::shared_ptr vote); boost::optional election_status (nano::store::read_transaction const & transaction, std::shared_ptr const & block); void process_inactive_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr const & block); void process_active_confirmation (nano::store::read_transaction const & transaction, std::shared_ptr const & block, nano::election_status_type status); diff --git a/nano/node/election.cpp b/nano/node/election.cpp index bcfb576a5f..03e7e68cc9 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -570,7 +570,7 @@ bool nano::election::replace_by_weight (nano::unique_lock & lock_a, // Sort in ascending order std::sort (sorted.begin (), sorted.end (), [] (auto const & left, auto const & right) { return left.second < right.second; }); // Replace if lowest tally is below inactive cache new block weight - auto inactive_existing = node.inactive_vote_cache.find (hash_a); + auto inactive_existing = node.vote_cache.find (hash_a); auto inactive_tally = inactive_existing ? inactive_existing->tally () : 0; if (inactive_tally > 0 && sorted.size () < max_blocks) { diff --git a/nano/node/node.cpp b/nano/node/node.cpp index 0c764d9de4..b63a47b97b 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -191,7 +191,7 @@ nano::node::node (boost::asio::io_context & io_ctx_a, boost::filesystem::path co 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), - inactive_vote_cache{ nano::nodeconfig_to_vote_cache_config (config, flags) }, + vote_cache{ nano::nodeconfig_to_vote_cache_config (config, flags) }, 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), @@ -208,7 +208,7 @@ nano::node::node (boost::asio::io_context & io_ctx_a, boost::filesystem::path co block_broadcast{ network, block_arrival, !flags.disable_block_processor_republishing }, block_publisher{ active }, gap_tracker{ gap_cache }, - process_live_dispatcher{ ledger, scheduler.priority, inactive_vote_cache, websocket } + process_live_dispatcher{ ledger, scheduler.priority, vote_cache, websocket } { block_broadcast.connect (block_processor); block_publisher.connect (block_processor); @@ -218,7 +218,7 @@ nano::node::node (boost::asio::io_context & io_ctx_a, boost::filesystem::path co this->block_processor.add (info.block); }); - inactive_vote_cache.rep_weight_query = [this] (nano::account const & rep) { + vote_cache.rep_weight_query = [this] (nano::account const & rep) { return ledger.weight (rep); }; @@ -579,8 +579,8 @@ std::unique_ptr nano::collect_container_info (no composite->add_component (collect_container_info (node.confirmation_height_processor, "confirmation_height_processor")); composite->add_component (collect_container_info (node.distributed_work, "distributed_work")); composite->add_component (collect_container_info (node.aggregator, "request_aggregator")); - composite->add_component (node.scheduler.collect_container_info ("scheduler")); - composite->add_component (node.inactive_vote_cache.collect_container_info ("inactive_vote_cache")); + composite->add_component (node.scheduler.collect_container_info ("election_scheduler")); + composite->add_component (node.vote_cache.collect_container_info ("vote_cache")); composite->add_component (collect_container_info (node.generator, "vote_generator")); composite->add_component (collect_container_info (node.final_generator, "vote_generator_final")); composite->add_component (node.ascendboot.collect_container_info ("bootstrap_ascending")); diff --git a/nano/node/node.hpp b/nano/node/node.hpp index 870562dc55..092861c4e2 100644 --- a/nano/node/node.hpp +++ b/nano/node/node.hpp @@ -180,7 +180,7 @@ class node final : public std::enable_shared_from_this nano::block_uniquer block_uniquer; nano::vote_uniquer vote_uniquer; nano::confirmation_height_processor confirmation_height_processor; - nano::vote_cache inactive_vote_cache; + nano::vote_cache vote_cache; nano::vote_generator generator; nano::vote_generator final_generator; nano::active_transactions active; diff --git a/nano/node/process_live_dispatcher.cpp b/nano/node/process_live_dispatcher.cpp index fa23af6370..97019f2163 100644 --- a/nano/node/process_live_dispatcher.cpp +++ b/nano/node/process_live_dispatcher.cpp @@ -8,10 +8,10 @@ #include #include -nano::process_live_dispatcher::process_live_dispatcher (nano::ledger & ledger, nano::scheduler::priority & scheduler, nano::vote_cache & inactive_vote_cache, nano::websocket_server & websocket) : +nano::process_live_dispatcher::process_live_dispatcher (nano::ledger & ledger, nano::scheduler::priority & scheduler, nano::vote_cache & vote_cache, nano::websocket_server & websocket) : ledger{ ledger }, scheduler{ scheduler }, - inactive_vote_cache{ inactive_vote_cache }, + vote_cache{ vote_cache }, websocket{ websocket } { } @@ -50,7 +50,7 @@ void nano::process_live_dispatcher::process_live (nano::block const & block, sto } // Notify inactive vote cache about a new live block - inactive_vote_cache.trigger (block.hash ()); + vote_cache.trigger (block.hash ()); if (websocket.server && websocket.server->any_subscriber (nano::websocket::topic::new_unconfirmed_block)) { diff --git a/nano/node/process_live_dispatcher.hpp b/nano/node/process_live_dispatcher.hpp index c07525a0ff..ec30d94913 100644 --- a/nano/node/process_live_dispatcher.hpp +++ b/nano/node/process_live_dispatcher.hpp @@ -23,7 +23,7 @@ namespace scheduler class process_live_dispatcher { public: - process_live_dispatcher (nano::ledger & ledger, nano::scheduler::priority & scheduler, nano::vote_cache & inactive_vote_cache, nano::websocket_server & websocket); + process_live_dispatcher (nano::ledger &, nano::scheduler::priority &, nano::vote_cache &, nano::websocket_server &); void connect (nano::block_processor & block_processor); private: @@ -33,7 +33,7 @@ class process_live_dispatcher nano::ledger & ledger; nano::scheduler::priority & scheduler; - nano::vote_cache & inactive_vote_cache; + nano::vote_cache & vote_cache; nano::websocket_server & websocket; }; } diff --git a/nano/node/scheduler/component.cpp b/nano/node/scheduler/component.cpp index f1462f3893..9a4f3de4e1 100644 --- a/nano/node/scheduler/component.cpp +++ b/nano/node/scheduler/component.cpp @@ -6,7 +6,7 @@ #include nano::scheduler::component::component (nano::node & node) : - hinted_impl{ std::make_unique (nano::scheduler::hinted::config{ node.config }, node, node.inactive_vote_cache, node.active, node.online_reps, node.stats) }, + hinted_impl{ std::make_unique (nano::scheduler::hinted::config{ node.config }, node, node.vote_cache, node.active, node.online_reps, node.stats) }, manual_impl{ std::make_unique (node) }, optimistic_impl{ std::make_unique (node.config.optimistic_scheduler, node, node.ledger, node.active, node.network_params.network, node.stats) }, priority_impl{ std::make_unique (node, node.stats) }, diff --git a/nano/node/scheduler/hinted.cpp b/nano/node/scheduler/hinted.cpp index 59321a79c3..4da2ddcffa 100644 --- a/nano/node/scheduler/hinted.cpp +++ b/nano/node/scheduler/hinted.cpp @@ -7,10 +7,10 @@ nano::scheduler::hinted::config::config (nano::node_config const & config) : { } -nano::scheduler::hinted::hinted (config const & config_a, nano::node & node_a, nano::vote_cache & inactive_vote_cache_a, nano::active_transactions & active_a, nano::online_reps & online_reps_a, nano::stats & stats_a) : +nano::scheduler::hinted::hinted (config const & config_a, nano::node & node_a, nano::vote_cache & vote_cache_a, nano::active_transactions & active_a, nano::online_reps & online_reps_a, nano::stats & stats_a) : config_m{ config_a }, node{ node_a }, - inactive_vote_cache{ inactive_vote_cache_a }, + vote_cache{ vote_cache_a }, active{ active_a }, online_reps{ online_reps_a }, stats{ stats_a } @@ -54,7 +54,7 @@ bool nano::scheduler::hinted::predicate (nano::uint128_t const & minimum_tally) if (active.vacancy (nano::election_behavior::hinted) > 0) { // Check if there is any vote cache entry surpassing our minimum vote tally threshold - if (inactive_vote_cache.peek (minimum_tally)) + if (vote_cache.peek (minimum_tally)) { return true; } @@ -64,7 +64,7 @@ bool nano::scheduler::hinted::predicate (nano::uint128_t const & minimum_tally) bool nano::scheduler::hinted::run_one (nano::uint128_t const & minimum_tally) { - if (auto top = inactive_vote_cache.pop (minimum_tally); top) + if (auto top = vote_cache.pop (minimum_tally); top) { const auto hash = top->hash (); // Hash of block we want to hint diff --git a/nano/node/scheduler/hinted.hpp b/nano/node/scheduler/hinted.hpp index ba344095d2..d81f3d119b 100644 --- a/nano/node/scheduler/hinted.hpp +++ b/nano/node/scheduler/hinted.hpp @@ -52,7 +52,7 @@ class hinted final private: // Dependencies nano::node & node; - nano::vote_cache & inactive_vote_cache; + nano::vote_cache & vote_cache; nano::active_transactions & active; nano::online_reps & online_reps; nano::stats & stats; diff --git a/nano/slow_test/vote_cache.cpp b/nano/slow_test/vote_cache.cpp index f0ee8035ed..b4f5f373b3 100644 --- a/nano/slow_test/vote_cache.cpp +++ b/nano/slow_test/vote_cache.cpp @@ -184,7 +184,7 @@ TEST (vote_cache, perf_singlethreaded) ASSERT_EQ (node.stats.count (nano::stat::type::vote_cache, nano::stat::detail::vote_processed, nano::stat::dir::in), vote_count * single_vote_size * single_vote_reps); // Ensure vote cache size is at max capacity - ASSERT_EQ (node.inactive_vote_cache.cache_size (), flags.inactive_votes_cache_size); + ASSERT_EQ (node.vote_cache.cache_size (), flags.inactive_votes_cache_size); } TEST (vote_cache, perf_multithreaded) @@ -247,5 +247,5 @@ TEST (vote_cache, perf_multithreaded) std::cout << "total votes processed: " << node.stats.count (nano::stat::type::vote_cache, nano::stat::detail::vote_processed, nano::stat::dir::in) << std::endl; // Ensure vote cache size is at max capacity - ASSERT_EQ (node.inactive_vote_cache.cache_size (), flags.inactive_votes_cache_size); + ASSERT_EQ (node.vote_cache.cache_size (), flags.inactive_votes_cache_size); } From 92ec038725ca9c4085e2be70ddeaf0f10db15916 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 2 Oct 2023 18:33:26 +0200 Subject: [PATCH 04/13] Remove friend declarations from schedulers This is bad --- nano/node/scheduler/hinted.hpp | 7 +++---- nano/node/scheduler/optimistic.hpp | 7 ++++--- nano/node/scheduler/priority.hpp | 10 +++++----- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/nano/node/scheduler/hinted.hpp b/nano/node/scheduler/hinted.hpp index d81f3d119b..2d95e2bba1 100644 --- a/nano/node/scheduler/hinted.hpp +++ b/nano/node/scheduler/hinted.hpp @@ -22,10 +22,6 @@ namespace nano::scheduler */ class hinted final { - friend class component; - void start (); - void stop (); - public: // Config struct config final { @@ -38,6 +34,9 @@ class hinted final hinted (config const &, nano::node &, nano::vote_cache &, nano::active_transactions &, nano::online_reps &, nano::stats &); ~hinted (); + void start (); + void stop (); + /* * Notify about changes in AEC vacancy */ diff --git a/nano/node/scheduler/optimistic.hpp b/nano/node/scheduler/optimistic.hpp index 8794789e79..627b74d621 100644 --- a/nano/node/scheduler/optimistic.hpp +++ b/nano/node/scheduler/optimistic.hpp @@ -44,17 +44,18 @@ class optimistic_config final /** Maximum number of candidates stored in memory */ std::size_t max_size{ 1024 * 64 }; }; + class optimistic final { - friend class component; - void start (); - void stop (); struct entry; public: optimistic (optimistic_config const &, nano::node &, nano::ledger &, nano::active_transactions &, nano::network_constants const & network_constants, nano::stats &); ~optimistic (); + void start (); + void stop (); + /** * Called from backlog population to process accounts with unconfirmed blocks */ diff --git a/nano/node/scheduler/priority.hpp b/nano/node/scheduler/priority.hpp index 30bf33ad31..e232b3d971 100644 --- a/nano/node/scheduler/priority.hpp +++ b/nano/node/scheduler/priority.hpp @@ -23,15 +23,13 @@ namespace nano::scheduler class buckets; class priority final { - friend class component; - void start (); - void stop (); - std::unique_ptr collect_container_info (std::string const & name); - public: priority (nano::node &, nano::stats &); ~priority (); + void start (); + void stop (); + /** * Activates the first unconfirmed block of \p account_a * @return true if account was activated @@ -41,6 +39,8 @@ class priority final std::size_t size () const; bool empty () const; + std::unique_ptr collect_container_info (std::string const & name); + private: // Dependencies nano::node & node; nano::stats & stats; From 8edbf6db364afc35183bc77b067bbc39d1c38179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 18 Sep 2023 14:55:06 +0200 Subject: [PATCH 05/13] Activate dependents in hinted scheduler --- nano/lib/stats_enums.hpp | 5 + nano/node/active_transactions.cpp | 1 - nano/node/node.cpp | 7 +- nano/node/node.hpp | 1 + nano/node/scheduler/hinted.cpp | 158 +++++++++++++++++++++--------- nano/node/scheduler/hinted.hpp | 38 ++++++- nano/node/vote_cache.cpp | 56 +++++++++-- nano/node/vote_cache.hpp | 44 +++++---- 8 files changed, 234 insertions(+), 76 deletions(-) diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index 778c9214ef..79ef835802 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -227,6 +227,11 @@ enum class detail : uint8_t // hinting missing_block, + dependent_unconfirmed, + already_confirmed, + activate, + activate_immediate, + dependent_activated, // bootstrap server response, diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index db8a831085..5079cb2cd8 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -338,7 +338,6 @@ void nano::active_transactions::cleanup_election (nano::unique_lock auto erased (blocks.erase (hash)); (void)erased; debug_assert (erased == 1); - node.vote_cache.erase (hash); } roots.get ().erase (roots.get ().find (election->qualified_root)); diff --git a/nano/node/node.cpp b/nano/node/node.cpp index b63a47b97b..e9275ef6aa 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -1273,9 +1273,14 @@ bool nano::node::block_confirmed (nano::block_hash const & hash_a) return ledger.block_confirmed (transaction, hash_a); } +bool nano::node::block_confirmed_or_being_confirmed (nano::store::transaction const & transaction, nano::block_hash const & hash_a) +{ + return confirmation_height_processor.is_processing_block (hash_a) || ledger.block_confirmed (transaction, hash_a); +} + bool nano::node::block_confirmed_or_being_confirmed (nano::block_hash const & hash_a) { - return confirmation_height_processor.is_processing_block (hash_a) || ledger.block_confirmed (store.tx_begin_read (), hash_a); + return block_confirmed_or_being_confirmed (store.tx_begin_read (), hash_a); } void nano::node::ongoing_online_weight_calculation_queue () diff --git a/nano/node/node.hpp b/nano/node/node.hpp index 092861c4e2..14b773c07e 100644 --- a/nano/node/node.hpp +++ b/nano/node/node.hpp @@ -124,6 +124,7 @@ class node final : public std::enable_shared_from_this void add_initial_peers (); void start_election (std::shared_ptr const & block); bool block_confirmed (nano::block_hash const &); + bool block_confirmed_or_being_confirmed (nano::store::transaction const &, nano::block_hash const &); bool block_confirmed_or_being_confirmed (nano::block_hash const &); void do_rpc_callback (boost::asio::ip::tcp::resolver::iterator i_a, std::string const &, uint16_t, std::shared_ptr const &, std::shared_ptr const &, std::shared_ptr const &); void ongoing_online_weight_calculation (); diff --git a/nano/node/scheduler/hinted.cpp b/nano/node/scheduler/hinted.cpp index 4da2ddcffa..68b6368dc0 100644 --- a/nano/node/scheduler/hinted.cpp +++ b/nano/node/scheduler/hinted.cpp @@ -3,7 +3,7 @@ #include nano::scheduler::hinted::config::config (nano::node_config const & config) : - vote_cache_check_interval_ms{ config.network_params.network.is_dev_network () ? 100u : 1000u } + vote_cache_check_interval_ms{ config.network_params.network.is_dev_network () ? 100u : 5000u } { } @@ -48,51 +48,95 @@ void nano::scheduler::hinted::notify () condition.notify_all (); } -bool nano::scheduler::hinted::predicate (nano::uint128_t const & minimum_tally) const +bool nano::scheduler::hinted::predicate () const { // Check if there is space inside AEC for a new hinted election - if (active.vacancy (nano::election_behavior::hinted) > 0) - { - // Check if there is any vote cache entry surpassing our minimum vote tally threshold - if (vote_cache.peek (minimum_tally)) - { - return true; - } - } - return false; + return active.vacancy (nano::election_behavior::hinted) > 0; } -bool nano::scheduler::hinted::run_one (nano::uint128_t const & minimum_tally) +void nano::scheduler::hinted::activate (const nano::store::transaction & transaction, const nano::block_hash & hash, bool check_dependents) { - if (auto top = vote_cache.pop (minimum_tally); top) + std::stack stack; + stack.push (hash); + + while (!stack.empty ()) { - const auto hash = top->hash (); // Hash of block we want to hint + const nano::block_hash current_hash = stack.top (); + stack.pop (); // Check if block exists - auto block = node.block (hash); - if (block != nullptr) + if (auto block = node.store.block.get (transaction, current_hash); block) { // Ensure block is not already confirmed - if (!node.block_confirmed_or_being_confirmed (hash)) + if (node.block_confirmed_or_being_confirmed (transaction, current_hash)) { - // Try to insert it into AEC as hinted election - // We check for AEC vacancy inside our predicate - auto result = node.active.insert (block, nano::election_behavior::hinted); - - stats.inc (nano::stat::type::hinting, result.inserted ? nano::stat::detail::insert : nano::stat::detail::insert_failed); + stats.inc (nano::stat::type::hinting, nano::stat::detail::already_confirmed); + continue; // Move on to the next item in the stack + } - return result.inserted; // Return whether block was inserted + if (check_dependents) + { + // Perform a depth-first search of the dependency graph + if (!node.ledger.dependents_confirmed (transaction, *block)) + { + stats.inc (nano::stat::type::hinting, nano::stat::detail::dependent_unconfirmed); + auto dependents = node.ledger.dependent_blocks (transaction, *block); + for (const auto & dependent_hash : dependents) + { + if (!dependent_hash.is_zero ()) + { + stack.push (dependent_hash); // Add dependent block to the stack + } + } + continue; // Move on to the next item in the stack + } } + + // Try to insert it into AEC as hinted election + auto result = node.active.insert (block, nano::election_behavior::hinted); + stats.inc (nano::stat::type::hinting, result.inserted ? nano::stat::detail::insert : nano::stat::detail::insert_failed); } else { - // Missing block in ledger to start an election - node.bootstrap_block (hash); - stats.inc (nano::stat::type::hinting, nano::stat::detail::missing_block); + node.bootstrap_block (current_hash); + } + } +} + +void nano::scheduler::hinted::run_iterative () +{ + const auto minimum_tally = tally_threshold (); + const auto minimum_final_tally = final_tally_threshold (); + + auto transaction = node.store.tx_begin_read (); + + for (auto const & entry : vote_cache.top (minimum_tally)) + { + if (!predicate ()) + { + return; + } + + if (cooldown (entry.hash)) + { + continue; + } + + // Check dependents only if cached tally is lower than quorum + if (entry.final_tally < minimum_final_tally) + { + // Ensure all dependent blocks are already confirmed before activating + stats.inc (nano::stat::type::hinting, nano::stat::detail::activate); + activate (transaction, entry.hash, /* activate dependents */ true); + } + else + { + // Blocks with a vote tally higher than quorum, can be activated and confirmed immediately + stats.inc (nano::stat::type::hinting, nano::stat::detail::activate_immediate); + activate (transaction, entry.hash, false); } } - return false; } void nano::scheduler::hinted::run () @@ -100,35 +144,61 @@ void nano::scheduler::hinted::run () nano::unique_lock lock{ mutex }; while (!stopped) { - // It is possible that if we are waiting long enough this tally becomes outdated due to changes in trended online weight - // However this is only used here for hinting, election does independent tally calculation, so there is no need to ensure it's always up-to-date - const auto minimum_tally = tally_threshold (); + stats.inc (nano::stat::type::hinting, nano::stat::detail::loop); - // Periodically wakeup for condition checking - // We are not notified every time new vote arrives in inactive vote cache as that happens too often - condition.wait_for (lock, std::chrono::milliseconds (config_m.vote_cache_check_interval_ms), [this, minimum_tally] () { - return stopped || predicate (minimum_tally); - }); + condition.wait_for (lock, config.check_interval); debug_assert ((std::this_thread::yield (), true)); // Introduce some random delay in debug builds if (!stopped) { - // We don't need the lock when running main loop - lock.unlock (); - - if (predicate (minimum_tally)) + if (predicate ()) { - run_one (minimum_tally); + run_iterative (); } - - lock.lock (); } } } nano::uint128_t nano::scheduler::hinted::tally_threshold () const { - auto min_tally = (online_reps.trended () / 100) * node.config.election_hint_weight_percent; - return min_tally; + // auto min_tally = (online_reps.trended () / 100) * node.config.election_hint_weight_percent; + // return min_tally; + + return 0; +} + +nano::uint128_t nano::scheduler::hinted::final_tally_threshold () const +{ + auto quorum = online_reps.delta (); + return quorum; } + +bool nano::scheduler::hinted::cooldown (const nano::block_hash & hash) +{ + auto const now = std::chrono::steady_clock::now (); + auto const cooldown = std::chrono::seconds{ 15 }; + + // Check if the hash is still in the cooldown period using the hashed index + auto const & hashed_index = cooldowns_m.get (); + if (auto it = hashed_index.find (hash); it != hashed_index.end ()) + { + if (it->timeout > now) + { + return true; // Needs cooldown + } + cooldowns_m.erase (it); // Entry is outdated, so remove it + } + + // Insert the new entry + cooldowns_m.insert ({ hash, now + cooldown }); + + // Trim old entries + auto & seq_index = cooldowns_m.get (); + while (!seq_index.empty () && seq_index.begin ()->timeout <= now) + { + seq_index.erase (seq_index.begin ()); + } + + return false; // No need to cooldown +} \ No newline at end of file diff --git a/nano/node/scheduler/hinted.hpp b/nano/node/scheduler/hinted.hpp index 2d95e2bba1..095ea8064b 100644 --- a/nano/node/scheduler/hinted.hpp +++ b/nano/node/scheduler/hinted.hpp @@ -4,8 +4,14 @@ #include #include +#include +#include +#include + +#include #include #include +#include namespace nano { @@ -43,11 +49,13 @@ class hinted final void notify (); private: - bool predicate (nano::uint128_t const & minimum_tally) const; + bool predicate () const; void run (); - bool run_one (nano::uint128_t const & minimum_tally); + void run_iterative (); + void activate (nano::store::transaction const &, nano::block_hash const & hash, bool check_dependents); nano::uint128_t tally_threshold () const; + nano::uint128_t final_tally_threshold () const; private: // Dependencies nano::node & node; @@ -63,5 +71,31 @@ class hinted final nano::condition_variable condition; mutable nano::mutex mutex; std::thread thread; + +private: + bool cooldown (nano::block_hash const & hash); + + struct cooldown_entry + { + nano::block_hash hash; + std::chrono::steady_clock::time_point timeout; + }; + + // clang-format off + class tag_hash {}; + class tag_timeout {}; + // clang-format on + + // clang-format off + using ordered_cooldowns = boost::multi_index_container, + mi::member>, + mi::ordered_non_unique, + mi::member> + >>; + // clang-format on + + ordered_cooldowns cooldowns_m; }; } diff --git a/nano/node/vote_cache.cpp b/nano/node/vote_cache.cpp index 785e179e3f..890821b6c0 100644 --- a/nano/node/vote_cache.cpp +++ b/nano/node/vote_cache.cpp @@ -117,17 +117,22 @@ void nano::vote_cache::vote_impl (const nano::block_hash & hash, const nano::acc auto & cache_by_hash = cache.get (); if (auto existing = cache_by_hash.find (hash); existing != cache_by_hash.end ()) { - bool success = cache_by_hash.modify (existing, [&representative, ×tamp, &rep_weight] (entry & ent) { - ent.vote (representative, timestamp, rep_weight); + bool modified = false; + bool success = cache_by_hash.modify (existing, [&representative, ×tamp, &rep_weight, &modified] (entry & ent) { + modified = ent.vote (representative, timestamp, rep_weight); }); release_assert (success); // Ensure iterator `existing` is valid - auto & queue_by_hash = queue.get (); - if (auto queue_existing = queue_by_hash.find (hash); queue_existing != queue_by_hash.end ()) + if (modified) { - queue_by_hash.modify (queue_existing, [&existing] (queue_entry & ent) { - ent.tally = existing->tally (); - }); + auto & queue_by_hash = queue.get (); + if (auto queue_existing = queue_by_hash.find (hash); queue_existing != queue_by_hash.end ()) + { + queue_by_hash.modify (queue_existing, [&existing] (queue_entry & ent) { + ent.tally = existing->tally (); + ent.final_tally = existing->final_tally (); + }); + } } } else @@ -143,7 +148,7 @@ void nano::vote_cache::vote_impl (const nano::block_hash & hash, const nano::acc { queue_by_hash.erase (queue_existing); } - queue_by_hash.insert ({ hash, cache_entry.tally () }); + queue_by_hash.insert ({ hash, cache_entry.tally (), cache_entry.final_tally () }); trim_overflow_locked (); } @@ -243,13 +248,13 @@ void nano::vote_cache::trigger (const nano::block_hash & hash) { nano::lock_guard lock{ mutex }; - auto & queue_by_hash = queue.get (); // Only reinsert to queue if it is not already in queue and there are votes in passive cache + auto & queue_by_hash = queue.get (); if (auto existing_queue = queue_by_hash.find (hash); existing_queue == queue_by_hash.end ()) { if (auto maybe_cache_entry = find_locked (hash); maybe_cache_entry) { - queue_by_hash.insert ({ hash, maybe_cache_entry->tally () }); + queue_by_hash.insert ({ hash, maybe_cache_entry->tally (), maybe_cache_entry->final_tally () }); trim_overflow_locked (); } @@ -283,6 +288,37 @@ void nano::vote_cache::trim_overflow_locked () } } +std::vector nano::vote_cache::top (const nano::uint128_t & min_tally) const +{ + std::vector results; + { + nano::lock_guard lock{ mutex }; + + for (auto & entry : cache.get ()) + { + if (entry.tally () < min_tally) + { + break; + } + results.push_back ({ entry.hash (), entry.tally (), entry.final_tally () }); + } + } + + // Sort by final tally then by normal tally, descending + std::sort (results.begin (), results.end (), [] (auto const & a, auto const & b) { + if (a.final_tally == b.final_tally) + { + return a.tally > b.tally; + } + else + { + return a.final_tally > b.final_tally; + } + }); + + return results; +} + std::unique_ptr nano::vote_cache::collect_container_info (const std::string & name) { auto composite = std::make_unique (name); diff --git a/nano/node/vote_cache.hpp b/nano/node/vote_cache.hpp index 48bfd2fd53..d88998fd2c 100644 --- a/nano/node/vote_cache.hpp +++ b/nano/node/vote_cache.hpp @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -25,17 +26,6 @@ class active_transactions; class election; class vote; -/** - * A container holding votes that do not match any active or recently finished elections. - * It keeps track of votes in two internal structures: cache and queue - * - * Cache: Stores votes associated with a particular block hash with a bounded maximum number of votes per hash. - * When cache size exceeds `max_size` oldest entries are evicted first. - * - * Queue: Keeps track of block hashes ordered by total cached vote tally. - * When inserting a new vote into cache, the queue is atomically updated. - * When queue size exceeds `max_size` oldest entries are evicted first. - */ class vote_cache final { public: @@ -47,7 +37,7 @@ class vote_cache final public: /** - * Class that stores votes associated with a single block hash + * Stores votes associated with a single block hash */ class entry final { @@ -72,11 +62,8 @@ class vote_cache final * Inserts votes stored in this entry into an election */ std::size_t fill (std::shared_ptr const & election) const; - /* - * Size of this entry - */ - std::size_t size () const; + std::size_t size () const; nano::block_hash hash () const; nano::uint128_t tally () const; nano::uint128_t final_tally () const; @@ -96,6 +83,7 @@ class vote_cache final public: nano::block_hash hash{ 0 }; nano::uint128_t tally{ 0 }; + nano::uint128_t final_tally{ 0 }; }; public: @@ -136,6 +124,19 @@ class vote_cache final bool cache_empty () const; bool queue_empty () const; +public: + struct top_entry + { + nano::block_hash hash; + nano::uint128_t tally; + nano::uint128_t final_tally; + }; + + /** + * Returns blocks with highest observed tally, greater than `min_tally` + */ + std::vector top (nano::uint128_t const & min_tally) const; + public: // Container info std::unique_ptr collect_container_info (std::string const & name); @@ -154,8 +155,9 @@ class vote_cache final // clang-format off class tag_sequenced {}; - class tag_tally {}; class tag_hash {}; + class tag_tally {}; + class tag_final_tally {}; // clang-format on // clang-format off @@ -163,7 +165,11 @@ class vote_cache final mi::indexed_by< mi::sequenced>, mi::hashed_unique, - mi::const_mem_fun> + mi::const_mem_fun>, + mi::ordered_non_unique, + mi::const_mem_fun, std::greater<>>, // DESC + mi::ordered_non_unique, + mi::const_mem_fun, std::greater<>> // DESC >>; // clang-format on ordered_cache cache; @@ -174,6 +180,8 @@ class vote_cache final mi::sequenced>, mi::ordered_non_unique, mi::member>, + mi::ordered_non_unique, + mi::member>, mi::hashed_unique, mi::member> >>; From 77c30b79ace41e89ede82dc6ab8d67bbc1b69169 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Fri, 22 Sep 2023 01:36:48 +0200 Subject: [PATCH 06/13] Dedicated config for hinted scheduler --- nano/core_test/active_transactions.cpp | 4 +- nano/core_test/toml.cpp | 46 ++++++------------- nano/node/nodeconfig.cpp | 16 +++---- nano/node/nodeconfig.hpp | 3 +- nano/node/scheduler/component.cpp | 2 +- nano/node/scheduler/hinted.cpp | 62 +++++++++++++++++++++----- nano/node/scheduler/hinted.hpp | 29 +++++++----- 7 files changed, 95 insertions(+), 67 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index a3212bf9fa..a8cdd5cdf0 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -438,7 +438,7 @@ TEST (active_transactions, inactive_votes_cache_election_start) nano::send_block_builder send_block_builder; nano::state_block_builder state_block_builder; // Enough weight to trigger election hinting but not enough to confirm block on its own - auto amount = ((node.online_reps.trended () / 100) * node.config.election_hint_weight_percent) / 2 + 1000 * nano::Gxrb_ratio; + auto amount = ((node.online_reps.trended () / 100) * node.config.hinted_scheduler.hinting_threshold_percent) / 2 + 1000 * nano::Gxrb_ratio; auto send1 = send_block_builder.make_block () .previous (latest) .destination (key1.pub) @@ -1427,7 +1427,7 @@ TEST (active_transactions, limit_vote_hinted_elections) // Setup representatives // Enough weight to trigger election hinting but not enough to confirm block on its own - const auto amount = ((node.online_reps.trended () / 100) * node.config.election_hint_weight_percent) + 1000 * nano::Gxrb_ratio; + const auto amount = ((node.online_reps.trended () / 100) * node.config.hinted_scheduler.hinting_threshold_percent) + 1000 * nano::Gxrb_ratio; nano::keypair rep1 = nano::test::setup_rep (system, node, amount / 2); nano::keypair rep2 = nano::test::setup_rep (system, node, amount / 2); diff --git a/nano/core_test/toml.cpp b/nano/core_test/toml.cpp index b8098dd6ea..1c3a88b01a 100644 --- a/nano/core_test/toml.cpp +++ b/nano/core_test/toml.cpp @@ -176,7 +176,6 @@ TEST (toml, daemon_config_deserialize_defaults) ASSERT_EQ (conf.node.secondary_work_peers, defaults.node.secondary_work_peers); ASSERT_EQ (conf.node.online_weight_minimum, defaults.node.online_weight_minimum); ASSERT_EQ (conf.node.rep_crawler_weight_minimum, defaults.node.rep_crawler_weight_minimum); - ASSERT_EQ (conf.node.election_hint_weight_percent, defaults.node.election_hint_weight_percent); ASSERT_EQ (conf.node.password_fanout, defaults.node.password_fanout); ASSERT_EQ (conf.node.peering_port, defaults.node.peering_port); ASSERT_EQ (conf.node.pow_sleep_interval, defaults.node.pow_sleep_interval); @@ -272,6 +271,10 @@ TEST (toml, daemon_config_deserialize_defaults) ASSERT_EQ (conf.node.optimistic_scheduler.enabled, defaults.node.optimistic_scheduler.enabled); ASSERT_EQ (conf.node.optimistic_scheduler.gap_threshold, defaults.node.optimistic_scheduler.gap_threshold); ASSERT_EQ (conf.node.optimistic_scheduler.max_size, defaults.node.optimistic_scheduler.max_size); + + 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 ()); } TEST (toml, optional_child) @@ -425,7 +428,6 @@ TEST (toml, daemon_config_deserialize_no_defaults) background_threads = 999 online_weight_minimum = "999" rep_crawler_weight_minimum = "999" - election_hint_weight_percent = 19 password_fanout = 999 peering_port = 999 pow_sleep_interval= 999 @@ -534,6 +536,11 @@ TEST (toml, daemon_config_deserialize_no_defaults) gap_threshold = 999 max_size = 999 + [node.hinted_scheduler] + hinting_threshold = 99 + check_interval = 999 + block_cooldown = 999 + [node.rocksdb] enable = true memory_multiplier = 3 @@ -606,7 +613,6 @@ TEST (toml, daemon_config_deserialize_no_defaults) ASSERT_NE (conf.node.max_pruning_depth, defaults.node.max_pruning_depth); ASSERT_NE (conf.node.online_weight_minimum, defaults.node.online_weight_minimum); ASSERT_NE (conf.node.rep_crawler_weight_minimum, defaults.node.rep_crawler_weight_minimum); - ASSERT_NE (conf.node.election_hint_weight_percent, defaults.node.election_hint_weight_percent); ASSERT_NE (conf.node.password_fanout, defaults.node.password_fanout); ASSERT_NE (conf.node.peering_port, defaults.node.peering_port); ASSERT_NE (conf.node.pow_sleep_interval, defaults.node.pow_sleep_interval); @@ -703,6 +709,10 @@ TEST (toml, daemon_config_deserialize_no_defaults) ASSERT_NE (conf.node.optimistic_scheduler.enabled, defaults.node.optimistic_scheduler.enabled); ASSERT_NE (conf.node.optimistic_scheduler.gap_threshold, defaults.node.optimistic_scheduler.gap_threshold); ASSERT_NE (conf.node.optimistic_scheduler.max_size, defaults.node.optimistic_scheduler.max_size); + + 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 ()); } /** There should be no required values **/ @@ -835,36 +845,6 @@ TEST (toml, daemon_config_deserialize_errors) ASSERT_EQ (conf.node.frontiers_confirmation, nano::frontiers_confirmation_mode::invalid); } - { - std::stringstream ss; - ss << R"toml( - [node] - election_hint_weight_percent = 4 - )toml"; - - nano::tomlconfig toml; - toml.read (ss); - nano::daemon_config conf; - conf.deserialize_toml (toml); - - ASSERT_EQ (toml.get_error ().get_message (), "election_hint_weight_percent must be a number between 5 and 50"); - } - - { - std::stringstream ss; - ss << R"toml( - [node] - election_hint_weight_percent = 51 - )toml"; - - nano::tomlconfig toml; - toml.read (ss); - nano::daemon_config conf; - conf.deserialize_toml (toml); - - ASSERT_EQ (toml.get_error ().get_message (), "election_hint_weight_percent must be a number between 5 and 50"); - } - { std::stringstream ss; ss << R"toml( diff --git a/nano/node/nodeconfig.cpp b/nano/node/nodeconfig.cpp index 529d6fa14e..581914dc8a 100644 --- a/nano/node/nodeconfig.cpp +++ b/nano/node/nodeconfig.cpp @@ -28,6 +28,7 @@ nano::node_config::node_config (nano::network_params & network_params) : nano::node_config::node_config (const std::optional & peering_port_a, nano::logging const & logging_a, nano::network_params & network_params) : network_params{ network_params }, peering_port{ peering_port_a }, + hinted_scheduler{ network_params.network }, logging{ logging_a }, websocket_config{ network_params.network }, ipc_config{ network_params.network }, @@ -89,7 +90,6 @@ nano::error nano::node_config::serialize_toml (nano::tomlconfig & toml) const toml.put ("bootstrap_fraction_numerator", bootstrap_fraction_numerator, "Change bootstrap threshold (online stake / 256 * bootstrap_fraction_numerator).\ntype:uint32"); toml.put ("receive_minimum", receive_minimum.to_string_dec (), "Minimum receive amount. Only affects node wallets. A large amount is recommended to avoid automatic work generation for tiny transactions.\ntype:string,amount,raw"); toml.put ("online_weight_minimum", online_weight_minimum.to_string_dec (), "When calculating online weight, the node is forced to assume at least this much voting weight is online, thus setting a floor for voting weight to confirm transactions at online_weight_minimum * \"quorum delta\".\ntype:string,amount,raw"); - toml.put ("election_hint_weight_percent", election_hint_weight_percent, "Percentage of online weight to hint at starting an election. Defaults to 10.\ntype:uint32,[5,50]"); toml.put ("password_fanout", password_fanout, "Password fanout factor.\ntype:uint64"); toml.put ("io_threads", io_threads, "Number of threads dedicated to I/O operations. Defaults to the number of CPU threads, and at least 4.\ntype:uint64"); toml.put ("network_threads", network_threads, "Number of threads dedicated to processing network messages. Defaults to the number of CPU threads, and at least 4.\ntype:uint64"); @@ -261,6 +261,12 @@ nano::error nano::node_config::deserialize_toml (nano::tomlconfig & toml) optimistic_scheduler.deserialize (config_l); } + if (toml.has_key ("hinted_scheduler")) + { + auto config_l = toml.get_required_child ("hinted_scheduler"); + hinted_scheduler.deserialize (config_l); + } + if (toml.has_key ("bootstrap_ascending")) { auto config_l = toml.get_required_child ("bootstrap_ascending"); @@ -361,7 +367,6 @@ nano::error nano::node_config::deserialize_toml (nano::tomlconfig & toml) } toml.get ("bootstrap_fraction_numerator", bootstrap_fraction_numerator); - toml.get ("election_hint_weight_percent", election_hint_weight_percent); toml.get ("password_fanout", password_fanout); toml.get ("io_threads", io_threads); toml.get ("work_threads", work_threads); @@ -446,11 +451,6 @@ nano::error nano::node_config::deserialize_toml (nano::tomlconfig & toml) experimental_config_l.get ("max_pruning_depth", max_pruning_depth); } - // Validate ranges - if (election_hint_weight_percent < 5 || election_hint_weight_percent > 50) - { - toml.get_error ().set ("election_hint_weight_percent must be a number between 5 and 50"); - } if (password_fanout < 16 || password_fanout > 1024 * 1024) { toml.get_error ().set ("password_fanout must be a number between 16 and 1048576"); @@ -558,4 +558,4 @@ nano::account nano::node_config::random_representative () const std::size_t index (nano::random_pool::generate_word32 (0, static_cast (preconfigured_representatives.size () - 1))); auto result (preconfigured_representatives[index]); return result; -} +} \ No newline at end of file diff --git a/nano/node/nodeconfig.hpp b/nano/node/nodeconfig.hpp index 1c23d57d91..daf962c30c 100644 --- a/nano/node/nodeconfig.hpp +++ b/nano/node/nodeconfig.hpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -47,6 +48,7 @@ class node_config nano::network_params network_params; std::optional peering_port{}; nano::scheduler::optimistic_config optimistic_scheduler; + nano::scheduler::hinted_config hinted_scheduler; nano::logging logging; std::vector> work_peers; std::vector> secondary_work_peers{ { "127.0.0.1", 8076 } }; /* Default of nano-pow-server */ @@ -59,7 +61,6 @@ class node_config std::chrono::milliseconds vote_generator_delay{ std::chrono::milliseconds (100) }; unsigned vote_generator_threshold{ 3 }; nano::amount online_weight_minimum{ 60000 * nano::Gxrb_ratio }; - unsigned election_hint_weight_percent{ 50 }; unsigned password_fanout{ 1024 }; unsigned io_threads{ std::max (4u, nano::hardware_concurrency ()) }; unsigned network_threads{ std::max (4u, nano::hardware_concurrency ()) }; diff --git a/nano/node/scheduler/component.cpp b/nano/node/scheduler/component.cpp index 9a4f3de4e1..2108c683bb 100644 --- a/nano/node/scheduler/component.cpp +++ b/nano/node/scheduler/component.cpp @@ -6,7 +6,7 @@ #include nano::scheduler::component::component (nano::node & node) : - hinted_impl{ std::make_unique (nano::scheduler::hinted::config{ node.config }, node, node.vote_cache, node.active, node.online_reps, node.stats) }, + hinted_impl{ std::make_unique (node.config.hinted_scheduler, node, node.vote_cache, node.active, node.online_reps, node.stats) }, manual_impl{ std::make_unique (node) }, optimistic_impl{ std::make_unique (node.config.optimistic_scheduler, node, node.ledger, node.active, node.network_params.network, node.stats) }, priority_impl{ std::make_unique (node, node.stats) }, diff --git a/nano/node/scheduler/hinted.cpp b/nano/node/scheduler/hinted.cpp index 68b6368dc0..661e055299 100644 --- a/nano/node/scheduler/hinted.cpp +++ b/nano/node/scheduler/hinted.cpp @@ -1,14 +1,14 @@ #include +#include #include #include -nano::scheduler::hinted::config::config (nano::node_config const & config) : - vote_cache_check_interval_ms{ config.network_params.network.is_dev_network () ? 100u : 5000u } -{ -} +/* + * hinted + */ -nano::scheduler::hinted::hinted (config const & config_a, nano::node & node_a, nano::vote_cache & vote_cache_a, nano::active_transactions & active_a, nano::online_reps & online_reps_a, nano::stats & stats_a) : - config_m{ config_a }, +nano::scheduler::hinted::hinted (hinted_config const & config_a, nano::node & node_a, nano::vote_cache & vote_cache_a, nano::active_transactions & active_a, nano::online_reps & online_reps_a, nano::stats & stats_a) : + config{ config_a }, node{ node_a }, vote_cache{ vote_cache_a }, active{ active_a }, @@ -162,10 +162,8 @@ void nano::scheduler::hinted::run () nano::uint128_t nano::scheduler::hinted::tally_threshold () const { - // auto min_tally = (online_reps.trended () / 100) * node.config.election_hint_weight_percent; - // return min_tally; - - return 0; + auto min_tally = (online_reps.trended () / 100) * config.hinting_threshold_percent; + return min_tally; } nano::uint128_t nano::scheduler::hinted::final_tally_threshold () const @@ -177,7 +175,6 @@ nano::uint128_t nano::scheduler::hinted::final_tally_threshold () const bool nano::scheduler::hinted::cooldown (const nano::block_hash & hash) { auto const now = std::chrono::steady_clock::now (); - auto const cooldown = std::chrono::seconds{ 15 }; // Check if the hash is still in the cooldown period using the hashed index auto const & hashed_index = cooldowns_m.get (); @@ -191,7 +188,7 @@ bool nano::scheduler::hinted::cooldown (const nano::block_hash & hash) } // Insert the new entry - cooldowns_m.insert ({ hash, now + cooldown }); + cooldowns_m.insert ({ hash, now + config.block_cooldown }); // Trim old entries auto & seq_index = cooldowns_m.get (); @@ -201,4 +198,45 @@ bool nano::scheduler::hinted::cooldown (const nano::block_hash & hash) } return false; // No need to cooldown +} + +/* + * hinted_config + */ + +nano::scheduler::hinted_config::hinted_config (nano::network_constants const & network) +{ + if (network.is_dev_network ()) + { + check_interval = std::chrono::milliseconds{ 100 }; + } +} + +nano::error nano::scheduler::hinted_config::serialize (nano::tomlconfig & toml) const +{ + 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"); + + return toml.get_error (); +} + +nano::error nano::scheduler::hinted_config::deserialize (nano::tomlconfig & toml) +{ + toml.get ("hinting_threshold", hinting_threshold_percent); + + auto check_interval_l = check_interval.count (); + toml.get ("check_interval", check_interval_l); + check_interval = std::chrono::milliseconds{ check_interval_l }; + + auto block_cooldown_l = block_cooldown.count (); + toml.get ("block_cooldown", block_cooldown_l); + block_cooldown = std::chrono::milliseconds{ block_cooldown_l }; + + if (hinting_threshold_percent > 100) + { + toml.get_error ().set ("hinting_threshold must be a number between 0 and 100"); + } + + return toml.get_error (); } \ No newline at end of file diff --git a/nano/node/scheduler/hinted.hpp b/nano/node/scheduler/hinted.hpp index 095ea8064b..fa09903704 100644 --- a/nano/node/scheduler/hinted.hpp +++ b/nano/node/scheduler/hinted.hpp @@ -13,6 +13,8 @@ #include #include +namespace mi = boost::multi_index; + namespace nano { class node; @@ -21,23 +23,30 @@ class active_transactions; class vote_cache; class online_reps; } + namespace nano::scheduler { +class hinted_config final +{ +public: + explicit hinted_config (nano::network_constants const &); + + nano::error deserialize (nano::tomlconfig & toml); + nano::error serialize (nano::tomlconfig & toml) const; + +public: + std::chrono::milliseconds check_interval{ 5000 }; + std::chrono::milliseconds block_cooldown{ 10000 }; + unsigned hinting_threshold_percent{ 10 }; +}; + /* * Monitors inactive vote cache and schedules elections with the highest observed vote tally. */ class hinted final { -public: // Config - struct config final - { - explicit config (node_config const & config); - // Interval of wakeup to check inactive vote cache when idle - uint64_t vote_cache_check_interval_ms; - }; - public: - hinted (config const &, nano::node &, nano::vote_cache &, nano::active_transactions &, nano::online_reps &, nano::stats &); + hinted (hinted_config const &, nano::node &, nano::vote_cache &, nano::active_transactions &, nano::online_reps &, nano::stats &); ~hinted (); void start (); @@ -65,7 +74,7 @@ class hinted final nano::stats & stats; private: - config const config_m; + hinted_config const & config; bool stopped{ false }; nano::condition_variable condition; From 5ca17def1b91752f7bf810b27154352ff01f2a8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sat, 7 Oct 2023 22:03:19 +0200 Subject: [PATCH 07/13] Cleanup `vote_cache` --- nano/core_test/active_transactions.cpp | 12 +- nano/core_test/vote_cache.cpp | 167 ++++++++++++++----------- nano/node/process_live_dispatcher.cpp | 3 - nano/node/vote_cache.cpp | 155 +++-------------------- nano/node/vote_cache.hpp | 58 +-------- 5 files changed, 120 insertions(+), 275 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index a8cdd5cdf0..095f3ef889 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -238,7 +238,7 @@ TEST (active_transactions, inactive_votes_cache) .build_shared (); auto vote (std::make_shared (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::vote::timestamp_max, nano::vote::duration_max, std::vector (1, send->hash ()))); node.vote_processor.vote (vote, std::make_shared (node, node)); - ASSERT_TIMELY (5s, node.vote_cache.cache_size () == 1); + ASSERT_TIMELY (5s, node.vote_cache.size () == 1); node.process_active (send); node.block_processor.flush (); ASSERT_TIMELY (5s, node.ledger.block_confirmed (node.store.tx_begin_read (), send->hash ())); @@ -264,7 +264,7 @@ TEST (active_transactions, inactive_votes_cache_non_final) // Non-final vote auto vote = std::make_shared (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, 0, 0, std::vector (1, send->hash ())); node.vote_processor.vote (vote, std::make_shared (node, node)); - ASSERT_TIMELY (5s, node.vote_cache.cache_size () == 1); + ASSERT_TIMELY (5s, node.vote_cache.size () == 1); node.process_active (send); std::shared_ptr election; @@ -301,7 +301,7 @@ TEST (active_transactions, inactive_votes_cache_fork) auto const vote = std::make_shared (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::vote::timestamp_max, nano::vote::duration_max, std::vector (1, send1->hash ())); node.vote_processor.vote (vote, std::make_shared (node, node)); - ASSERT_TIMELY (5s, node.vote_cache.cache_size () == 1); + ASSERT_TIMELY (5s, node.vote_cache.size () == 1); node.process_active (send2); @@ -418,7 +418,7 @@ TEST (active_transactions, inactive_votes_cache_multiple_votes) ASSERT_TIMELY (5s, node.vote_cache.find (send1->hash ())); ASSERT_TIMELY (5s, node.vote_cache.find (send1->hash ())->voters ().size () == 2); - ASSERT_EQ (1, node.vote_cache.cache_size ()); + ASSERT_EQ (1, node.vote_cache.size ()); node.scheduler.priority.activate (nano::dev::genesis_key.pub, node.store.tx_begin_read ()); std::shared_ptr election; ASSERT_TIMELY (5s, election = node.active.election (send1->qualified_root ())); @@ -497,7 +497,7 @@ TEST (active_transactions, inactive_votes_cache_election_start) std::vector hashes{ open1->hash (), open2->hash (), send4->hash () }; auto vote1 (std::make_shared (key1.pub, key1.prv, 0, 0, hashes)); node.vote_processor.vote (vote1, std::make_shared (node, node)); - ASSERT_TIMELY (5s, node.vote_cache.cache_size () == 3); + ASSERT_TIMELY (5s, node.vote_cache.size () == 3); ASSERT_TRUE (node.active.empty ()); ASSERT_EQ (1, node.ledger.cache.cemented_count); // 2 votes are required to start election (dev network) @@ -1444,7 +1444,7 @@ TEST (active_transactions, limit_vote_hinted_elections) auto vote1 = nano::test::make_vote (rep1, { open0, open1 }); node.vote_processor.vote (vote1, nano::test::fake_channel (node)); // Ensure new inactive vote cache entries were created - ASSERT_TIMELY (5s, node.vote_cache.cache_size () == 2); + ASSERT_TIMELY (5s, node.vote_cache.size () == 2); // And no elections are getting started yet ASSERT_ALWAYS (1s, node.active.empty ()); // And nothing got confirmed yet diff --git a/nano/core_test/vote_cache.cpp b/nano/core_test/vote_cache.cpp index 555be41123..e225a572e5 100644 --- a/nano/core_test/vote_cache.cpp +++ b/nano/core_test/vote_cache.cpp @@ -33,18 +33,6 @@ nano::keypair create_rep (nano::uint128_t weight) return key; } -std::shared_ptr create_vote (nano::keypair & key, uint64_t timestamp, uint8_t duration, std::vector hashes) -{ - return nano::test::make_vote (key, hashes, timestamp, duration); -} - -nano::block_hash random_hash () -{ - nano::block_hash random_hash; - nano::random_pool::generate_block (random_hash.bytes.data (), random_hash.bytes.size ()); - return random_hash; -} - nano::vote_cache::config make_config (std::size_t max_size = 1024) { nano::vote_cache::config cfg{}; @@ -56,9 +44,9 @@ nano::vote_cache::config make_config (std::size_t max_size = 1024) TEST (vote_cache, construction) { nano::vote_cache vote_cache{ make_config () }; - ASSERT_EQ (0, vote_cache.cache_size ()); - ASSERT_TRUE (vote_cache.cache_empty ()); - auto hash1 = random_hash (); + ASSERT_EQ (0, vote_cache.size ()); + ASSERT_TRUE (vote_cache.empty ()); + auto hash1 = nano::test::random_hash (); ASSERT_FALSE (vote_cache.find (hash1)); } @@ -70,18 +58,24 @@ TEST (vote_cache, insert_one_hash) nano::vote_cache vote_cache{ make_config () }; vote_cache.rep_weight_query = rep_weight_query (); auto rep1 = create_rep (7); - auto hash1 = random_hash (); - auto vote1 = create_vote (rep1, 1024 * 1024, 0, { hash1 }); + auto hash1 = nano::test::random_hash (); + auto vote1 = nano::test::make_vote (rep1, { hash1 }, 1024 * 1024); vote_cache.vote (vote1->hashes.front (), vote1); - ASSERT_EQ (1, vote_cache.cache_size ()); - ASSERT_TRUE (vote_cache.find (hash1)); - auto peek1 = vote_cache.peek (); + ASSERT_EQ (1, vote_cache.size ()); + + auto peek1 = vote_cache.find (hash1); ASSERT_TRUE (peek1); ASSERT_EQ (peek1->hash (), hash1); ASSERT_EQ (peek1->voters ().size (), 1); ASSERT_EQ (peek1->voters ().front ().representative, rep1.pub); // account ASSERT_EQ (peek1->voters ().front ().timestamp, 1024 * 1024); // timestamp ASSERT_EQ (peek1->tally (), 7); + + auto tops = vote_cache.top (0); + ASSERT_EQ (tops.size (), 1); + ASSERT_EQ (tops[0].hash, hash1); + ASSERT_EQ (tops[0].tally, 7); + ASSERT_EQ (tops[0].final_tally, 0); } /* @@ -92,23 +86,30 @@ TEST (vote_cache, insert_one_hash_many_votes) { nano::vote_cache vote_cache{ make_config () }; vote_cache.rep_weight_query = rep_weight_query (); - auto hash1 = random_hash (); + auto hash1 = nano::test::random_hash (); auto rep1 = create_rep (7); auto rep2 = create_rep (9); auto rep3 = create_rep (11); - auto vote1 = create_vote (rep1, 1 * 1024 * 1024, 0, { hash1 }); - auto vote2 = create_vote (rep2, 2 * 1024 * 1024, 0, { hash1 }); - auto vote3 = create_vote (rep3, 3 * 1024 * 1024, 0, { hash1 }); + auto vote1 = nano::test::make_vote (rep1, { hash1 }, 1 * 1024 * 1024); + auto vote2 = nano::test::make_vote (rep2, { hash1 }, 2 * 1024 * 1024); + auto vote3 = nano::test::make_vote (rep3, { hash1 }, 3 * 1024 * 1024); vote_cache.vote (vote1->hashes.front (), vote1); vote_cache.vote (vote2->hashes.front (), vote2); vote_cache.vote (vote3->hashes.front (), vote3); + // We have 3 votes but for a single hash, so just one entry in vote cache - ASSERT_EQ (1, vote_cache.cache_size ()); - auto peek1 = vote_cache.peek (); + ASSERT_EQ (1, vote_cache.size ()); + auto peek1 = vote_cache.find (hash1); ASSERT_TRUE (peek1); ASSERT_EQ (peek1->voters ().size (), 3); // Tally must be the sum of rep weights ASSERT_EQ (peek1->tally (), 7 + 9 + 11); + + auto tops = vote_cache.top (0); + ASSERT_EQ (tops.size (), 1); + ASSERT_EQ (tops[0].hash, hash1); + ASSERT_EQ (tops[0].tally, 7 + 9 + 11); + ASSERT_EQ (tops[0].final_tally, 0); } /* @@ -120,31 +121,36 @@ TEST (vote_cache, insert_many_hashes_many_votes) nano::vote_cache vote_cache{ make_config () }; vote_cache.rep_weight_query = rep_weight_query (); // There will be 3 random hashes to vote for - auto hash1 = random_hash (); - auto hash2 = random_hash (); - auto hash3 = random_hash (); + auto hash1 = nano::test::random_hash (); + auto hash2 = nano::test::random_hash (); + auto hash3 = nano::test::random_hash (); // There will be 4 reps with different weights auto rep1 = create_rep (7); auto rep2 = create_rep (9); auto rep3 = create_rep (11); auto rep4 = create_rep (13); // Votes: rep1 > hash1, rep2 > hash2, rep3 > hash3, rep4 > hash1 (the same as rep1) - auto vote1 = create_vote (rep1, 1024 * 1024, 0, { hash1 }); - auto vote2 = create_vote (rep2, 1024 * 1024, 0, { hash2 }); - auto vote3 = create_vote (rep3, 1024 * 1024, 0, { hash3 }); - auto vote4 = create_vote (rep4, 1024 * 1024, 0, { hash1 }); + auto vote1 = nano::test::make_vote (rep1, { hash1 }, 1024 * 1024); + auto vote2 = nano::test::make_vote (rep2, { hash2 }, 1024 * 1024); + auto vote3 = nano::test::make_vote (rep3, { hash3 }, 1024 * 1024); + auto vote4 = nano::test::make_vote (rep4, { hash1 }, 1024 * 1024); // Insert first 3 votes in cache vote_cache.vote (vote1->hashes.front (), vote1); vote_cache.vote (vote2->hashes.front (), vote2); vote_cache.vote (vote3->hashes.front (), vote3); // Ensure all of those are properly inserted - ASSERT_EQ (3, vote_cache.cache_size ()); + ASSERT_EQ (3, vote_cache.size ()); ASSERT_TRUE (vote_cache.find (hash1)); ASSERT_TRUE (vote_cache.find (hash2)); ASSERT_TRUE (vote_cache.find (hash3)); // Ensure that first entry in queue is the one for hash3 (rep3 has the highest weight of the first 3 reps) - auto peek1 = vote_cache.peek (); + auto tops1 = vote_cache.top (0); + ASSERT_EQ (tops1.size (), 3); + ASSERT_EQ (tops1[0].hash, hash3); + ASSERT_EQ (tops1[0].tally, 11); + + auto peek1 = vote_cache.find (tops1[0].hash); ASSERT_TRUE (peek1); ASSERT_EQ (peek1->voters ().size (), 1); ASSERT_EQ (peek1->tally (), 11); @@ -154,28 +160,36 @@ TEST (vote_cache, insert_many_hashes_many_votes) vote_cache.vote (vote4->hashes.front (), vote4); // Ensure that the first entry in queue is now the one for hash1 (rep1 + rep4 tally weight) - auto pop1 = vote_cache.pop (); + auto tops2 = vote_cache.top (0); + ASSERT_EQ (tops1.size (), 3); + ASSERT_EQ (tops2[0].hash, hash1); + ASSERT_EQ (tops2[0].tally, 7 + 13); + + auto pop1 = vote_cache.find (tops2[0].hash); ASSERT_TRUE (pop1); ASSERT_EQ ((*pop1).voters ().size (), 2); ASSERT_EQ ((*pop1).tally (), 7 + 13); ASSERT_EQ ((*pop1).hash (), hash1); - ASSERT_TRUE (vote_cache.find (hash1)); // Only pop from queue, votes should still be stored in cache - // After popping the previous entry, the next entry in queue should be hash3 (rep3 tally weight) - auto pop2 = vote_cache.pop (); + // The next entry in queue should be hash3 (rep3 tally weight) + ASSERT_EQ (tops2[1].hash, hash3); + ASSERT_EQ (tops2[1].tally, 11); + + auto pop2 = vote_cache.find (tops2[1].hash); ASSERT_EQ ((*pop2).voters ().size (), 1); ASSERT_EQ ((*pop2).tally (), 11); ASSERT_EQ ((*pop2).hash (), hash3); ASSERT_TRUE (vote_cache.find (hash3)); // And last one should be hash2 with rep2 tally weight - auto pop3 = vote_cache.pop (); + ASSERT_EQ (tops2[2].hash, hash2); + ASSERT_EQ (tops2[2].tally, 9); + + auto pop3 = vote_cache.find (tops2[2].hash); ASSERT_EQ ((*pop3).voters ().size (), 1); ASSERT_EQ ((*pop3).tally (), 9); ASSERT_EQ ((*pop3).hash (), hash2); ASSERT_TRUE (vote_cache.find (hash2)); - - ASSERT_TRUE (vote_cache.queue_empty ()); } /* @@ -185,13 +199,13 @@ TEST (vote_cache, insert_duplicate) { nano::vote_cache vote_cache{ make_config () }; vote_cache.rep_weight_query = rep_weight_query (); - auto hash1 = random_hash (); + auto hash1 = nano::test::random_hash (); auto rep1 = create_rep (9); - auto vote1 = create_vote (rep1, 1 * 1024 * 1024, 0, { hash1 }); - auto vote2 = create_vote (rep1, 1 * 1024 * 1024, 0, { hash1 }); + auto vote1 = nano::test::make_vote (rep1, { hash1 }, 1 * 1024 * 1024); + auto vote2 = nano::test::make_vote (rep1, { hash1 }, 1 * 1024 * 1024); vote_cache.vote (vote1->hashes.front (), vote1); vote_cache.vote (vote2->hashes.front (), vote2); - ASSERT_EQ (1, vote_cache.cache_size ()); + ASSERT_EQ (1, vote_cache.size ()); } /* @@ -201,17 +215,17 @@ TEST (vote_cache, insert_newer) { nano::vote_cache vote_cache{ make_config () }; vote_cache.rep_weight_query = rep_weight_query (); - auto hash1 = random_hash (); + auto hash1 = nano::test::random_hash (); auto rep1 = create_rep (9); - auto vote1 = create_vote (rep1, 1 * 1024 * 1024, 0, { hash1 }); + auto vote1 = nano::test::make_vote (rep1, { hash1 }, 1 * 1024 * 1024); vote_cache.vote (vote1->hashes.front (), vote1); - auto peek1 = vote_cache.peek (); + auto peek1 = vote_cache.find (hash1); ASSERT_TRUE (peek1); - auto vote2 = create_vote (rep1, nano::vote::timestamp_max, nano::vote::duration_max, { hash1 }); + auto vote2 = nano::test::make_final_vote (rep1, { hash1 }); vote_cache.vote (vote2->hashes.front (), vote2); - auto peek2 = vote_cache.peek (); + auto peek2 = vote_cache.find (hash1); ASSERT_TRUE (peek2); - ASSERT_EQ (1, vote_cache.cache_size ()); + ASSERT_EQ (1, vote_cache.size ()); ASSERT_EQ (1, peek2->voters ().size ()); // Second entry should have timestamp greater than the first one ASSERT_GT (peek2->voters ().front ().timestamp, peek1->voters ().front ().timestamp); @@ -225,17 +239,17 @@ TEST (vote_cache, insert_older) { nano::vote_cache vote_cache{ make_config () }; vote_cache.rep_weight_query = rep_weight_query (); - auto hash1 = random_hash (); + auto hash1 = nano::test::random_hash (); auto rep1 = create_rep (9); - auto vote1 = create_vote (rep1, 2 * 1024 * 1024, 0, { hash1 }); + auto vote1 = nano::test::make_vote (rep1, { hash1 }, 2 * 1024 * 1024); vote_cache.vote (vote1->hashes.front (), vote1); - auto peek1 = vote_cache.peek (); + auto peek1 = vote_cache.find (hash1); ASSERT_TRUE (peek1); - auto vote2 = create_vote (rep1, 1 * 1024 * 1024, 0, { hash1 }); + auto vote2 = nano::test::make_vote (rep1, { hash1 }, 1 * 1024 * 1024); vote_cache.vote (vote2->hashes.front (), vote2); - auto peek2 = vote_cache.peek (); + auto peek2 = vote_cache.find (hash1); ASSERT_TRUE (peek2); - ASSERT_EQ (1, vote_cache.cache_size ()); + ASSERT_EQ (1, vote_cache.size ()); ASSERT_EQ (1, peek2->voters ().size ()); ASSERT_EQ (peek2->voters ().front ().timestamp, peek1->voters ().front ().timestamp); // timestamp2 == timestamp1 } @@ -247,25 +261,26 @@ TEST (vote_cache, erase) { nano::vote_cache vote_cache{ make_config () }; vote_cache.rep_weight_query = rep_weight_query (); - auto hash1 = random_hash (); - auto hash2 = random_hash (); - auto hash3 = random_hash (); + auto hash1 = nano::test::random_hash (); + auto hash2 = nano::test::random_hash (); + auto hash3 = nano::test::random_hash (); auto rep1 = create_rep (7); auto rep2 = create_rep (9); auto rep3 = create_rep (11); auto rep4 = create_rep (13); - auto vote1 = create_vote (rep1, 1024 * 1024, 0, { hash1 }); - auto vote2 = create_vote (rep2, 1024 * 1024, 0, { hash2 }); - auto vote3 = create_vote (rep3, 1024 * 1024, 0, { hash3 }); + auto vote1 = nano::test::make_vote (rep1, { hash1 }, 1024 * 1024); + auto vote2 = nano::test::make_vote (rep2, { hash2 }, 1024 * 1024); + auto vote3 = nano::test::make_vote (rep3, { hash3 }, 1024 * 1024); vote_cache.vote (vote1->hashes.front (), vote1); vote_cache.vote (vote2->hashes.front (), vote2); vote_cache.vote (vote3->hashes.front (), vote3); - ASSERT_EQ (3, vote_cache.cache_size ()); + ASSERT_EQ (3, vote_cache.size ()); + ASSERT_FALSE (vote_cache.empty ()); ASSERT_TRUE (vote_cache.find (hash1)); ASSERT_TRUE (vote_cache.find (hash2)); ASSERT_TRUE (vote_cache.find (hash3)); vote_cache.erase (hash2); - ASSERT_EQ (2, vote_cache.cache_size ()); + ASSERT_EQ (2, vote_cache.size ()); ASSERT_TRUE (vote_cache.find (hash1)); ASSERT_FALSE (vote_cache.find (hash2)); ASSERT_TRUE (vote_cache.find (hash3)); @@ -274,7 +289,7 @@ TEST (vote_cache, erase) ASSERT_FALSE (vote_cache.find (hash1)); ASSERT_FALSE (vote_cache.find (hash2)); ASSERT_FALSE (vote_cache.find (hash3)); - ASSERT_TRUE (vote_cache.cache_empty ()); + ASSERT_TRUE (vote_cache.empty ()); } /* @@ -290,15 +305,15 @@ TEST (vote_cache, overfill) { // The more recent the vote, the less voting weight it has auto rep1 = create_rep (count - n); - auto hash1 = random_hash (); - auto vote1 = create_vote (rep1, 1024 * 1024, 0, { hash1 }); + auto hash1 = nano::test::random_hash (); + auto vote1 = nano::test::make_vote (rep1, { hash1 }, 1024 * 1024); vote_cache.vote (vote1->hashes.front (), vote1); } - ASSERT_LT (vote_cache.cache_size (), count); - auto peek1 = vote_cache.peek (); - ASSERT_TRUE (peek1); + ASSERT_LT (vote_cache.size (), count); // Check that oldest votes are dropped first - ASSERT_EQ (peek1->tally (), 1024); + auto tops = vote_cache.top (0); + ASSERT_EQ (tops.size (), 1024); + ASSERT_EQ (tops[0].tally, 1024); } /* @@ -309,12 +324,12 @@ TEST (vote_cache, overfill_entry) nano::vote_cache vote_cache{ make_config () }; vote_cache.rep_weight_query = rep_weight_query (); const int count = 1024; - auto hash1 = random_hash (); + auto hash1 = nano::test::random_hash (); for (int n = 0; n < count; ++n) { auto rep1 = create_rep (9); - auto vote1 = create_vote (rep1, 1024 * 1024, 0, { hash1 }); + auto vote1 = nano::test::make_vote (rep1, { hash1 }, 1024 * 1024); vote_cache.vote (vote1->hashes.front (), vote1); } - ASSERT_EQ (1, vote_cache.cache_size ()); + ASSERT_EQ (1, vote_cache.size ()); } \ No newline at end of file diff --git a/nano/node/process_live_dispatcher.cpp b/nano/node/process_live_dispatcher.cpp index 97019f2163..c7a6d16dbf 100644 --- a/nano/node/process_live_dispatcher.cpp +++ b/nano/node/process_live_dispatcher.cpp @@ -49,9 +49,6 @@ void nano::process_live_dispatcher::process_live (nano::block const & block, sto scheduler.activate (account, transaction); } - // Notify inactive vote cache about a new live block - vote_cache.trigger (block.hash ()); - if (websocket.server && websocket.server->any_subscriber (nano::websocket::topic::new_unconfirmed_block)) { websocket.server->broadcast (nano::websocket::message_builder ().new_block_arrived (block)); diff --git a/nano/node/vote_cache.cpp b/nano/node/vote_cache.cpp index 890821b6c0..c9450240d5 100644 --- a/nano/node/vote_cache.cpp +++ b/nano/node/vote_cache.cpp @@ -102,38 +102,18 @@ nano::vote_cache::vote_cache (const config config_a) : void nano::vote_cache::vote (const nano::block_hash & hash, const std::shared_ptr vote) { - auto weight = rep_weight_query (vote->account); - vote_impl (hash, vote->account, vote->timestamp (), weight); -} + auto const representative = vote->account; + auto const timestamp = vote->timestamp (); + auto const rep_weight = rep_weight_query (representative); -void nano::vote_cache::vote_impl (const nano::block_hash & hash, const nano::account & representative, uint64_t const & timestamp, const nano::uint128_t & rep_weight) -{ nano::unique_lock lock{ mutex }; - /** - * If there is no cache entry for the block hash, create a new entry for both cache and queue. - * Otherwise update existing cache entry and, if queue contains entry for the block hash, update the queue entry - */ auto & cache_by_hash = cache.get (); if (auto existing = cache_by_hash.find (hash); existing != cache_by_hash.end ()) { - bool modified = false; - bool success = cache_by_hash.modify (existing, [&representative, ×tamp, &rep_weight, &modified] (entry & ent) { - modified = ent.vote (representative, timestamp, rep_weight); + cache_by_hash.modify (existing, [&representative, ×tamp, &rep_weight] (entry & ent) { + ent.vote (representative, timestamp, rep_weight); }); - release_assert (success); // Ensure iterator `existing` is valid - - if (modified) - { - auto & queue_by_hash = queue.get (); - if (auto queue_existing = queue_by_hash.find (hash); queue_existing != queue_by_hash.end ()) - { - queue_by_hash.modify (queue_existing, [&existing] (queue_entry & ent) { - ent.tally = existing->tally (); - ent.final_tally = existing->final_tally (); - }); - } - } } else { @@ -142,150 +122,50 @@ void nano::vote_cache::vote_impl (const nano::block_hash & hash, const nano::acc cache.get ().insert (cache_entry); - // If a stale entry for the same hash already exists in queue, replace it by a new entry with fresh tally - auto & queue_by_hash = queue.get (); - if (auto queue_existing = queue_by_hash.find (hash); queue_existing != queue_by_hash.end ()) + // When cache overflown remove the oldest entry + if (cache.size () > max_size) { - queue_by_hash.erase (queue_existing); + cache.get ().pop_front (); } - queue_by_hash.insert ({ hash, cache_entry.tally (), cache_entry.final_tally () }); - - trim_overflow_locked (); } } -bool nano::vote_cache::cache_empty () const +bool nano::vote_cache::empty () const { nano::lock_guard lock{ mutex }; return cache.empty (); } -bool nano::vote_cache::queue_empty () const -{ - nano::lock_guard lock{ mutex }; - return queue.empty (); -} - -std::size_t nano::vote_cache::cache_size () const +std::size_t nano::vote_cache::size () const { nano::lock_guard lock{ mutex }; return cache.size (); } -std::size_t nano::vote_cache::queue_size () const -{ - nano::lock_guard lock{ mutex }; - return queue.size (); -} - std::optional nano::vote_cache::find (const nano::block_hash & hash) const -{ - nano::lock_guard lock{ mutex }; - return find_locked (hash); -} - -bool nano::vote_cache::erase (const nano::block_hash & hash) { nano::lock_guard lock{ mutex }; - bool result = false; auto & cache_by_hash = cache.get (); if (auto existing = cache_by_hash.find (hash); existing != cache_by_hash.end ()) { - cache_by_hash.erase (existing); - result = true; - } - auto & queue_by_hash = queue.get (); - if (auto existing = queue_by_hash.find (hash); existing != queue_by_hash.end ()) - { - queue_by_hash.erase (existing); - } - return result; -} - -std::optional nano::vote_cache::pop (nano::uint128_t const & min_tally) -{ - nano::lock_guard lock{ mutex }; - - if (!queue.empty ()) - { - auto & queue_by_tally = queue.get (); - auto top = std::prev (queue_by_tally.end ()); // Iterator to element with the highest tally - if (auto maybe_cache_entry = find_locked (top->hash); maybe_cache_entry) - { - // Here we check whether our best candidate passes the minimum vote tally threshold - // If yes, erase it from the queue (but still keep the votes in cache) - if (maybe_cache_entry->tally () >= min_tally) - { - queue_by_tally.erase (top); - return maybe_cache_entry.value (); - } - } - } - return {}; -} - -std::optional nano::vote_cache::peek (nano::uint128_t const & min_tally) const -{ - nano::lock_guard lock{ mutex }; - - if (!queue.empty ()) - { - auto & queue_by_tally = queue.get (); - auto top = std::prev (queue_by_tally.end ()); // Iterator to element with the highest tally - if (auto maybe_cache_entry = find_locked (top->hash); maybe_cache_entry) - { - if (maybe_cache_entry->tally () >= min_tally) - { - return maybe_cache_entry.value (); - } - } + return *existing; } return {}; } -void nano::vote_cache::trigger (const nano::block_hash & hash) +bool nano::vote_cache::erase (const nano::block_hash & hash) { nano::lock_guard lock{ mutex }; - // Only reinsert to queue if it is not already in queue and there are votes in passive cache - auto & queue_by_hash = queue.get (); - if (auto existing_queue = queue_by_hash.find (hash); existing_queue == queue_by_hash.end ()) - { - if (auto maybe_cache_entry = find_locked (hash); maybe_cache_entry) - { - queue_by_hash.insert ({ hash, maybe_cache_entry->tally (), maybe_cache_entry->final_tally () }); - - trim_overflow_locked (); - } - } -} - -std::optional nano::vote_cache::find_locked (const nano::block_hash & hash) const -{ - debug_assert (!mutex.try_lock ()); - + bool result = false; auto & cache_by_hash = cache.get (); if (auto existing = cache_by_hash.find (hash); existing != cache_by_hash.end ()) { - return *existing; - } - return {}; -} - -void nano::vote_cache::trim_overflow_locked () -{ - debug_assert (!mutex.try_lock ()); - - // When cache overflown remove the oldest entry - if (cache.size () > max_size) - { - cache.get ().pop_front (); - } - if (queue.size () > max_size) - { - queue.get ().pop_front (); + cache_by_hash.erase (existing); + result = true; } + return result; } std::vector nano::vote_cache::top (const nano::uint128_t & min_tally) const @@ -322,7 +202,6 @@ std::vector nano::vote_cache::top (const nano::uint std::unique_ptr nano::vote_cache::collect_container_info (const std::string & name) { auto composite = std::make_unique (name); - composite->add_component (std::make_unique (container_info{ "cache", cache_size (), sizeof (ordered_cache::value_type) })); - composite->add_component (std::make_unique (container_info{ "queue", queue_size (), sizeof (ordered_queue::value_type) })); + composite->add_component (std::make_unique (container_info{ "cache", size (), sizeof (ordered_cache::value_type) })); return composite; } \ No newline at end of file diff --git a/nano/node/vote_cache.hpp b/nano/node/vote_cache.hpp index d88998fd2c..50ebeb4b09 100644 --- a/nano/node/vote_cache.hpp +++ b/nano/node/vote_cache.hpp @@ -77,15 +77,6 @@ class vote_cache final nano::uint128_t final_tally_m{ 0 }; }; -private: - class queue_entry final - { - public: - nano::block_hash hash{ 0 }; - nano::uint128_t tally{ 0 }; - nano::uint128_t final_tally{ 0 }; - }; - public: explicit vote_cache (const config); @@ -102,27 +93,9 @@ class vote_cache final * @return true if hash existed and was erased, false otherwise */ bool erase (nano::block_hash const & hash); - /** - * Returns an entry with the highest tally. - * @param min_tally minimum tally threshold, entries below with their voting weight below this will be ignored - */ - std::optional peek (nano::uint128_t const & min_tally = 0) const; - /** - * Returns an entry with the highest tally and removes it from container. - * @param min_tally minimum tally threshold, entries below with their voting weight below this will be ignored - */ - std::optional pop (nano::uint128_t const & min_tally = 0); - /** - * Reinserts a block into the queue. - * It is possible that we dequeue a hash that doesn't have a received block yet (for eg. if publish message was lost). - * We need a way to reinsert that hash into the queue when we finally receive the block - */ - void trigger (const nano::block_hash & hash); - std::size_t cache_size () const; - std::size_t queue_size () const; - bool cache_empty () const; - bool queue_empty () const; + std::size_t size () const; + bool empty () const; public: struct top_entry @@ -133,7 +106,9 @@ class vote_cache final }; /** - * Returns blocks with highest observed tally, greater than `min_tally` + * Returns blocks with highest observed tally + * The blocks are sorted in descending order by final tally, then by tally + * @param min_tally minimum tally threshold, entries below with their voting weight below this will be ignored */ std::vector top (nano::uint128_t const & min_tally) const; @@ -147,17 +122,12 @@ class vote_cache final std::function rep_weight_query{ [] (nano::account const & rep) { return 0; } }; private: - void vote_impl (nano::block_hash const & hash, nano::account const & representative, uint64_t const & timestamp, nano::uint128_t const & rep_weight); - std::optional find_locked (nano::block_hash const & hash) const; - void trim_overflow_locked (); - const std::size_t max_size; // clang-format off class tag_sequenced {}; class tag_hash {}; class tag_tally {}; - class tag_final_tally {}; // clang-format on // clang-format off @@ -167,27 +137,11 @@ class vote_cache final mi::hashed_unique, mi::const_mem_fun>, mi::ordered_non_unique, - mi::const_mem_fun, std::greater<>>, // DESC - mi::ordered_non_unique, - mi::const_mem_fun, std::greater<>> // DESC + mi::const_mem_fun, std::greater<>> // DESC >>; // clang-format on ordered_cache cache; - // clang-format off - using ordered_queue = boost::multi_index_container>, - mi::ordered_non_unique, - mi::member>, - mi::ordered_non_unique, - mi::member>, - mi::hashed_unique, - mi::member> - >>; - // clang-format on - ordered_queue queue; - mutable nano::mutex mutex; }; } \ No newline at end of file From 10206b819da324796e43af971fe63c0741e887f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sat, 7 Oct 2023 22:33:22 +0200 Subject: [PATCH 08/13] Dedicated config for vote cache --- nano/core_test/toml.cpp | 10 +++++++++ nano/core_test/vote_cache.cpp | 38 +++++++++++++++++++--------------- nano/node/cli.cpp | 5 ----- nano/node/node.cpp | 9 +------- nano/node/node.hpp | 1 - nano/node/nodeconfig.cpp | 10 +++++++++ nano/node/nodeconfig.hpp | 3 ++- nano/node/scheduler/hinted.cpp | 2 +- nano/node/vote_cache.cpp | 35 ++++++++++++++++++++++++------- nano/node/vote_cache.hpp | 29 +++++++++++++++----------- nano/slow_test/vote_cache.cpp | 4 ++-- 11 files changed, 92 insertions(+), 54 deletions(-) diff --git a/nano/core_test/toml.cpp b/nano/core_test/toml.cpp index 1c3a88b01a..02845ba1f9 100644 --- a/nano/core_test/toml.cpp +++ b/nano/core_test/toml.cpp @@ -275,6 +275,9 @@ 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.vote_cache.max_size, defaults.node.vote_cache.max_size); + ASSERT_EQ (conf.node.vote_cache.max_voters, defaults.node.vote_cache.max_voters); } TEST (toml, optional_child) @@ -551,6 +554,10 @@ TEST (toml, daemon_config_deserialize_no_defaults) max_pruning_age = 999 max_pruning_depth = 999 + [node.vote_cache] + max_size = 999 + max_voters = 999 + [opencl] device = 999 enable = true @@ -713,6 +720,9 @@ 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.vote_cache.max_size, defaults.node.vote_cache.max_size); + ASSERT_NE (conf.node.vote_cache.max_voters, defaults.node.vote_cache.max_voters); } /** There should be no required values **/ diff --git a/nano/core_test/vote_cache.cpp b/nano/core_test/vote_cache.cpp index e225a572e5..f94dda0679 100644 --- a/nano/core_test/vote_cache.cpp +++ b/nano/core_test/vote_cache.cpp @@ -32,18 +32,12 @@ nano::keypair create_rep (nano::uint128_t weight) register_rep (key.pub, weight); return key; } - -nano::vote_cache::config make_config (std::size_t max_size = 1024) -{ - nano::vote_cache::config cfg{}; - cfg.max_size = max_size; - return cfg; -} } TEST (vote_cache, construction) { - nano::vote_cache vote_cache{ make_config () }; + nano::vote_cache_config cfg; + nano::vote_cache vote_cache{ cfg }; ASSERT_EQ (0, vote_cache.size ()); ASSERT_TRUE (vote_cache.empty ()); auto hash1 = nano::test::random_hash (); @@ -55,7 +49,8 @@ TEST (vote_cache, construction) */ TEST (vote_cache, insert_one_hash) { - nano::vote_cache vote_cache{ make_config () }; + nano::vote_cache_config cfg; + nano::vote_cache vote_cache{ cfg }; vote_cache.rep_weight_query = rep_weight_query (); auto rep1 = create_rep (7); auto hash1 = nano::test::random_hash (); @@ -84,7 +79,8 @@ TEST (vote_cache, insert_one_hash) */ TEST (vote_cache, insert_one_hash_many_votes) { - nano::vote_cache vote_cache{ make_config () }; + nano::vote_cache_config cfg; + nano::vote_cache vote_cache{ cfg }; vote_cache.rep_weight_query = rep_weight_query (); auto hash1 = nano::test::random_hash (); auto rep1 = create_rep (7); @@ -118,7 +114,8 @@ TEST (vote_cache, insert_one_hash_many_votes) */ TEST (vote_cache, insert_many_hashes_many_votes) { - nano::vote_cache vote_cache{ make_config () }; + nano::vote_cache_config cfg; + nano::vote_cache vote_cache{ cfg }; vote_cache.rep_weight_query = rep_weight_query (); // There will be 3 random hashes to vote for auto hash1 = nano::test::random_hash (); @@ -197,7 +194,8 @@ TEST (vote_cache, insert_many_hashes_many_votes) */ TEST (vote_cache, insert_duplicate) { - nano::vote_cache vote_cache{ make_config () }; + nano::vote_cache_config cfg; + nano::vote_cache vote_cache{ cfg }; vote_cache.rep_weight_query = rep_weight_query (); auto hash1 = nano::test::random_hash (); auto rep1 = create_rep (9); @@ -213,7 +211,8 @@ TEST (vote_cache, insert_duplicate) */ TEST (vote_cache, insert_newer) { - nano::vote_cache vote_cache{ make_config () }; + nano::vote_cache_config cfg; + nano::vote_cache vote_cache{ cfg }; vote_cache.rep_weight_query = rep_weight_query (); auto hash1 = nano::test::random_hash (); auto rep1 = create_rep (9); @@ -237,7 +236,8 @@ TEST (vote_cache, insert_newer) */ TEST (vote_cache, insert_older) { - nano::vote_cache vote_cache{ make_config () }; + nano::vote_cache_config cfg; + nano::vote_cache vote_cache{ cfg }; vote_cache.rep_weight_query = rep_weight_query (); auto hash1 = nano::test::random_hash (); auto rep1 = create_rep (9); @@ -259,7 +259,8 @@ TEST (vote_cache, insert_older) */ TEST (vote_cache, erase) { - nano::vote_cache vote_cache{ make_config () }; + nano::vote_cache_config cfg; + nano::vote_cache vote_cache{ cfg }; vote_cache.rep_weight_query = rep_weight_query (); auto hash1 = nano::test::random_hash (); auto hash2 = nano::test::random_hash (); @@ -298,7 +299,9 @@ TEST (vote_cache, erase) TEST (vote_cache, overfill) { // Create a vote cache with max size set to 1024 - nano::vote_cache vote_cache{ make_config (1024) }; + nano::vote_cache_config cfg; + cfg.max_size = 1024; + nano::vote_cache vote_cache{ cfg }; vote_cache.rep_weight_query = rep_weight_query (); const int count = 16 * 1024; for (int n = 0; n < count; ++n) @@ -321,7 +324,8 @@ TEST (vote_cache, overfill) */ TEST (vote_cache, overfill_entry) { - nano::vote_cache vote_cache{ make_config () }; + nano::vote_cache_config cfg; + nano::vote_cache vote_cache{ cfg }; vote_cache.rep_weight_query = rep_weight_query (); const int count = 1024; auto hash1 = nano::test::random_hash (); diff --git a/nano/node/cli.cpp b/nano/node/cli.cpp index b3b63447ed..3eb0c29940 100644 --- a/nano/node/cli.cpp +++ b/nano/node/cli.cpp @@ -160,11 +160,6 @@ std::error_code nano::update_flags (nano::node_flags & flags_a, boost::program_o { flags_a.block_processor_verification_size = block_processor_verification_size_it->second.as (); } - auto inactive_votes_cache_size_it = vm.find ("inactive_votes_cache_size"); - if (inactive_votes_cache_size_it != vm.end ()) - { - flags_a.inactive_votes_cache_size = inactive_votes_cache_size_it->second.as (); - } auto vote_processor_capacity_it = vm.find ("vote_processor_capacity"); if (vote_processor_capacity_it != vm.end ()) { diff --git a/nano/node/node.cpp b/nano/node/node.cpp index e9275ef6aa..07c5c73cff 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -49,13 +49,6 @@ nano::backlog_population::config nano::backlog_population_config (const nano::no return cfg; } -nano::vote_cache::config nano::nodeconfig_to_vote_cache_config (node_config const & config, node_flags const & flags) -{ - vote_cache::config cfg{}; - cfg.max_size = flags.inactive_votes_cache_size; - return cfg; -} - nano::outbound_bandwidth_limiter::config nano::outbound_bandwidth_limiter_config (const nano::node_config & config) { outbound_bandwidth_limiter::config cfg{}; @@ -191,7 +184,7 @@ nano::node::node (boost::asio::io_context & io_ctx_a, boost::filesystem::path co 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{ nano::nodeconfig_to_vote_cache_config (config, flags) }, + vote_cache{ config.vote_cache }, 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), diff --git a/nano/node/node.hpp b/nano/node/node.hpp index 14b773c07e..acc219ae2a 100644 --- a/nano/node/node.hpp +++ b/nano/node/node.hpp @@ -65,7 +65,6 @@ std::unique_ptr collect_container_info (rep_crawler & // Configs backlog_population::config backlog_population_config (node_config const &); -vote_cache::config nodeconfig_to_vote_cache_config (node_config const &, node_flags const &); outbound_bandwidth_limiter::config outbound_bandwidth_limiter_config (node_config const &); class node final : public std::enable_shared_from_this diff --git a/nano/node/nodeconfig.cpp b/nano/node/nodeconfig.cpp index 581914dc8a..2453c2c24d 100644 --- a/nano/node/nodeconfig.cpp +++ b/nano/node/nodeconfig.cpp @@ -204,6 +204,10 @@ nano::error nano::node_config::serialize_toml (nano::tomlconfig & toml) const bootstrap_ascending.serialize (bootstrap_ascending_l); toml.put_child ("bootstrap_ascending", bootstrap_ascending_l); + nano::tomlconfig vote_cache_l; + vote_cache.serialize (vote_cache_l); + toml.put_child ("vote_cache", vote_cache_l); + return toml.get_error (); } @@ -273,6 +277,12 @@ nano::error nano::node_config::deserialize_toml (nano::tomlconfig & toml) bootstrap_ascending.deserialize (config_l); } + if (toml.has_key ("vote_cache")) + { + auto config_l = toml.get_required_child ("vote_cache"); + vote_cache.deserialize (config_l); + } + if (toml.has_key ("work_peers")) { work_peers.clear (); diff --git a/nano/node/nodeconfig.hpp b/nano/node/nodeconfig.hpp index daf962c30c..0cc4116270 100644 --- a/nano/node/nodeconfig.hpp +++ b/nano/node/nodeconfig.hpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -124,6 +125,7 @@ class node_config unsigned backlog_scan_batch_size{ 10 * 1000 }; /** Number of times per second to run backlog population batches. Number of accounts per single batch is `backlog_scan_batch_size / backlog_scan_frequency` */ unsigned backlog_scan_frequency{ 10 }; + nano::vote_cache_config vote_cache; public: std::string serialize_frontiers_confirmation (nano::frontiers_confirmation_mode) const; @@ -171,7 +173,6 @@ class node_flags final std::size_t block_processor_batch_size{ 0 }; std::size_t block_processor_full_size{ 65536 }; std::size_t block_processor_verification_size{ 0 }; - std::size_t inactive_votes_cache_size{ 1024 * 128 }; std::size_t vote_processor_capacity{ 144 * 1024 }; std::size_t bootstrap_interval{ 0 }; // For testing only }; diff --git a/nano/node/scheduler/hinted.cpp b/nano/node/scheduler/hinted.cpp index 661e055299..3884ff33d9 100644 --- a/nano/node/scheduler/hinted.cpp +++ b/nano/node/scheduler/hinted.cpp @@ -223,7 +223,7 @@ nano::error nano::scheduler::hinted_config::serialize (nano::tomlconfig & toml) nano::error nano::scheduler::hinted_config::deserialize (nano::tomlconfig & toml) { - toml.get ("hinting_threshold", hinting_threshold_percent); + toml.get ("hinting_threshold", hinting_threshold_percent); auto check_interval_l = check_interval.count (); toml.get ("check_interval", check_interval_l); diff --git a/nano/node/vote_cache.cpp b/nano/node/vote_cache.cpp index c9450240d5..4b3622ad44 100644 --- a/nano/node/vote_cache.cpp +++ b/nano/node/vote_cache.cpp @@ -1,3 +1,4 @@ +#include #include #include @@ -10,7 +11,7 @@ nano::vote_cache::entry::entry (const nano::block_hash & hash) : { } -bool nano::vote_cache::entry::vote (const nano::account & representative, const uint64_t & timestamp, const nano::uint128_t & rep_weight) +bool nano::vote_cache::entry::vote (const nano::account & representative, const uint64_t & timestamp, const nano::uint128_t & rep_weight, std::size_t max_voters) { auto existing = std::find_if (voters_m.begin (), voters_m.end (), [&representative] (auto const & item) { return item.representative == representative; }); if (existing != voters_m.end ()) @@ -95,8 +96,8 @@ std::vector nano::vote_cache::entry::voter * vote_cache */ -nano::vote_cache::vote_cache (const config config_a) : - max_size{ config_a.max_size } +nano::vote_cache::vote_cache (vote_cache_config const & config_a) : + config{ config_a } { } @@ -111,19 +112,19 @@ void nano::vote_cache::vote (const nano::block_hash & hash, const std::shared_pt auto & cache_by_hash = cache.get (); if (auto existing = cache_by_hash.find (hash); existing != cache_by_hash.end ()) { - cache_by_hash.modify (existing, [&representative, ×tamp, &rep_weight] (entry & ent) { - ent.vote (representative, timestamp, rep_weight); + cache_by_hash.modify (existing, [this, &representative, ×tamp, &rep_weight] (entry & ent) { + ent.vote (representative, timestamp, rep_weight, config.max_voters); }); } else { entry cache_entry{ hash }; - cache_entry.vote (representative, timestamp, rep_weight); + cache_entry.vote (representative, timestamp, rep_weight, config.max_voters); cache.get ().insert (cache_entry); // When cache overflown remove the oldest entry - if (cache.size () > max_size) + if (cache.size () > config.max_size) { cache.get ().pop_front (); } @@ -204,4 +205,24 @@ std::unique_ptr nano::vote_cache::collect_contai auto composite = std::make_unique (name); composite->add_component (std::make_unique (container_info{ "cache", size (), sizeof (ordered_cache::value_type) })); return composite; +} + +/* + * vote_cache_config + */ + +nano::error nano::vote_cache_config::serialize (nano::tomlconfig & toml) const +{ + toml.put ("max_size", max_size, "Maximum number of blocks to cache votes for. \ntype:uint64"); + toml.put ("max_voters", max_voters, "Maximum number of voters to cache per block. \ntype:uint64"); + + return toml.get_error (); +} + +nano::error nano::vote_cache_config::deserialize (nano::tomlconfig & toml) +{ + toml.get ("max_size", max_size); + toml.get ("max_voters", max_voters); + + return toml.get_error (); } \ No newline at end of file diff --git a/nano/node/vote_cache.hpp b/nano/node/vote_cache.hpp index 50ebeb4b09..a076981f7f 100644 --- a/nano/node/vote_cache.hpp +++ b/nano/node/vote_cache.hpp @@ -25,16 +25,23 @@ class node; class active_transactions; class election; class vote; +} -class vote_cache final +namespace nano +{ +class vote_cache_config final { public: - class config final - { - public: - std::size_t max_size; - }; + nano::error deserialize (nano::tomlconfig & toml); + nano::error serialize (nano::tomlconfig & toml) const; +public: + std::size_t max_size{ 1024 * 128 }; + std::size_t max_voters{ 128 }; +}; + +class vote_cache final +{ public: /** * Stores votes associated with a single block hash @@ -49,15 +56,13 @@ class vote_cache final }; public: - constexpr static int max_voters = 80; - explicit entry (nano::block_hash const & hash); /** * Adds a vote into a list, checks for duplicates and updates timestamp if new one is greater * @return true if current tally changed, false otherwise */ - bool vote (nano::account const & representative, uint64_t const & timestamp, nano::uint128_t const & rep_weight); + bool vote (nano::account const & representative, uint64_t const & timestamp, nano::uint128_t const & rep_weight, std::size_t max_voters); /** * Inserts votes stored in this entry into an election */ @@ -78,7 +83,7 @@ class vote_cache final }; public: - explicit vote_cache (const config); + explicit vote_cache (vote_cache_config const &); /** * Adds a new vote to cache @@ -119,10 +124,10 @@ class vote_cache final /** * Function used to query rep weight for tally calculation */ - std::function rep_weight_query{ [] (nano::account const & rep) { return 0; } }; + std::function rep_weight_query{ [] (nano::account const & rep) { debug_assert (false); return 0; } }; private: - const std::size_t max_size; + vote_cache_config const & config; // clang-format off class tag_sequenced {}; diff --git a/nano/slow_test/vote_cache.cpp b/nano/slow_test/vote_cache.cpp index b4f5f373b3..05b5b66ed7 100644 --- a/nano/slow_test/vote_cache.cpp +++ b/nano/slow_test/vote_cache.cpp @@ -184,7 +184,7 @@ TEST (vote_cache, perf_singlethreaded) ASSERT_EQ (node.stats.count (nano::stat::type::vote_cache, nano::stat::detail::vote_processed, nano::stat::dir::in), vote_count * single_vote_size * single_vote_reps); // Ensure vote cache size is at max capacity - ASSERT_EQ (node.vote_cache.cache_size (), flags.inactive_votes_cache_size); + ASSERT_EQ (node.vote_cache.size (), config.vote_cache.max_size); } TEST (vote_cache, perf_multithreaded) @@ -247,5 +247,5 @@ TEST (vote_cache, perf_multithreaded) std::cout << "total votes processed: " << node.stats.count (nano::stat::type::vote_cache, nano::stat::detail::vote_processed, nano::stat::dir::in) << std::endl; // Ensure vote cache size is at max capacity - ASSERT_EQ (node.vote_cache.cache_size (), flags.inactive_votes_cache_size); + ASSERT_EQ (node.vote_cache.size (), config.vote_cache.max_size); } From 20f77685ebef75d3b5a7b8d10e5ba1611d298764 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sat, 7 Oct 2023 23:27:21 +0200 Subject: [PATCH 09/13] Fix `active_transactions.inactive_votes_cache_election_start` test --- nano/core_test/active_transactions.cpp | 19 ++++++++++++------- nano/node/scheduler/hinted.cpp | 1 + 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index 095f3ef889..10de180ec6 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -493,23 +493,28 @@ TEST (active_transactions, inactive_votes_cache_election_start) .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) .work (*system.work.generate (send3->hash ())) .build_shared (); + // Inactive votes - std::vector hashes{ open1->hash (), open2->hash (), send4->hash () }; - auto vote1 (std::make_shared (key1.pub, key1.prv, 0, 0, hashes)); + auto vote1 = nano::test::make_vote (key1, { open1, open2, send4 }); node.vote_processor.vote (vote1, std::make_shared (node, node)); ASSERT_TIMELY (5s, node.vote_cache.size () == 3); ASSERT_TRUE (node.active.empty ()); ASSERT_EQ (1, node.ledger.cache.cemented_count); + // 2 votes are required to start election (dev network) - auto vote2 (std::make_shared (key2.pub, key2.prv, 0, 0, hashes)); + auto vote2 = nano::test::make_vote (key2, { open1, open2, send4 }); node.vote_processor.vote (vote2, std::make_shared (node, node)); - // Only open1 & open2 blocks elections should start (send4 is missing previous block in ledger) - ASSERT_TIMELY (5s, 2 == node.active.size ()); + // Only election for send1 should start, other blocks are missing dependencies and don't have enough final weight + ASSERT_TIMELY_EQ (5s, 1, node.active.size ()); + ASSERT_TRUE (node.active.active (send1->hash ())); + // Confirm elections with weight quorum - auto vote0 (std::make_shared (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::vote::timestamp_max, nano::vote::duration_max, hashes)); // Final vote for confirmation + auto vote0 = nano::test::make_final_vote (nano::dev::genesis_key, { open1, open2, send4 }); node.vote_processor.vote (vote0, std::make_shared (node, node)); - ASSERT_TIMELY (5s, node.active.empty ()); + ASSERT_TIMELY_EQ (5s, 0, node.active.size ()); ASSERT_TIMELY (5s, 5 == node.ledger.cache.cemented_count); + ASSERT_TRUE (nano::test::confirmed (node, { send1, send2, open1, open2 })); + // A late block arrival also checks the inactive votes cache ASSERT_TRUE (node.active.empty ()); auto send4_cache (node.vote_cache.find (send4->hash ())); diff --git a/nano/node/scheduler/hinted.cpp b/nano/node/scheduler/hinted.cpp index 3884ff33d9..9b5213c4c9 100644 --- a/nano/node/scheduler/hinted.cpp +++ b/nano/node/scheduler/hinted.cpp @@ -209,6 +209,7 @@ nano::scheduler::hinted_config::hinted_config (nano::network_constants const & n if (network.is_dev_network ()) { check_interval = std::chrono::milliseconds{ 100 }; + block_cooldown = std::chrono::milliseconds{ 100 }; } } From fbe519b816f85d4d3d67da62552043d0377af557 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sun, 8 Oct 2023 14:04:56 +0200 Subject: [PATCH 10/13] Erase vote cache entries when block is confirmed --- nano/node/scheduler/hinted.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/nano/node/scheduler/hinted.cpp b/nano/node/scheduler/hinted.cpp index 9b5213c4c9..2532c45755 100644 --- a/nano/node/scheduler/hinted.cpp +++ b/nano/node/scheduler/hinted.cpp @@ -71,6 +71,7 @@ void nano::scheduler::hinted::activate (const nano::store::transaction & transac if (node.block_confirmed_or_being_confirmed (transaction, current_hash)) { stats.inc (nano::stat::type::hinting, nano::stat::detail::already_confirmed); + vote_cache.erase (current_hash); // Remove from vote cache continue; // Move on to the next item in the stack } From d7c09e38ff354a1d2b8a9c9905371d058c391516 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sun, 8 Oct 2023 14:55:21 +0200 Subject: [PATCH 11/13] Adjust default config values --- nano/node/scheduler/hinted.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nano/node/scheduler/hinted.hpp b/nano/node/scheduler/hinted.hpp index fa09903704..a18d11dcb8 100644 --- a/nano/node/scheduler/hinted.hpp +++ b/nano/node/scheduler/hinted.hpp @@ -35,8 +35,8 @@ class hinted_config final nano::error serialize (nano::tomlconfig & toml) const; public: - std::chrono::milliseconds check_interval{ 5000 }; - std::chrono::milliseconds block_cooldown{ 10000 }; + std::chrono::milliseconds check_interval{ 1000 }; + std::chrono::milliseconds block_cooldown{ 5000 }; unsigned hinting_threshold_percent{ 10 }; }; From 41986ffcc3bf5b560c223bce69a0770deb746830 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 9 Oct 2023 14:00:28 +0200 Subject: [PATCH 12/13] Adjust AEC vacancy notification threshold --- nano/node/scheduler/hinted.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/nano/node/scheduler/hinted.cpp b/nano/node/scheduler/hinted.cpp index 2532c45755..13a36d91bd 100644 --- a/nano/node/scheduler/hinted.cpp +++ b/nano/node/scheduler/hinted.cpp @@ -45,7 +45,12 @@ void nano::scheduler::hinted::stop () void nano::scheduler::hinted::notify () { - condition.notify_all (); + // 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)) + { + condition.notify_all (); + } } bool nano::scheduler::hinted::predicate () const From 91ffaf2d698e31eb70babe7259f10802e71175f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 9 Oct 2023 20:02:16 +0200 Subject: [PATCH 13/13] Remove unnecessary container info test --- nano/rpc_test/rpc.cpp | 39 +-------------------------------------- 1 file changed, 1 insertion(+), 38 deletions(-) diff --git a/nano/rpc_test/rpc.cpp b/nano/rpc_test/rpc.cpp index 59b83a9fe0..6f86112cf7 100644 --- a/nano/rpc_test/rpc.cpp +++ b/nano/rpc_test/rpc.cpp @@ -6934,41 +6934,4 @@ TEST (rpc, confirmation_info) ASSERT_EQ (1, representatives.size ()); ASSERT_EQ (0, response.get ("total_tally")); } -} - -/** Test election scheduler container object stats - * The test set the AEC size to 0 and then creates a block which gets stuck - * in the election scheduler waiting for a slot in the AEC. Then it confirms that - * the stats RPC call shows the corresponding scheduler bucket incrementing - */ -TEST (node, election_scheduler_container_info) -{ - nano::test::system system; - nano::node_config node_config; - node_config.active_elections_size = 0; - nano::node_flags node_flags; - auto node = add_ipc_enabled_node (system, node_config); - auto const rpc_ctx = add_rpc (system, node); - - // create a send block - auto send1 = nano::state_block_builder () - .account (nano::dev::genesis_key.pub) - .previous (nano::dev::genesis->hash ()) - .representative (nano::dev::genesis_key.pub) - .balance (nano::dev::constants.genesis_amount - 1) - .link (nano::public_key ()) - .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) - .work (*node->work_generate_blocking (nano::dev::genesis->hash ())) - .build_shared (); - - // process the block and wait for it to show up in the election scheduler - node->process_active (send1); - ASSERT_TIMELY (10s, node->scheduler.priority.size () == 1); - - // now check the RPC call - boost::property_tree::ptree request; - request.put ("action", "stats"); - request.put ("type", "objects"); - auto response = wait_response (system, rpc_ctx, request); - auto es = response.get_child ("node").get_child ("scheduler"); -} +} \ No newline at end of file