From d241751868e8153c327076c4fe83748b0cf66f7f Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Thu, 9 May 2024 15:41:49 +0100 Subject: [PATCH 1/5] Remove election_behavior argument for manual scheduler insertion. This value was never changed from default. --- nano/node/scheduler/manual.cpp | 4 ++-- nano/node/scheduler/manual.hpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nano/node/scheduler/manual.cpp b/nano/node/scheduler/manual.cpp index 787a0fbd94..85108e6a82 100644 --- a/nano/node/scheduler/manual.cpp +++ b/nano/node/scheduler/manual.cpp @@ -39,10 +39,10 @@ void nano::scheduler::manual::notify () condition.notify_all (); } -void nano::scheduler::manual::push (std::shared_ptr const & block_a, boost::optional const & previous_balance_a, nano::election_behavior election_behavior_a) +void nano::scheduler::manual::push (std::shared_ptr const & block_a, boost::optional const & previous_balance_a) { nano::lock_guard lock{ mutex }; - queue.push_back (std::make_tuple (block_a, previous_balance_a, election_behavior_a)); + queue.push_back (std::make_tuple (block_a, previous_balance_a, nano::election_behavior::normal)); notify (); } diff --git a/nano/node/scheduler/manual.hpp b/nano/node/scheduler/manual.hpp index 947c2fe4d8..ee6c060048 100644 --- a/nano/node/scheduler/manual.hpp +++ b/nano/node/scheduler/manual.hpp @@ -39,7 +39,7 @@ class manual final // Manualy start an election for a block // Call action with confirmed block, may be different than what we started with - void push (std::shared_ptr const &, boost::optional const & = boost::none, nano::election_behavior = nano::election_behavior::normal); + void push (std::shared_ptr const &, boost::optional const & = boost::none); std::unique_ptr collect_container_info (std::string const & name) const; }; // class manual From f9d1b8844ff0a9b843fd10b53e5cddb83fde9ec6 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Thu, 9 May 2024 16:24:49 +0100 Subject: [PATCH 2/5] Splitting manual election_behavior from normal. --- nano/core_test/active_elections.cpp | 6 +++--- nano/core_test/election.cpp | 2 +- nano/lib/stats_enums.hpp | 1 + nano/node/active_elections.cpp | 6 ++++++ nano/node/election.cpp | 2 ++ nano/node/election_behavior.hpp | 1 + nano/node/json_handler.cpp | 5 +++++ nano/node/scheduler/manual.cpp | 2 +- 8 files changed, 20 insertions(+), 5 deletions(-) diff --git a/nano/core_test/active_elections.cpp b/nano/core_test/active_elections.cpp index d36d764a5b..a99feb48d1 100644 --- a/nano/core_test/active_elections.cpp +++ b/nano/core_test/active_elections.cpp @@ -669,7 +669,7 @@ TEST (active_elections, dropped_cleanup) ASSERT_FALSE (node.network.publish_filter.apply (block_bytes.data (), block_bytes.size ())); // An election was recently dropped - ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::normal)); + ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::manual)); // Block cleared from active ASSERT_FALSE (node.vote_router.active (hash)); @@ -687,7 +687,7 @@ TEST (active_elections, dropped_cleanup) ASSERT_TRUE (node.network.publish_filter.apply (block_bytes.data (), block_bytes.size ())); // Not dropped - ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::normal)); + ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::manual)); // Block cleared from active ASSERT_FALSE (node.vote_router.active (hash)); @@ -1418,7 +1418,7 @@ TEST (active_elections, fifo) ASSERT_TIMELY_EQ (5s, node.active.size (), 1); // Ensure overflow stats have been incremented - ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::normal)); + ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::manual)); // Ensure the surviving transaction is the least recently inserted ASSERT_TIMELY (1s, node.active.election (receive2->qualified_root ()) != nullptr); diff --git a/nano/core_test/election.cpp b/nano/core_test/election.cpp index c285e12c6d..7d8bda40d7 100644 --- a/nano/core_test/election.cpp +++ b/nano/core_test/election.cpp @@ -27,7 +27,7 @@ TEST (election, behavior) auto chain = nano::test::setup_chain (system, *system.nodes[0], 1, nano::dev::genesis_key, false); auto election = nano::test::start_election (system, *system.nodes[0], chain[0]->hash ()); ASSERT_NE (nullptr, election); - ASSERT_EQ (nano::election_behavior::normal, election->behavior ()); + ASSERT_EQ (nano::election_behavior::manual, election->behavior ()); } TEST (election, quorum_minimum_flip_success) diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index 616a8a573e..397443c426 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -219,6 +219,7 @@ enum class detail broadcast_block_repeat, // election types + manual, normal, hinted, optimistic, diff --git a/nano/node/active_elections.cpp b/nano/node/active_elections.cpp index 3c223752df..378974d419 100644 --- a/nano/node/active_elections.cpp +++ b/nano/node/active_elections.cpp @@ -187,6 +187,10 @@ int64_t nano::active_elections::limit (nano::election_behavior behavior) const { switch (behavior) { + case nano::election_behavior::manual: + { + return std::numeric_limits::max (); + } case nano::election_behavior::normal: { return static_cast (config.size); @@ -212,6 +216,8 @@ int64_t nano::active_elections::vacancy (nano::election_behavior behavior) const nano::lock_guard guard{ mutex }; switch (behavior) { + case nano::election_behavior::manual: + return std::numeric_limits::max (); case nano::election_behavior::normal: return limit () - static_cast (roots.size ()); case nano::election_behavior::hinted: diff --git a/nano/node/election.cpp b/nano/node/election.cpp index 56415165a1..404458ba0e 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -142,6 +142,7 @@ std::chrono::milliseconds nano::election::confirm_req_time () const { switch (behavior ()) { + case election_behavior::manual: case election_behavior::normal: case election_behavior::hinted: return base_latency () * 5; @@ -295,6 +296,7 @@ std::chrono::milliseconds nano::election::time_to_live () const { switch (behavior ()) { + case election_behavior::manual: case election_behavior::normal: return std::chrono::milliseconds (5 * 60 * 1000); case election_behavior::hinted: diff --git a/nano/node/election_behavior.hpp b/nano/node/election_behavior.hpp index c38b1c8de6..a4f1dee910 100644 --- a/nano/node/election_behavior.hpp +++ b/nano/node/election_behavior.hpp @@ -8,6 +8,7 @@ namespace nano { enum class election_behavior { + manual, normal, /** * Hinted elections: diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp index d1978048ef..9b5f81279e 100644 --- a/nano/node/json_handler.cpp +++ b/nano/node/json_handler.cpp @@ -2009,6 +2009,7 @@ void nano::json_handler::confirmation_active () void nano::json_handler::election_statistics () { auto active_elections = node.active.list_active (); + unsigned manual_count = 0; unsigned normal_count = 0; unsigned hinted_count = 0; unsigned optimistic_count = 0; @@ -2027,6 +2028,9 @@ void nano::json_handler::election_statistics () switch (election->behavior ()) { + case election_behavior::manual: + manual_count++; + break; case election_behavior::normal: normal_count++; break; @@ -2046,6 +2050,7 @@ void nano::json_handler::election_statistics () std::stringstream stream_utilization; stream_utilization << std::fixed << std::setprecision (2) << utilization_percentage; + response_l.put ("manual", manual_count); response_l.put ("normal", normal_count); response_l.put ("hinted", hinted_count); response_l.put ("optimistic", optimistic_count); diff --git a/nano/node/scheduler/manual.cpp b/nano/node/scheduler/manual.cpp index 85108e6a82..3235a91c20 100644 --- a/nano/node/scheduler/manual.cpp +++ b/nano/node/scheduler/manual.cpp @@ -42,7 +42,7 @@ void nano::scheduler::manual::notify () void nano::scheduler::manual::push (std::shared_ptr const & block_a, boost::optional const & previous_balance_a) { nano::lock_guard lock{ mutex }; - queue.push_back (std::make_tuple (block_a, previous_balance_a, nano::election_behavior::normal)); + queue.push_back (std::make_tuple (block_a, previous_balance_a, nano::election_behavior::manual)); notify (); } From 433e0c9d6d1b9dddc354a4d9904c14ca268c247e Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Fri, 10 May 2024 09:26:05 +0100 Subject: [PATCH 3/5] Renaming election_behavior::normal to election_behavior::priority --- nano/core_test/active_elections.cpp | 2 +- nano/core_test/confirmation_solicitor.cpp | 10 +++++----- nano/core_test/election.cpp | 2 +- nano/lib/stats_enums.hpp | 2 +- nano/node/active_elections.cpp | 6 +++--- nano/node/active_elections.hpp | 6 +++--- nano/node/election.cpp | 4 ++-- nano/node/election.hpp | 2 +- nano/node/election_behavior.hpp | 2 +- nano/node/json_handler.cpp | 8 ++++---- nano/rpc_test/rpc.cpp | 2 +- 11 files changed, 23 insertions(+), 23 deletions(-) diff --git a/nano/core_test/active_elections.cpp b/nano/core_test/active_elections.cpp index a99feb48d1..bfc162b4af 100644 --- a/nano/core_test/active_elections.cpp +++ b/nano/core_test/active_elections.cpp @@ -1487,7 +1487,7 @@ TEST (active_elections, limit_vote_hinted_elections) ASSERT_TIMELY (5s, nano::test::active (node, { open1 })); // Ensure there was no overflow of elections - ASSERT_EQ (0, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::normal)); + ASSERT_EQ (0, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::priority)); } /* diff --git a/nano/core_test/confirmation_solicitor.cpp b/nano/core_test/confirmation_solicitor.cpp index f3609a36a6..ca48a2ee8a 100644 --- a/nano/core_test/confirmation_solicitor.cpp +++ b/nano/core_test/confirmation_solicitor.cpp @@ -46,11 +46,11 @@ TEST (confirmation_solicitor, batches) nano::lock_guard guard (node2.active.mutex); for (size_t i (0); i < nano::network::confirm_req_hashes_max; ++i) { - auto election (std::make_shared (node2, send, nullptr, nullptr, nano::election_behavior::normal)); + auto election (std::make_shared (node2, send, nullptr, nullptr, nano::election_behavior::priority)); ASSERT_FALSE (solicitor.add (*election)); } // Reached the maximum amount of requests for the channel - auto election (std::make_shared (node2, send, nullptr, nullptr, nano::election_behavior::normal)); + auto election (std::make_shared (node2, send, nullptr, nullptr, nano::election_behavior::priority)); // Broadcasting should be immediate ASSERT_EQ (0, node2.stats.count (nano::stat::type::message, nano::stat::detail::publish, nano::stat::dir::out)); ASSERT_FALSE (solicitor.broadcast (*election)); @@ -92,7 +92,7 @@ TEST (confirmation_solicitor, different_hash) .work (*system.work.generate (nano::dev::genesis->hash ())) .build (); send->sideband_set ({}); - auto election (std::make_shared (node2, send, nullptr, nullptr, nano::election_behavior::normal)); + auto election (std::make_shared (node2, send, nullptr, nullptr, nano::election_behavior::priority)); // Add a vote for something else, not the winner election->last_votes[representative.account] = { std::chrono::steady_clock::now (), 1, 1 }; // Ensure the request and broadcast goes through @@ -136,7 +136,7 @@ TEST (confirmation_solicitor, bypass_max_requests_cap) .work (*system.work.generate (nano::dev::genesis->hash ())) .build (); send->sideband_set ({}); - auto election (std::make_shared (node2, send, nullptr, nullptr, nano::election_behavior::normal)); + auto election (std::make_shared (node2, send, nullptr, nullptr, nano::election_behavior::priority)); // Add a vote for something else, not the winner for (auto const & rep : representatives) { @@ -149,7 +149,7 @@ TEST (confirmation_solicitor, bypass_max_requests_cap) ASSERT_TIMELY_EQ (6s, max_representatives + 1, node2.stats.count (nano::stat::type::message, nano::stat::detail::confirm_req, nano::stat::dir::out)); solicitor.prepare (representatives); - auto election2 (std::make_shared (node2, send, nullptr, nullptr, nano::election_behavior::normal)); + auto election2 (std::make_shared (node2, send, nullptr, nullptr, nano::election_behavior::priority)); ASSERT_FALSE (solicitor.add (*election2)); ASSERT_FALSE (solicitor.broadcast (*election2)); diff --git a/nano/core_test/election.cpp b/nano/core_test/election.cpp index 7d8bda40d7..82bcb7f131 100644 --- a/nano/core_test/election.cpp +++ b/nano/core_test/election.cpp @@ -18,7 +18,7 @@ TEST (election, construction) nano::test::system system (1); auto & node = *system.nodes[0]; auto election = std::make_shared ( - node, nano::dev::genesis, [] (auto const &) {}, [] (auto const &) {}, nano::election_behavior::normal); + node, nano::dev::genesis, [] (auto const &) {}, [] (auto const &) {}, nano::election_behavior::priority); } TEST (election, behavior) diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index 397443c426..5c6b4f3f84 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -220,7 +220,7 @@ enum class detail // election types manual, - normal, + priority, hinted, optimistic, diff --git a/nano/node/active_elections.cpp b/nano/node/active_elections.cpp index 378974d419..2275e0d586 100644 --- a/nano/node/active_elections.cpp +++ b/nano/node/active_elections.cpp @@ -191,7 +191,7 @@ int64_t nano::active_elections::limit (nano::election_behavior behavior) const { return std::numeric_limits::max (); } - case nano::election_behavior::normal: + case nano::election_behavior::priority: { return static_cast (config.size); } @@ -218,7 +218,7 @@ int64_t nano::active_elections::vacancy (nano::election_behavior behavior) const { case nano::election_behavior::manual: return std::numeric_limits::max (); - case nano::election_behavior::normal: + case nano::election_behavior::priority: return limit () - static_cast (roots.size ()); case nano::election_behavior::hinted: case nano::election_behavior::optimistic: @@ -563,7 +563,7 @@ std::unique_ptr nano::collect_container_info (ac auto composite = std::make_unique (name); composite->add_component (std::make_unique (container_info{ "roots", active_elections.roots.size (), sizeof (decltype (active_elections.roots)::value_type) })); composite->add_component (std::make_unique (container_info{ "election_winner_details", active_elections.election_winner_details_size (), sizeof (decltype (active_elections.election_winner_details)::value_type) })); - composite->add_component (std::make_unique (container_info{ "normal", static_cast (active_elections.count_by_behavior[nano::election_behavior::normal]), 0 })); + composite->add_component (std::make_unique (container_info{ "normal", static_cast (active_elections.count_by_behavior[nano::election_behavior::priority]), 0 })); composite->add_component (std::make_unique (container_info{ "hinted", static_cast (active_elections.count_by_behavior[nano::election_behavior::hinted]), 0 })); composite->add_component (std::make_unique (container_info{ "optimistic", static_cast (active_elections.count_by_behavior[nano::election_behavior::optimistic]), 0 })); diff --git a/nano/node/active_elections.hpp b/nano/node/active_elections.hpp index 5631b8c2f8..26340ea748 100644 --- a/nano/node/active_elections.hpp +++ b/nano/node/active_elections.hpp @@ -108,7 +108,7 @@ class active_elections final /** * Starts new election with a specified behavior type */ - nano::election_insertion_result insert (std::shared_ptr const &, nano::election_behavior = nano::election_behavior::normal); + nano::election_insertion_result insert (std::shared_ptr const &, nano::election_behavior = nano::election_behavior::priority); // Is the root of this block in the roots container bool active (nano::block const &) const; bool active (nano::qualified_root const &) const; @@ -128,11 +128,11 @@ class active_elections final * Maximum number of elections that should be present in this container * NOTE: This is only a soft limit, it is possible for this container to exceed this count */ - int64_t limit (nano::election_behavior behavior = nano::election_behavior::normal) const; + int64_t limit (nano::election_behavior behavior = nano::election_behavior::priority) const; /** * How many election slots are available for specified election type */ - int64_t vacancy (nano::election_behavior behavior = nano::election_behavior::normal) const; + int64_t vacancy (nano::election_behavior behavior = nano::election_behavior::priority) const; std::function vacancy_update{ [] () {} }; std::size_t election_winner_details_size (); diff --git a/nano/node/election.cpp b/nano/node/election.cpp index 404458ba0e..02d6a0cb1a 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -143,7 +143,7 @@ std::chrono::milliseconds nano::election::confirm_req_time () const switch (behavior ()) { case election_behavior::manual: - case election_behavior::normal: + case election_behavior::priority: case election_behavior::hinted: return base_latency () * 5; case election_behavior::optimistic: @@ -297,7 +297,7 @@ std::chrono::milliseconds nano::election::time_to_live () const switch (behavior ()) { case election_behavior::manual: - case election_behavior::normal: + case election_behavior::priority: return std::chrono::milliseconds (5 * 60 * 1000); case election_behavior::hinted: case election_behavior::optimistic: diff --git a/nano/node/election.hpp b/nano/node/election.hpp index 2ebe729b8f..a8ab8c05e5 100644 --- a/nano/node/election.hpp +++ b/nano/node/election.hpp @@ -172,7 +172,7 @@ class election final : public std::enable_shared_from_this mutable nano::uint128_t final_weight{ 0 }; mutable std::unordered_map last_tally; - nano::election_behavior const behavior_m{ nano::election_behavior::normal }; + nano::election_behavior const behavior_m{ nano::election_behavior::priority }; std::chrono::steady_clock::time_point const election_start{ std::chrono::steady_clock::now () }; mutable nano::mutex mutex; diff --git a/nano/node/election_behavior.hpp b/nano/node/election_behavior.hpp index a4f1dee910..1224b1787d 100644 --- a/nano/node/election_behavior.hpp +++ b/nano/node/election_behavior.hpp @@ -9,7 +9,7 @@ namespace nano enum class election_behavior { manual, - normal, + priority, /** * Hinted elections: * - shorter timespan diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp index 9b5f81279e..d8deb88692 100644 --- a/nano/node/json_handler.cpp +++ b/nano/node/json_handler.cpp @@ -2010,7 +2010,7 @@ void nano::json_handler::election_statistics () { auto active_elections = node.active.list_active (); unsigned manual_count = 0; - unsigned normal_count = 0; + unsigned priority_count = 0; unsigned hinted_count = 0; unsigned optimistic_count = 0; unsigned total_count = 0; @@ -2031,8 +2031,8 @@ void nano::json_handler::election_statistics () case election_behavior::manual: manual_count++; break; - case election_behavior::normal: - normal_count++; + case election_behavior::priority: + priority_count++; break; case election_behavior::hinted: hinted_count++; @@ -2051,7 +2051,7 @@ void nano::json_handler::election_statistics () stream_utilization << std::fixed << std::setprecision (2) << utilization_percentage; response_l.put ("manual", manual_count); - response_l.put ("normal", normal_count); + response_l.put ("priority", priority_count); response_l.put ("hinted", hinted_count); response_l.put ("optimistic", optimistic_count); response_l.put ("total", total_count); diff --git a/nano/rpc_test/rpc.cpp b/nano/rpc_test/rpc.cpp index 37e3e66cf2..3093ca9ab6 100644 --- a/nano/rpc_test/rpc.cpp +++ b/nano/rpc_test/rpc.cpp @@ -6962,7 +6962,7 @@ TEST (rpc, election_statistics) request.put ("action", "election_statistics"); auto response = wait_response (system, rpc_ctx, request); - ASSERT_EQ ("1", response.get ("normal")); + ASSERT_EQ ("1", response.get ("priority")); ASSERT_EQ ("0", response.get ("hinted")); ASSERT_EQ ("0", response.get ("optimistic")); ASSERT_EQ ("1", response.get ("total")); From f0a8deae483f9fc5500fc10259a25acb7fea0c02 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Fri, 10 May 2024 09:58:22 +0100 Subject: [PATCH 4/5] Remove default nano::election_behavior arguments --- nano/core_test/active_elections.cpp | 18 +++++++++--------- nano/node/active_elections.cpp | 4 ++-- nano/node/active_elections.hpp | 4 ++-- nano/node/election.hpp | 4 ++-- nano/node/scheduler/manual.hpp | 2 +- nano/node/scheduler/priority.cpp | 2 +- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/nano/core_test/active_elections.cpp b/nano/core_test/active_elections.cpp index bfc162b4af..d110111cc5 100644 --- a/nano/core_test/active_elections.cpp +++ b/nano/core_test/active_elections.cpp @@ -1325,18 +1325,18 @@ TEST (active_elections, vacancy) .build (); node.active.vacancy_update = [&updated] () { updated = true; }; ASSERT_EQ (nano::block_status::progress, node.process (send)); - ASSERT_EQ (1, node.active.vacancy ()); + ASSERT_EQ (1, node.active.vacancy (nano::election_behavior::priority)); ASSERT_EQ (0, node.active.size ()); node.scheduler.priority.activate (node.ledger.tx_begin_read (), nano::dev::genesis_key.pub); ASSERT_TIMELY (1s, updated); updated = false; - ASSERT_EQ (0, node.active.vacancy ()); + ASSERT_EQ (0, node.active.vacancy (nano::election_behavior::priority)); ASSERT_EQ (1, node.active.size ()); auto election1 = node.active.election (send->qualified_root ()); ASSERT_NE (nullptr, election1); election1->force_confirm (); ASSERT_TIMELY (1s, updated); - ASSERT_EQ (1, node.active.vacancy ()); + ASSERT_EQ (1, node.active.vacancy (nano::election_behavior::priority)); ASSERT_EQ (0, node.active.size ()); } } @@ -1521,9 +1521,9 @@ TEST (active_elections, allow_limited_overflow) } // Ensure number of active elections reaches AEC limit and there is no overfill - ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit ()); + ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit (nano::election_behavior::priority)); // And it stays that way without increasing - ASSERT_ALWAYS (1s, node.active.size () == node.active.limit ()); + ASSERT_ALWAYS (1s, node.active.size () == node.active.limit (nano::election_behavior::priority)); // Insert votes for the second part of the blocks, so that those are scheduled as hinted elections for (auto const & block : blocks2) @@ -1534,9 +1534,9 @@ TEST (active_elections, allow_limited_overflow) } // Ensure active elections overfill AEC only up to normal + hinted limit - ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit () + node.active.limit (nano::election_behavior::hinted)); + ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit (nano::election_behavior::priority) + node.active.limit (nano::election_behavior::hinted)); // And it stays that way without increasing - ASSERT_ALWAYS (1s, node.active.size () == node.active.limit () + node.active.limit (nano::election_behavior::hinted)); + ASSERT_ALWAYS (1s, node.active.size () == node.active.limit (nano::election_behavior::priority) + node.active.limit (nano::election_behavior::hinted)); } /* @@ -1583,7 +1583,7 @@ TEST (active_elections, allow_limited_overflow_adapt) } // Ensure number of active elections reaches AEC limit and there is no overfill - ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit ()); + ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit (nano::election_behavior::priority)); // And it stays that way without increasing - ASSERT_ALWAYS (1s, node.active.size () == node.active.limit ()); + ASSERT_ALWAYS (1s, node.active.size () == node.active.limit (nano::election_behavior::priority)); } diff --git a/nano/node/active_elections.cpp b/nano/node/active_elections.cpp index 2275e0d586..88eb6398e4 100644 --- a/nano/node/active_elections.cpp +++ b/nano/node/active_elections.cpp @@ -219,7 +219,7 @@ int64_t nano::active_elections::vacancy (nano::election_behavior behavior) const case nano::election_behavior::manual: return std::numeric_limits::max (); case nano::election_behavior::priority: - return limit () - static_cast (roots.size ()); + return limit (nano::election_behavior::priority) - static_cast (roots.size ()); case nano::election_behavior::hinted: case nano::election_behavior::optimistic: return limit (behavior) - count_by_behavior[behavior]; @@ -372,7 +372,7 @@ void nano::active_elections::trim () * However, it is possible that AEC will be temporarily overfilled in case it's running at full capacity and election hinting or manual queue kicks in. * That case will lead to unwanted churning of elections, so this allows for AEC to be overfilled to 125% until erasing of elections happens. */ - while (vacancy () < -(limit () / 4)) + while (vacancy (nano::election_behavior::priority) < -(limit (nano::election_behavior::priority) / 4)) { node.stats.inc (nano::stat::type::active, nano::stat::detail::erase_oldest); erase_oldest (); diff --git a/nano/node/active_elections.hpp b/nano/node/active_elections.hpp index 26340ea748..5e4b0aff05 100644 --- a/nano/node/active_elections.hpp +++ b/nano/node/active_elections.hpp @@ -128,11 +128,11 @@ class active_elections final * Maximum number of elections that should be present in this container * NOTE: This is only a soft limit, it is possible for this container to exceed this count */ - int64_t limit (nano::election_behavior behavior = nano::election_behavior::priority) const; + int64_t limit (nano::election_behavior behavior) const; /** * How many election slots are available for specified election type */ - int64_t vacancy (nano::election_behavior behavior = nano::election_behavior::priority) const; + int64_t vacancy (nano::election_behavior behavior) const; std::function vacancy_update{ [] () {} }; std::size_t election_winner_details_size (); diff --git a/nano/node/election.hpp b/nano/node/election.hpp index a8ab8c05e5..6111191e2b 100644 --- a/nano/node/election.hpp +++ b/nano/node/election.hpp @@ -3,7 +3,6 @@ #include #include #include -#include #include #include #include @@ -17,6 +16,7 @@ namespace nano class block; class channel; class confirmation_solicitor; +enum class election_behavior; class inactive_cache_information; class node; enum class vote_code; @@ -172,7 +172,7 @@ class election final : public std::enable_shared_from_this mutable nano::uint128_t final_weight{ 0 }; mutable std::unordered_map last_tally; - nano::election_behavior const behavior_m{ nano::election_behavior::priority }; + nano::election_behavior const behavior_m; std::chrono::steady_clock::time_point const election_start{ std::chrono::steady_clock::now () }; mutable nano::mutex mutex; diff --git a/nano/node/scheduler/manual.hpp b/nano/node/scheduler/manual.hpp index ee6c060048..648a7e934c 100644 --- a/nano/node/scheduler/manual.hpp +++ b/nano/node/scheduler/manual.hpp @@ -1,7 +1,6 @@ #pragma once #include #include -#include #include @@ -12,6 +11,7 @@ namespace nano { class block; +enum class election_behavior; class node; } diff --git a/nano/node/scheduler/priority.cpp b/nano/node/scheduler/priority.cpp index 0a91f8e560..d16d33fe56 100644 --- a/nano/node/scheduler/priority.cpp +++ b/nano/node/scheduler/priority.cpp @@ -95,7 +95,7 @@ bool nano::scheduler::priority::empty () const bool nano::scheduler::priority::predicate () const { - return node.active.vacancy () > 0 && !buckets->empty (); + return node.active.vacancy (nano::election_behavior::priority) > 0 && !buckets->empty (); } void nano::scheduler::priority::run () From 53589b34d43ed0b291e1a9cdd9b90b4a17d7c12a Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Fri, 10 May 2024 10:20:10 +0100 Subject: [PATCH 5/5] Remove active_elections::trim Each scheduler checks its own limits with calls to active_elections::vacancy. Trim is problematic as it indiscriminately cancels elections without consideration to why it was scheduled or its priority. --- nano/core_test/active_elections.cpp | 83 ----------------------------- nano/node/active_elections.cpp | 26 --------- nano/node/active_elections.hpp | 3 -- 3 files changed, 112 deletions(-) diff --git a/nano/core_test/active_elections.cpp b/nano/core_test/active_elections.cpp index d110111cc5..9818755d07 100644 --- a/nano/core_test/active_elections.cpp +++ b/nano/core_test/active_elections.cpp @@ -1341,89 +1341,6 @@ TEST (active_elections, vacancy) } } -// Ensure transactions in excess of capacity are removed in fifo order -TEST (active_elections, fifo) -{ - nano::test::system system{}; - - nano::node_config config = system.default_config (); - config.active_elections.size = 1; - config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled; - - auto & node = *system.add_node (config); - auto latest_hash = nano::dev::genesis->hash (); - nano::keypair key0{}; - nano::state_block_builder builder{}; - - // Construct two pending entries that can be received simultaneously - auto send1 = builder.make_block () - .previous (latest_hash) - .account (nano::dev::genesis_key.pub) - .representative (nano::dev::genesis_key.pub) - .link (key0.pub) - .balance (nano::dev::constants.genesis_amount - 1) - .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) - .work (*system.work.generate (latest_hash)) - .build (); - ASSERT_EQ (nano::block_status::progress, node.process (send1)); - node.process_confirmed (nano::election_status{ send1 }); - ASSERT_TIMELY (5s, node.block_confirmed (send1->hash ())); - - nano::keypair key1{}; - latest_hash = send1->hash (); - auto send2 = builder.make_block () - .previous (latest_hash) - .account (nano::dev::genesis_key.pub) - .representative (nano::dev::genesis_key.pub) - .link (key1.pub) - .balance (nano::dev::constants.genesis_amount - 2) - .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) - .work (*system.work.generate (latest_hash)) - .build (); - ASSERT_EQ (nano::block_status::progress, node.process (send2)); - node.process_confirmed (nano::election_status{ send2 }); - ASSERT_TIMELY (5s, node.block_confirmed (send2->hash ())); - - auto receive1 = builder.make_block () - .previous (0) - .account (key0.pub) - .representative (nano::dev::genesis_key.pub) - .link (send1->hash ()) - .balance (1) - .sign (key0.prv, key0.pub) - .work (*system.work.generate (key0.pub)) - .build (); - ASSERT_EQ (nano::block_status::progress, node.process (receive1)); - - auto receive2 = builder.make_block () - .previous (0) - .account (key1.pub) - .representative (nano::dev::genesis_key.pub) - .link (send2->hash ()) - .balance (1) - .sign (key1.prv, key1.pub) - .work (*system.work.generate (key1.pub)) - .build (); - ASSERT_EQ (nano::block_status::progress, node.process (receive2)); - - // Ensure first transaction becomes active - node.scheduler.manual.push (receive1); - ASSERT_TIMELY (5s, node.active.election (receive1->qualified_root ()) != nullptr); - - // Ensure second transaction becomes active - node.scheduler.manual.push (receive2); - ASSERT_TIMELY (5s, node.active.election (receive2->qualified_root ()) != nullptr); - - // Ensure excess transactions get trimmed - ASSERT_TIMELY_EQ (5s, node.active.size (), 1); - - // Ensure overflow stats have been incremented - ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::manual)); - - // Ensure the surviving transaction is the least recently inserted - ASSERT_TIMELY (1s, node.active.election (receive2->qualified_root ()) != nullptr); -} - /* * Ensures we limit the number of vote hinted elections in AEC */ diff --git a/nano/node/active_elections.cpp b/nano/node/active_elections.cpp index 88eb6398e4..b0e3507306 100644 --- a/nano/node/active_elections.cpp +++ b/nano/node/active_elections.cpp @@ -365,20 +365,6 @@ void nano::active_elections::request_loop () } } -void nano::active_elections::trim () -{ - /* - * Both normal and hinted election schedulers are well-behaved, meaning they first check for AEC vacancy before inserting new elections. - * However, it is possible that AEC will be temporarily overfilled in case it's running at full capacity and election hinting or manual queue kicks in. - * That case will lead to unwanted churning of elections, so this allows for AEC to be overfilled to 125% until erasing of elections happens. - */ - while (vacancy (nano::election_behavior::priority) < -(limit (nano::election_behavior::priority) / 4)) - { - node.stats.inc (nano::stat::type::active, nano::stat::detail::erase_oldest); - erase_oldest (); - } -} - nano::election_insertion_result nano::active_elections::insert (std::shared_ptr const & block_a, nano::election_behavior election_behavior_a) { debug_assert (block_a); @@ -449,8 +435,6 @@ nano::election_insertion_result nano::active_elections::insert (std::shared_ptr< result.election->broadcast_vote (); } - trim (); - return result; } @@ -495,16 +479,6 @@ bool nano::active_elections::erase (nano::qualified_root const & root_a) return false; } -void nano::active_elections::erase_oldest () -{ - nano::unique_lock lock{ mutex }; - if (!roots.empty ()) - { - auto item = roots.get ().front (); - cleanup_election (lock, item.election); - } -} - bool nano::active_elections::empty () const { nano::lock_guard lock{ mutex }; diff --git a/nano/node/active_elections.hpp b/nano/node/active_elections.hpp index 5e4b0aff05..869912f5c8 100644 --- a/nano/node/active_elections.hpp +++ b/nano/node/active_elections.hpp @@ -117,7 +117,6 @@ class active_elections final std::vector> list_active (std::size_t = std::numeric_limits::max ()); bool erase (nano::block const &); bool erase (nano::qualified_root const &); - void erase_oldest (); bool empty () const; std::size_t size () const; bool publish (std::shared_ptr const &); @@ -140,8 +139,6 @@ class active_elections final std::shared_ptr remove_election_winner_details (nano::block_hash const &); private: - // Erase elections if we're over capacity - void trim (); void request_loop (); void request_confirm (nano::unique_lock &); // Erase all blocks from active and, if not confirmed, clear digests from network filters