diff --git a/nano/node/node.cpp b/nano/node/node.cpp index cde6186b10..0cbf29c3b3 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -238,7 +238,10 @@ nano::node::node (std::shared_ptr io_ctx_a, std::filesy }); vote_router.vote_processed.add ([this] (std::shared_ptr const & vote, nano::vote_source source, std::unordered_map const & results) { - vote_cache.observe (vote, source, results); + if (source != nano::vote_source::cache) + { + vote_cache.insert (vote, results); + } }); // Republish vote if it is new and the node does not host a principal representative (or close to) diff --git a/nano/node/vote_cache.cpp b/nano/node/vote_cache.cpp index 5fbf905099..3b3e2d978e 100644 --- a/nano/node/vote_cache.cpp +++ b/nano/node/vote_cache.cpp @@ -20,6 +20,9 @@ bool nano::vote_cache_entry::vote (std::shared_ptr const & vote, con bool updated = vote_impl (vote, rep_weight, max_voters); if (updated) { + auto [tally, final_tally] = calculate_tally (); + tally_m = tally; + final_tally_m = final_tally; last_vote_m = std::chrono::steady_clock::now (); } return updated; @@ -36,15 +39,12 @@ bool nano::vote_cache_entry::vote_impl (std::shared_ptr const & vote // 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 (vote->timestamp () > existing->vote->timestamp ()) { + bool was_final = existing->vote->is_final (); voters.modify (existing, [&vote, &rep_weight] (auto & existing) { existing.vote = vote; existing.weight = rep_weight; }); - return true; - } - else - { - return false; + return !was_final && vote->is_final (); // Tally changed only if the vote became final } } else @@ -76,11 +76,8 @@ bool nano::vote_cache_entry::vote_impl (std::shared_ptr const & vote return true; } - else - { - return false; - } } + return false; // Tally unchanged } std::size_t nano::vote_cache_entry::size () const @@ -88,23 +85,15 @@ std::size_t nano::vote_cache_entry::size () const return voters.size (); } -nano::block_hash nano::vote_cache_entry::hash () const -{ - return hash_m; -} - -nano::uint128_t nano::vote_cache_entry::tally () const -{ - return std::accumulate (voters.begin (), voters.end (), nano::uint128_t{ 0 }, [] (auto sum, auto const & item) { - return sum + item.weight; - }); -} - -nano::uint128_t nano::vote_cache_entry::final_tally () const +auto nano::vote_cache_entry::calculate_tally () const -> std::pair { - return std::accumulate (voters.begin (), voters.end (), nano::uint128_t{ 0 }, [] (auto sum, auto const & item) { - return sum + (item.vote->is_final () ? item.weight : 0); - }); + nano::uint128_t tally{ 0 }, final_tally{ 0 }; + for (auto const & voter : voters) + { + tally += voter.weight; + final_tally += voter.vote->is_final () ? voter.weight : 0; + } + return { tally, final_tally }; } std::vector> nano::vote_cache_entry::votes () const @@ -113,11 +102,6 @@ std::vector> nano::vote_cache_entry::votes () const return { r.begin (), r.end () }; } -std::chrono::steady_clock::time_point nano::vote_cache_entry::last_vote () const -{ - return last_vote_m; -} - /* * vote_cache */ @@ -128,61 +112,66 @@ nano::vote_cache::vote_cache (vote_cache_config const & config_a, nano::stats & { } -void nano::vote_cache::observe (const std::shared_ptr & vote, nano::vote_source source, std::unordered_map results) +void nano::vote_cache::insert (std::shared_ptr const & vote, std::unordered_map const & results) { - if (source != nano::vote_source::cache) - { - insert (vote, [&results] (nano::block_hash const & hash) { - // This filters which hashes should be included in the vote cache - if (auto it = results.find (hash); it != results.end ()) - { - auto result = it->second; - // Cache votes with a corresponding active election (indicated by `vote_code::vote`) in case that election gets dropped - return result == nano::vote_code::vote || result == nano::vote_code::indeterminate; - } - debug_assert (false); - return false; - }); - } -} + // Results map should be empty or have the same hashes as the vote + debug_assert (results.empty () || std::all_of (vote->hashes.begin (), vote->hashes.end (), [&results] (auto const & hash) { return results.find (hash) != results.end (); })); -void nano::vote_cache::insert (std::shared_ptr const & vote, std::function filter) -{ auto const representative = vote->account; - auto const timestamp = vote->timestamp (); auto const rep_weight = rep_weight_query (representative); nano::lock_guard lock{ mutex }; - for (auto const & hash : vote->hashes) + // Cache votes with a corresponding active election (indicated by `vote_code::vote`) in case that election gets dropped + auto filter = [] (auto code) { + return code == nano::vote_code::vote || code == nano::vote_code::indeterminate; + }; + + // If results map is empty, insert all hashes (meant for testing) + if (results.empty ()) { - // Using filter callback here to avoid unnecessary relocking when processing large votes - if (!filter (hash)) + for (auto const & hash : vote->hashes) { - continue; + insert_impl (vote, hash, rep_weight); } - - if (auto existing = cache.find (hash); existing != cache.end ()) + } + else + { + for (auto const & [hash, code] : results) { - stats.inc (nano::stat::type::vote_cache, nano::stat::detail::update); - - cache.modify (existing, [this, &vote, &rep_weight] (entry & ent) { - ent.vote (vote, rep_weight, config.max_voters); - }); + if (filter (code)) + { + insert_impl (vote, hash, rep_weight); + } } - else - { - stats.inc (nano::stat::type::vote_cache, nano::stat::detail::insert); + } +} - entry cache_entry{ hash }; - cache_entry.vote (vote, rep_weight, config.max_voters); - cache.insert (cache_entry); +void nano::vote_cache::insert_impl (std::shared_ptr const & vote, nano::block_hash const & hash, nano::uint128_t const & rep_weight) +{ + debug_assert (!mutex.try_lock ()); + debug_assert (std::any_of (vote->hashes.begin (), vote->hashes.end (), [&hash] (auto const & vote_hash) { return vote_hash == hash; })); - // Remove the oldest entry if we have reached the capacity limit - if (cache.size () > config.max_size) - { - cache.get ().pop_front (); - } + if (auto existing = cache.find (hash); existing != cache.end ()) + { + stats.inc (nano::stat::type::vote_cache, nano::stat::detail::update); + + cache.modify (existing, [this, &vote, &rep_weight] (entry & ent) { + ent.vote (vote, rep_weight, config.max_voters); + }); + } + else + { + stats.inc (nano::stat::type::vote_cache, nano::stat::detail::insert); + + entry cache_entry{ hash }; + cache_entry.vote (vote, rep_weight, config.max_voters); + cache.insert (cache_entry); + + // Remove the oldest entry if we have reached the capacity limit + if (cache.size () > config.max_size) + { + cache.get ().pop_front (); } } } @@ -231,11 +220,11 @@ void nano::vote_cache::clear () cache.clear (); } -std::vector nano::vote_cache::top (const nano::uint128_t & min_tally) +std::deque nano::vote_cache::top (const nano::uint128_t & min_tally) { stats.inc (nano::stat::type::vote_cache, nano::stat::detail::top); - std::vector results; + std::deque results; { nano::lock_guard lock{ mutex }; @@ -244,12 +233,14 @@ std::vector nano::vote_cache::top (const nano::uint cleanup (); } - for (auto & entry : cache) + for (auto & entry : cache.get ()) { - if (entry.tally () >= min_tally) + auto tally = entry.tally (); + if (tally < min_tally) { - results.push_back ({ entry.hash (), entry.tally (), entry.final_tally () }); + break; } + results.push_back ({ entry.hash (), tally, entry.final_tally () }); } } diff --git a/nano/node/vote_cache.hpp b/nano/node/vote_cache.hpp index 596183a1b7..2040482948 100644 --- a/nano/node/vote_cache.hpp +++ b/nano/node/vote_cache.hpp @@ -68,14 +68,29 @@ class vote_cache_entry final bool vote (std::shared_ptr const & vote, nano::uint128_t const & rep_weight, std::size_t max_voters); std::size_t size () const; - nano::block_hash hash () const; - nano::uint128_t tally () const; - nano::uint128_t final_tally () const; std::vector> votes () const; - std::chrono::steady_clock::time_point last_vote () const; + +public: // Keep accessors inlined + nano::block_hash hash () const + { + return hash_m; + } + std::chrono::steady_clock::time_point last_vote () const + { + return last_vote_m; + } + nano::uint128_t tally () const + { + return tally_m; + } + nano::uint128_t final_tally () const + { + return final_tally_m; + } private: bool vote_impl (std::shared_ptr const & vote, nano::uint128_t const & rep_weight, std::size_t max_voters); + std::pair calculate_tally () const; // // clang-format off class tag_representative {}; @@ -95,6 +110,8 @@ class vote_cache_entry final nano::block_hash const hash_m; std::chrono::steady_clock::time_point last_vote_m{}; + nano::uint128_t tally_m{ 0 }; + nano::uint128_t final_tally_m{ 0 }; }; class vote_cache final @@ -110,12 +127,7 @@ class vote_cache final */ void insert ( std::shared_ptr const & vote, - std::function filter = [] (nano::block_hash const &) { return true; }); - - /** - * Should be called for every processed vote, filters which votes should be added to cache - */ - void observe (std::shared_ptr const & vote, nano::vote_source source, std::unordered_map); + std::unordered_map const & results = {}); /** * Tries to find an entry associated with block hash @@ -145,7 +157,7 @@ class vote_cache final * 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); + std::deque top (nano::uint128_t const & min_tally); public: // Container info std::unique_ptr collect_container_info (std::string const & name) const; @@ -161,11 +173,13 @@ class vote_cache final nano::stats & stats; private: + void insert_impl (std::shared_ptr const &, nano::block_hash const & hash, nano::uint128_t const & rep_weight); void cleanup (); // clang-format off class tag_sequenced {}; class tag_hash {}; + class tag_tally {}; // clang-format on // clang-format off @@ -173,7 +187,9 @@ class vote_cache final mi::indexed_by< mi::hashed_unique, mi::const_mem_fun>, - mi::sequenced> + mi::sequenced>, + mi::ordered_non_unique, + mi::const_mem_fun, std::greater<>> // DESC >>; // clang-format on ordered_cache cache; diff --git a/nano/node/vote_router.cpp b/nano/node/vote_router.cpp index c26b3b3861..b74fea4b1b 100644 --- a/nano/node/vote_router.cpp +++ b/nano/node/vote_router.cpp @@ -55,42 +55,54 @@ void nano::vote_router::disconnect (nano::block_hash const & hash) } // Validate a vote and apply it to the current election if one exists -std::unordered_map nano::vote_router::vote (std::shared_ptr const & vote, nano::vote_source source) +std::unordered_map nano::vote_router::vote (std::shared_ptr const & vote, nano::vote_source source, nano::block_hash filter) { debug_assert (!vote->validate ()); // false => valid vote + // If present, filter should be set to one of the hashes in the vote + debug_assert (filter.is_zero () || std::any_of (vote->hashes.begin (), vote->hashes.end (), [&filter] (auto const & hash) { + return hash == filter; + })); std::unordered_map results; std::unordered_map> process; - std::vector inactive; // Hashes that should be added to inactive vote cache { std::shared_lock lock{ mutex }; for (auto const & hash : vote->hashes) { + // Ignore votes for other hashes if a filter is set + if (!filter.is_zero () && hash != filter) + { + continue; + } + // Ignore duplicate hashes (should not happen with a well-behaved voting node) if (results.find (hash) != results.end ()) { continue; } - if (auto existing = elections.find (hash); existing != elections.end ()) - { - if (auto election = existing->second.lock (); election != nullptr) + auto find_election = [this] (auto const & hash) { + if (auto existing = elections.find (hash); existing != elections.end ()) { - process[hash] = election; + return existing->second.lock (); } - } - if (process.count (hash) != 0) - { - // There was an active election for hash - } - else if (!recently_confirmed.exists (hash)) + return std::shared_ptr{}; + }; + + if (auto election = find_election (hash)) { - inactive.emplace_back (hash); - results[hash] = nano::vote_code::indeterminate; + process[hash] = election; } else { - results[hash] = nano::vote_code::replay; + if (!recently_confirmed.exists (hash)) + { + results[hash] = nano::vote_code::indeterminate; + } + else + { + results[hash] = nano::vote_code::replay; + } } } } @@ -102,7 +114,7 @@ std::unordered_map nano::vote_router::vote (s } // All hashes should have their result set - debug_assert (std::all_of (vote->hashes.begin (), vote->hashes.end (), [&results] (auto const & hash) { + debug_assert (!filter.is_zero () || std::all_of (vote->hashes.begin (), vote->hashes.end (), [&results] (auto const & hash) { return results.find (hash) != results.end (); })); @@ -116,7 +128,7 @@ bool nano::vote_router::trigger_vote_cache (nano::block_hash const & hash) auto cached = cache.find (hash); for (auto const & cached_vote : cached) { - vote (cached_vote, nano::vote_source::cache); + vote (cached_vote, nano::vote_source::cache, hash); } return !cached.empty (); } diff --git a/nano/node/vote_router.hpp b/nano/node/vote_router.hpp index c66b6ade36..d0a5bd5289 100644 --- a/nano/node/vote_router.hpp +++ b/nano/node/vote_router.hpp @@ -55,7 +55,10 @@ class vote_router final void disconnect (nano::block_hash const & hash); // Route vote to associated elections // Distinguishes replay votes, cannot be determined if the block is not in any election - std::unordered_map vote (std::shared_ptr const &, nano::vote_source = nano::vote_source::live); + + // If 'filter' parameter is non-zero, only elections for the specified hash are notified. + // This eliminates duplicate processing when triggering votes from the vote_cache as the result of a specific election being created. + std::unordered_map vote (std::shared_ptr const &, nano::vote_source = nano::vote_source::live, nano::block_hash filter = { 0 }); bool trigger_vote_cache (nano::block_hash const & hash); bool active (nano::block_hash const & hash) const; std::shared_ptr election (nano::block_hash const & hash) const;