From bbfb7debb2befb3376fcb09892cc62c7e8824353 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sat, 2 Nov 2024 15:59:50 +0100 Subject: [PATCH 01/14] Erase lowest priority accounts first --- nano/lib/stats_enums.hpp | 4 ++-- nano/node/bootstrap/account_sets.cpp | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index 57055653c6..ffb66241b8 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -483,11 +483,11 @@ enum class detail next_frontier, blocking_insert, - blocking_erase_overflow, + blocking_overflow, priority_insert, priority_erase_by_threshold, priority_erase_by_blocking, - priority_erase_overflow, + priority_overflow, deprioritize, deprioritize_failed, sync_dependencies, diff --git a/nano/node/bootstrap/account_sets.cpp b/nano/node/bootstrap/account_sets.cpp index 00b522a26e..1ecd1d8900 100644 --- a/nano/node/bootstrap/account_sets.cpp +++ b/nano/node/bootstrap/account_sets.cpp @@ -212,17 +212,17 @@ void nano::bootstrap::account_sets::dependency_update (nano::block_hash const & void nano::bootstrap::account_sets::trim_overflow () { - while (priorities.size () > config.priorities_max) + while (!priorities.empty () && priorities.size () > config.priorities_max) { - // Erase the oldest entry - priorities.pop_front (); - stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::priority_erase_overflow); + // Erase the lowest priority entry + stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::priority_overflow); + priorities.get ().erase (std::prev (priorities.get ().end ())); } - while (blocking.size () > config.blocking_max) + while (!blocking.empty () && blocking.size () > config.blocking_max) { - // Erase the oldest entry + // Erase the lowest priority entry + stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::blocking_overflow); blocking.pop_front (); - stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::blocking_erase_overflow); } } From 026ab525113e04d8107754a121ea8e2763cce82f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sat, 2 Nov 2024 16:17:16 +0100 Subject: [PATCH 02/14] Use lowest priority for frontier accounts --- nano/node/bootstrap/account_sets.cpp | 4 ++-- nano/node/bootstrap/account_sets.hpp | 16 ++++++++-------- nano/node/bootstrap/bootstrap_service.cpp | 3 ++- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/nano/node/bootstrap/account_sets.cpp b/nano/node/bootstrap/account_sets.cpp index 1ecd1d8900..6a796188b1 100644 --- a/nano/node/bootstrap/account_sets.cpp +++ b/nano/node/bootstrap/account_sets.cpp @@ -81,7 +81,7 @@ void nano::bootstrap::account_sets::priority_down (nano::account const & account } } -void nano::bootstrap::account_sets::priority_set (nano::account const & account) +void nano::bootstrap::account_sets::priority_set (nano::account const & account, double priority) { if (account.is_zero ()) { @@ -94,7 +94,7 @@ void nano::bootstrap::account_sets::priority_set (nano::account const & account) if (iter == priorities.get ().end ()) { stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::priority_insert); - priorities.get ().insert ({ account, account_sets::priority_initial }); + priorities.get ().insert ({ account, priority }); trim_overflow (); } } diff --git a/nano/node/bootstrap/account_sets.hpp b/nano/node/bootstrap/account_sets.hpp index 5680600e41..9e04885c43 100644 --- a/nano/node/bootstrap/account_sets.hpp +++ b/nano/node/bootstrap/account_sets.hpp @@ -24,6 +24,13 @@ namespace bootstrap /** This class tracks accounts various account sets which are shared among the multiple bootstrap threads */ class account_sets { + public: // Constants + static double constexpr priority_initial = 2.0; + static double constexpr priority_increase = 2.0; + static double constexpr priority_divide = 2.0; + static double constexpr priority_max = 128.0; + static double constexpr priority_cutoff = 0.15; + public: account_sets (account_sets_config const &, nano::stats &); @@ -38,7 +45,7 @@ namespace bootstrap * Current implementation divides priority by 2.0f and saturates down to 1.0f. */ void priority_down (nano::account const & account); - void priority_set (nano::account const & account); + void priority_set (nano::account const & account, double priority = priority_initial); void block (nano::account const & account, nano::block_hash const & dependency); void unblock (nano::account const & account, std::optional const & hash = std::nullopt); @@ -148,13 +155,6 @@ namespace bootstrap ordered_priorities priorities; ordered_blocking blocking; - public: // Constants - static double constexpr priority_initial = 2.0; - static double constexpr priority_increase = 2.0; - static double constexpr priority_divide = 2.0; - static double constexpr priority_max = 128.0; - static double constexpr priority_cutoff = 0.15; - public: using info_t = std::tuple; // info_t info () const; diff --git a/nano/node/bootstrap/bootstrap_service.cpp b/nano/node/bootstrap/bootstrap_service.cpp index 1f54cacb31..98ea07c9c0 100644 --- a/nano/node/bootstrap/bootstrap_service.cpp +++ b/nano/node/bootstrap/bootstrap_service.cpp @@ -981,7 +981,8 @@ void nano::bootstrap_service::process_frontiers (std::deque Date: Sat, 2 Nov 2024 16:44:04 +0100 Subject: [PATCH 03/14] Use lowest priority for dependency scan --- nano/lib/stats_enums.hpp | 5 +++-- nano/node/bootstrap/account_sets.cpp | 6 +++--- nano/node/bootstrap/bootstrap_service.cpp | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index ffb66241b8..5367bc8684 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -485,8 +485,9 @@ enum class detail blocking_insert, blocking_overflow, priority_insert, - priority_erase_by_threshold, - priority_erase_by_blocking, + priority_set, + erase_by_threshold, + erase_by_blocking, priority_overflow, deprioritize, deprioritize_failed, diff --git a/nano/node/bootstrap/account_sets.cpp b/nano/node/bootstrap/account_sets.cpp index 6a796188b1..39857cead2 100644 --- a/nano/node/bootstrap/account_sets.cpp +++ b/nano/node/bootstrap/account_sets.cpp @@ -65,7 +65,7 @@ void nano::bootstrap::account_sets::priority_down (nano::account const & account auto priority_new = iter->priority / account_sets::priority_divide; if (priority_new <= account_sets::priority_cutoff) { - stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::priority_erase_by_threshold); + stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::erase_by_threshold); priorities.get ().erase (iter); } else @@ -93,7 +93,7 @@ void nano::bootstrap::account_sets::priority_set (nano::account const & account, auto iter = priorities.get ().find (account); if (iter == priorities.get ().end ()) { - stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::priority_insert); + stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::priority_set); priorities.get ().insert ({ account, priority }); trim_overflow (); } @@ -114,7 +114,7 @@ void nano::bootstrap::account_sets::block (nano::account const & account, nano:: auto entry = (existing == priorities.get ().end ()) ? priority_entry{ account, 0 } : *existing; priorities.get ().erase (account); - stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::priority_erase_by_blocking); + stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::erase_by_blocking); blocking.get ().insert ({ entry, dependency }); stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::blocking_insert); diff --git a/nano/node/bootstrap/bootstrap_service.cpp b/nano/node/bootstrap/bootstrap_service.cpp index 98ea07c9c0..17fdf2a308 100644 --- a/nano/node/bootstrap/bootstrap_service.cpp +++ b/nano/node/bootstrap/bootstrap_service.cpp @@ -843,7 +843,7 @@ void nano::bootstrap_service::process (const nano::asc_pull_ack::account_info_pa { nano::lock_guard lock{ mutex }; accounts.dependency_update (tag.hash, response.account); - accounts.priority_set (response.account); + accounts.priority_set (response.account, nano::bootstrap::account_sets::priority_cutoff); // Use the lowest possible priority here } } From e9608c89e530b0d2647610ba0f6f1d90fa965ddc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sun, 3 Nov 2024 10:30:52 +0100 Subject: [PATCH 04/14] Track fails --- nano/core_test/bootstrap.cpp | 59 +++++++++++++++++----------- nano/node/bootstrap/account_sets.cpp | 24 +++++------ nano/node/bootstrap/account_sets.hpp | 4 +- 3 files changed, 49 insertions(+), 38 deletions(-) diff --git a/nano/core_test/bootstrap.cpp b/nano/core_test/bootstrap.cpp index 78557b58f4..2c13390a16 100644 --- a/nano/core_test/bootstrap.cpp +++ b/nano/core_test/bootstrap.cpp @@ -27,11 +27,13 @@ nano::block_hash random_hash () } } +/* + * account_sets + */ + TEST (account_sets, construction) { nano::test::system system; - auto store = nano::make_store (system.logger, nano::unique_path (), nano::dev::constants); - ASSERT_FALSE (store->init_error ()); nano::account_sets_config config; nano::bootstrap::account_sets sets{ config, system.stats }; } @@ -41,8 +43,6 @@ TEST (account_sets, empty_blocked) nano::test::system system; nano::account account{ 1 }; - auto store = nano::make_store (system.logger, nano::unique_path (), nano::dev::constants); - ASSERT_FALSE (store->init_error ()); nano::account_sets_config config; nano::bootstrap::account_sets sets{ config, system.stats }; ASSERT_FALSE (sets.blocked (account)); @@ -53,8 +53,6 @@ TEST (account_sets, block) nano::test::system system; nano::account account{ 1 }; - auto store = nano::make_store (system.logger, nano::unique_path (), nano::dev::constants); - ASSERT_FALSE (store->init_error ()); nano::account_sets_config config; nano::bootstrap::account_sets sets{ config, system.stats }; sets.block (account, random_hash ()); @@ -66,8 +64,6 @@ TEST (account_sets, unblock) nano::test::system system; nano::account account{ 1 }; - auto store = nano::make_store (system.logger, nano::unique_path (), nano::dev::constants); - ASSERT_FALSE (store->init_error ()); nano::account_sets_config config; nano::bootstrap::account_sets sets{ config, system.stats }; auto hash = random_hash (); @@ -81,8 +77,6 @@ TEST (account_sets, priority_base) nano::test::system system; nano::account account{ 1 }; - auto store = nano::make_store (system.logger, nano::unique_path (), nano::dev::constants); - ASSERT_FALSE (store->init_error ()); nano::account_sets_config config; nano::bootstrap::account_sets sets{ config, system.stats }; ASSERT_EQ (0.0, sets.priority (account)); @@ -93,8 +87,6 @@ TEST (account_sets, priority_blocked) nano::test::system system; nano::account account{ 1 }; - auto store = nano::make_store (system.logger, nano::unique_path (), nano::dev::constants); - ASSERT_FALSE (store->init_error ()); nano::account_sets_config config; nano::bootstrap::account_sets sets{ config, system.stats }; sets.block (account, random_hash ()); @@ -107,8 +99,6 @@ TEST (account_sets, priority_unblock_keep) nano::test::system system; nano::account account{ 1 }; - auto store = nano::make_store (system.logger, nano::unique_path (), nano::dev::constants); - ASSERT_FALSE (store->init_error ()); nano::account_sets_config config; nano::bootstrap::account_sets sets{ config, system.stats }; sets.priority_up (account); @@ -126,37 +116,58 @@ TEST (account_sets, priority_up_down) nano::test::system system; nano::account account{ 1 }; - auto store = nano::make_store (system.logger, nano::unique_path (), nano::dev::constants); - ASSERT_FALSE (store->init_error ()); nano::account_sets_config config; nano::bootstrap::account_sets sets{ config, system.stats }; sets.priority_up (account); ASSERT_EQ (sets.priority (account), nano::bootstrap::account_sets::priority_initial); sets.priority_down (account); - ASSERT_EQ (sets.priority (account), nano::bootstrap::account_sets::priority_initial / nano::bootstrap::account_sets::priority_divide); + ASSERT_EQ (sets.priority (account), nano::bootstrap::account_sets::priority_initial); } -TEST (account_sets, priority_down_sat) +TEST (account_sets, priority_down_empty) { nano::test::system system; nano::account account{ 1 }; - auto store = nano::make_store (system.logger, nano::unique_path (), nano::dev::constants); - ASSERT_FALSE (store->init_error ()); nano::account_sets_config config; nano::bootstrap::account_sets sets{ config, system.stats }; sets.priority_down (account); ASSERT_EQ (0.0, sets.priority (account)); } +TEST (account_sets, priority_down_saturate) +{ + nano::test::system system; + + nano::account account{ 1 }; + nano::account_sets_config config; + nano::bootstrap::account_sets sets{ config, system.stats }; + sets.priority_up (account); + ASSERT_EQ (sets.priority (account), nano::bootstrap::account_sets::priority_initial); + for (int n = 0; n < 1000; ++n) + { + sets.priority_down (account); + } + ASSERT_FALSE (sets.prioritized (account)); +} + +TEST (account_sets, priority_set) +{ + nano::test::system system; + + nano::account account{ 1 }; + nano::account_sets_config config; + nano::bootstrap::account_sets sets{ config, system.stats }; + sets.priority_set (account, 10.0); + ASSERT_EQ (sets.priority (account), 10.0); +} + // Ensure priority value is bounded TEST (account_sets, saturate_priority) { nano::test::system system; nano::account account{ 1 }; - auto store = nano::make_store (system.logger, nano::unique_path (), nano::dev::constants); - ASSERT_FALSE (store->init_error ()); nano::account_sets_config config; nano::bootstrap::account_sets sets{ config, system.stats }; for (int n = 0; n < 1000; ++n) @@ -166,6 +177,10 @@ TEST (account_sets, saturate_priority) ASSERT_EQ (sets.priority (account), nano::bootstrap::account_sets::priority_max); } +/* + * bootstrap + */ + /** * Tests the base case for returning */ diff --git a/nano/node/bootstrap/account_sets.cpp b/nano/node/bootstrap/account_sets.cpp index 39857cead2..3564f41671 100644 --- a/nano/node/bootstrap/account_sets.cpp +++ b/nano/node/bootstrap/account_sets.cpp @@ -30,11 +30,11 @@ void nano::bootstrap::account_sets::priority_up (nano::account const & account) { stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::prioritize); - auto iter = priorities.get ().find (account); - if (iter != priorities.get ().end ()) + if (auto it = priorities.get ().find (account); it != priorities.get ().end ()) { - priorities.get ().modify (iter, [] (auto & val) { + priorities.get ().modify (it, [] (auto & val) { val.priority = std::min ((val.priority + account_sets::priority_increase), account_sets::priority_max); + val.fails = 0; }); } else @@ -57,21 +57,19 @@ void nano::bootstrap::account_sets::priority_down (nano::account const & account return; } - auto iter = priorities.get ().find (account); - if (iter != priorities.get ().end ()) + if (auto it = priorities.get ().find (account); it != priorities.get ().end ()) { stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::deprioritize); - auto priority_new = iter->priority / account_sets::priority_divide; - if (priority_new <= account_sets::priority_cutoff) + if (it->fails >= account_sets::max_fails || it->fails >= it->priority) { stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::erase_by_threshold); - priorities.get ().erase (iter); + priorities.get ().erase (it); } else { - priorities.get ().modify (iter, [priority_new] (auto & val) { - val.priority = priority_new; + priorities.get ().modify (it, [] (auto & val) { + val.fails += 1; }); } } @@ -90,8 +88,7 @@ void nano::bootstrap::account_sets::priority_set (nano::account const & account, if (!blocked (account)) { - auto iter = priorities.get ().find (account); - if (iter == priorities.get ().end ()) + if (!priorities.get ().contains (account)) { stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::priority_set); priorities.get ().insert ({ account, priority }); @@ -332,8 +329,7 @@ double nano::bootstrap::account_sets::priority (nano::account const & account) c { if (!blocked (account)) { - auto existing = priorities.get ().find (account); - if (existing != priorities.get ().end ()) + if (auto existing = priorities.get ().find (account); existing != priorities.get ().end ()) { return existing->priority; } diff --git a/nano/node/bootstrap/account_sets.hpp b/nano/node/bootstrap/account_sets.hpp index 9e04885c43..061da0fe5e 100644 --- a/nano/node/bootstrap/account_sets.hpp +++ b/nano/node/bootstrap/account_sets.hpp @@ -30,6 +30,7 @@ namespace bootstrap static double constexpr priority_divide = 2.0; static double constexpr priority_max = 128.0; static double constexpr priority_cutoff = 0.15; + static unsigned constexpr max_fails = 2; public: account_sets (account_sets_config const &, nano::stats &); @@ -93,7 +94,7 @@ namespace bootstrap { nano::account account; double priority; - + unsigned fails{ 0 }; id_t id{ generate_id () }; // Uniformly distributed, used for random querying std::chrono::steady_clock::time_point timestamp{}; }; @@ -103,7 +104,6 @@ namespace bootstrap priority_entry original_entry; nano::block_hash dependency; nano::account dependency_account{ 0 }; - id_t id{ generate_id () }; // Uniformly distributed, used for random querying nano::account account () const From f14154507ea9c824aa4bd43525288f2831ed347d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 4 Nov 2024 20:36:03 +0100 Subject: [PATCH 05/14] Cleanup --- nano/lib/stats_enums.hpp | 3 +++ nano/lib/thread_roles.cpp | 2 +- nano/lib/thread_roles.hpp | 2 +- nano/node/bootstrap/account_sets.cpp | 4 +++- nano/node/bootstrap/bootstrap_service.cpp | 18 +++++++++--------- nano/node/bootstrap/bootstrap_service.hpp | 9 +++++---- 6 files changed, 22 insertions(+), 16 deletions(-) diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index 5367bc8684..3a8da93a8c 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -145,6 +145,7 @@ enum class detail retry, prioritized, pending, + sync, // processing queue queue, @@ -459,6 +460,7 @@ enum class detail timestamp_reset, processing_frontiers, frontiers_dropped, + sync_accounts, prioritize, prioritize_failed, @@ -492,6 +494,7 @@ enum class detail deprioritize, deprioritize_failed, sync_dependencies, + dependency_synced, request_blocks, request_account_info, diff --git a/nano/lib/thread_roles.cpp b/nano/lib/thread_roles.cpp index ab5a0a1a04..efc6aa232d 100644 --- a/nano/lib/thread_roles.cpp +++ b/nano/lib/thread_roles.cpp @@ -106,7 +106,7 @@ std::string nano::thread_role::get_string (nano::thread_role::name role) case nano::thread_role::name::bootstrap_database_scan: thread_role_name_string = "Bootstrap db"; break; - case nano::thread_role::name::bootstrap_dependendy_walker: + case nano::thread_role::name::bootstrap_dependency_walker: thread_role_name_string = "Bootstrap walkr"; break; case nano::thread_role::name::bootstrap_frontier_scan: diff --git a/nano/lib/thread_roles.hpp b/nano/lib/thread_roles.hpp index 57c32f8925..04325743e9 100644 --- a/nano/lib/thread_roles.hpp +++ b/nano/lib/thread_roles.hpp @@ -41,7 +41,7 @@ enum class name telemetry, bootstrap, bootstrap_database_scan, - bootstrap_dependendy_walker, + bootstrap_dependency_walker, bootstrap_frontier_scan, bootstrap_cleanup, bootstrap_worker, diff --git a/nano/node/bootstrap/account_sets.cpp b/nano/node/bootstrap/account_sets.cpp index 3564f41671..4093644f5d 100644 --- a/nano/node/bootstrap/account_sets.cpp +++ b/nano/node/bootstrap/account_sets.cpp @@ -272,6 +272,8 @@ nano::block_hash nano::bootstrap::account_sets::next_blocking (std::function account 0) auto begin = blocking.get ().upper_bound (nano::account{ 0 }); auto end = blocking.get ().end (); @@ -287,7 +289,7 @@ void nano::bootstrap::account_sets::sync_dependencies () if (!blocked (entry.dependency_account) && !prioritized (entry.dependency_account)) { - stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::sync_dependencies); + stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::dependency_synced); priority_set (entry.dependency_account); } } diff --git a/nano/node/bootstrap/bootstrap_service.cpp b/nano/node/bootstrap/bootstrap_service.cpp index 17fdf2a308..cb314839be 100644 --- a/nano/node/bootstrap/bootstrap_service.cpp +++ b/nano/node/bootstrap/bootstrap_service.cpp @@ -60,7 +60,7 @@ nano::bootstrap_service::~bootstrap_service () debug_assert (!database_thread.joinable ()); debug_assert (!dependencies_thread.joinable ()); debug_assert (!frontiers_thread.joinable ()); - debug_assert (!timeout_thread.joinable ()); + debug_assert (!cleanup_thread.joinable ()); debug_assert (!workers.alive ()); } @@ -70,7 +70,7 @@ void nano::bootstrap_service::start () debug_assert (!database_thread.joinable ()); debug_assert (!dependencies_thread.joinable ()); debug_assert (!frontiers_thread.joinable ()); - debug_assert (!timeout_thread.joinable ()); + debug_assert (!cleanup_thread.joinable ()); if (!config.enable) { @@ -99,7 +99,7 @@ void nano::bootstrap_service::start () if (config.enable_dependency_walker) { dependencies_thread = std::thread ([this] () { - nano::thread_role::set (nano::thread_role::name::bootstrap_dependendy_walker); + nano::thread_role::set (nano::thread_role::name::bootstrap_dependency_walker); run_dependencies (); }); } @@ -112,7 +112,7 @@ void nano::bootstrap_service::start () }); } - timeout_thread = std::thread ([this] () { + cleanup_thread = std::thread ([this] () { nano::thread_role::set (nano::thread_role::name::bootstrap_cleanup); run_timeouts (); }); @@ -130,7 +130,7 @@ void nano::bootstrap_service::stop () nano::join_or_pass (database_thread); nano::join_or_pass (dependencies_thread); nano::join_or_pass (frontiers_thread); - nano::join_or_pass (timeout_thread); + nano::join_or_pass (cleanup_thread); workers.stop (); } @@ -418,7 +418,7 @@ nano::block_hash nano::bootstrap_service::next_blocking () debug_assert (!mutex.try_lock ()); auto blocking = accounts.next_blocking ([this] (nano::block_hash const & hash) { - return count_tags (hash, query_source::blocking) == 0; + return count_tags (hash, query_source::dependencies) == 0; }); if (blocking.is_zero ()) { @@ -590,7 +590,7 @@ void nano::bootstrap_service::run_database () } } -void nano::bootstrap_service::run_one_blocking () +void nano::bootstrap_service::run_one_dependency () { // No need to wait for blockprocessor, as we are not processing blocks auto channel = wait_channel (); @@ -603,7 +603,7 @@ void nano::bootstrap_service::run_one_blocking () { return; } - request_info (blocking, channel, query_source::blocking); + request_info (blocking, channel, query_source::dependencies); } void nano::bootstrap_service::run_dependencies () @@ -613,7 +613,7 @@ void nano::bootstrap_service::run_dependencies () { lock.unlock (); stats.inc (nano::stat::type::bootstrap, nano::stat::detail::loop_dependencies); - run_one_blocking (); + run_one_dependency (); lock.lock (); } } diff --git a/nano/node/bootstrap/bootstrap_service.hpp b/nano/node/bootstrap/bootstrap_service.hpp index 8cd7ac651e..dbeea117dd 100644 --- a/nano/node/bootstrap/bootstrap_service.hpp +++ b/nano/node/bootstrap/bootstrap_service.hpp @@ -76,7 +76,7 @@ class bootstrap_service invalid, priority, database, - blocking, + dependencies, frontiers, }; @@ -104,9 +104,9 @@ class bootstrap_service void run_database (); void run_one_database (bool should_throttle); void run_dependencies (); - void run_one_blocking (); - void run_one_frontier (); + void run_one_dependency (); void run_frontiers (); + void run_one_frontier (); void run_timeouts (); void cleanup_and_sync (); @@ -194,6 +194,7 @@ class bootstrap_service // Requests for accounts from database have much lower hitrate and could introduce strain on the network // A separate (lower) limiter ensures that we always reserve resources for querying accounts from priority queue nano::rate_limiter database_limiter; + // Rate limiter for frontier requests nano::rate_limiter frontiers_limiter; nano::interval sync_dependencies_interval; @@ -205,7 +206,7 @@ class bootstrap_service std::thread database_thread; std::thread dependencies_thread; std::thread frontiers_thread; - std::thread timeout_thread; + std::thread cleanup_thread; nano::thread_pool workers; nano::random_generator_mt rng; From d22bb9535de8471f1cdeae626bef9475e9ce8bce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 4 Nov 2024 21:19:58 +0100 Subject: [PATCH 06/14] Propagate invalid responses to peer scoring --- nano/lib/stats_enums.hpp | 1 + nano/node/bootstrap/bootstrap_service.cpp | 33 ++++++++++++++++------- nano/node/bootstrap/bootstrap_service.hpp | 8 +++--- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index 3a8da93a8c..754790e7f1 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -457,6 +457,7 @@ enum class detail loop_frontiers_processing, duplicate_request, invalid_response_type, + invalid_response, timestamp_reset, processing_frontiers, frontiers_dropped, diff --git a/nano/node/bootstrap/bootstrap_service.cpp b/nano/node/bootstrap/bootstrap_service.cpp index cb314839be..35d5f8a61c 100644 --- a/nano/node/bootstrap/bootstrap_service.cpp +++ b/nano/node/bootstrap/bootstrap_service.cpp @@ -746,17 +746,25 @@ void nano::bootstrap_service::process (nano::asc_pull_ack const & message, std:: stats.inc (nano::stat::type::bootstrap_reply, to_stat_detail (tag.type)); stats.sample (nano::stat::sample::bootstrap_tag_duration, nano::log::milliseconds_delta (tag.timestamp), { 0, config.request_timeout.count () }); - scoring.received_message (channel); - lock.unlock (); // Process the response payload - std::visit ([this, &tag] (auto && request) { return process (request, tag); }, message.payload); + bool ok = std::visit ([this, &tag] (auto && request) { return process (request, tag); }, message.payload); + if (ok) + { + lock.lock (); + scoring.received_message (channel); + lock.unlock (); + } + else + { + stats.inc (nano::stat::type::bootstrap, nano::stat::detail::invalid_response); + } condition.notify_all (); } -void nano::bootstrap_service::process (const nano::asc_pull_ack::blocks_payload & response, const async_tag & tag) +bool nano::bootstrap_service::process (const nano::asc_pull_ack::blocks_payload & response, const async_tag & tag) { debug_assert (tag.type == query_type::blocks_by_hash || tag.type == query_type::blocks_by_account); @@ -824,9 +832,11 @@ void nano::bootstrap_service::process (const nano::asc_pull_ack::blocks_payload } break; } + + return result != verify_result::invalid; } -void nano::bootstrap_service::process (const nano::asc_pull_ack::account_info_payload & response, const async_tag & tag) +bool nano::bootstrap_service::process (const nano::asc_pull_ack::account_info_payload & response, const async_tag & tag) { debug_assert (tag.type == query_type::account_info_by_hash); debug_assert (!tag.hash.is_zero ()); @@ -834,7 +844,7 @@ void nano::bootstrap_service::process (const nano::asc_pull_ack::account_info_pa if (response.account.is_zero ()) { stats.inc (nano::stat::type::bootstrap_process, nano::stat::detail::account_info_empty); - return; + return true; // OK, but nothing to do } stats.inc (nano::stat::type::bootstrap_process, nano::stat::detail::account_info); @@ -845,9 +855,11 @@ void nano::bootstrap_service::process (const nano::asc_pull_ack::account_info_pa accounts.dependency_update (tag.hash, response.account); accounts.priority_set (response.account, nano::bootstrap::account_sets::priority_cutoff); // Use the lowest possible priority here } + + return true; // OK, no way to verify the response } -void nano::bootstrap_service::process (const nano::asc_pull_ack::frontiers_payload & response, const async_tag & tag) +bool nano::bootstrap_service::process (const nano::asc_pull_ack::frontiers_payload & response, const async_tag & tag) { debug_assert (tag.type == query_type::frontiers); debug_assert (!tag.start.is_zero ()); @@ -855,7 +867,7 @@ void nano::bootstrap_service::process (const nano::asc_pull_ack::frontiers_paylo if (response.frontiers.empty ()) { stats.inc (nano::stat::type::bootstrap_process, nano::stat::detail::frontiers_empty); - return; + return true; // OK, but nothing to do } stats.inc (nano::stat::type::bootstrap_process, nano::stat::detail::frontiers); @@ -897,12 +909,15 @@ void nano::bootstrap_service::process (const nano::asc_pull_ack::frontiers_paylo } break; } + + return result != verify_result::invalid; } -void nano::bootstrap_service::process (const nano::empty_payload & response, const async_tag & tag) +bool nano::bootstrap_service::process (const nano::empty_payload & response, const async_tag & tag) { stats.inc (nano::stat::type::bootstrap_process, nano::stat::detail::empty); debug_assert (false, "empty payload"); // Should not happen + return false; // Invalid } void nano::bootstrap_service::process_frontiers (std::deque> const & frontiers) diff --git a/nano/node/bootstrap/bootstrap_service.hpp b/nano/node/bootstrap/bootstrap_service.hpp index dbeea117dd..1959df41e1 100644 --- a/nano/node/bootstrap/bootstrap_service.hpp +++ b/nano/node/bootstrap/bootstrap_service.hpp @@ -134,10 +134,10 @@ class bootstrap_service bool request_frontiers (nano::account, std::shared_ptr const &, query_source); bool send (std::shared_ptr const &, async_tag tag); - void process (nano::asc_pull_ack::blocks_payload const & response, async_tag const & tag); - void process (nano::asc_pull_ack::account_info_payload const & response, async_tag const & tag); - void process (nano::asc_pull_ack::frontiers_payload const & response, async_tag const & tag); - void process (nano::empty_payload const & response, async_tag const & tag); + bool process (nano::asc_pull_ack::blocks_payload const & response, async_tag const & tag); + bool process (nano::asc_pull_ack::account_info_payload const & response, async_tag const & tag); + bool process (nano::asc_pull_ack::frontiers_payload const & response, async_tag const & tag); + bool process (nano::empty_payload const & response, async_tag const & tag); void process_frontiers (std::deque> const & frontiers); From 2bf65e7de87438ce00c2c16e7925dfcf5060c330 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 4 Nov 2024 21:27:23 +0100 Subject: [PATCH 07/14] Peer scoring fix --- nano/node/bootstrap/peer_scoring.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nano/node/bootstrap/peer_scoring.cpp b/nano/node/bootstrap/peer_scoring.cpp index 31c5164b82..b445c6e3cc 100644 --- a/nano/node/bootstrap/peer_scoring.cpp +++ b/nano/node/bootstrap/peer_scoring.cpp @@ -62,7 +62,7 @@ std::shared_ptr nano::bootstrap::peer_scoring::channel { if (auto channel = score.shared ()) { - if (!channel->max ()) + if (!channel->max (nano::transport::traffic_type::bootstrap)) { if (!try_send_message (channel)) { From 09bfa2a8c36cc9b30f8b9fbe59d420d18bc74a6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 4 Nov 2024 21:32:03 +0100 Subject: [PATCH 08/14] Peer scoring container info --- nano/node/bootstrap/bootstrap_service.cpp | 1 + nano/node/bootstrap/peer_scoring.cpp | 40 +++++++++++++++++++---- nano/node/bootstrap/peer_scoring.hpp | 12 ++++--- 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/nano/node/bootstrap/bootstrap_service.cpp b/nano/node/bootstrap/bootstrap_service.cpp index 35d5f8a61c..2c4be83e4d 100644 --- a/nano/node/bootstrap/bootstrap_service.cpp +++ b/nano/node/bootstrap/bootstrap_service.cpp @@ -1115,6 +1115,7 @@ nano::container_info nano::bootstrap_service::container_info () const info.add ("database_scan", database_scan.container_info ()); info.add ("frontiers", frontiers.container_info ()); info.add ("workers", workers.container_info ()); + info.add ("peers", scoring.container_info ()); return info; } diff --git a/nano/node/bootstrap/peer_scoring.cpp b/nano/node/bootstrap/peer_scoring.cpp index b445c6e3cc..d3a76ac311 100644 --- a/nano/node/bootstrap/peer_scoring.cpp +++ b/nano/node/bootstrap/peer_scoring.cpp @@ -12,7 +12,17 @@ nano::bootstrap::peer_scoring::peer_scoring (bootstrap_config const & config_a, { } -bool nano::bootstrap::peer_scoring::try_send_message (std::shared_ptr channel) +bool nano::bootstrap::peer_scoring::limit_exceeded (std::shared_ptr const & channel) const +{ + auto & index = scoring.get (); + if (auto existing = index.find (channel.get ()); existing != index.end ()) + { + return existing->outstanding >= config.channel_limit; + } + return false; +} + +bool nano::bootstrap::peer_scoring::try_send_message (std::shared_ptr const & channel) { auto & index = scoring.get (); auto existing = index.find (channel.get ()); @@ -38,11 +48,10 @@ bool nano::bootstrap::peer_scoring::try_send_message (std::shared_ptr channel) +void nano::bootstrap::peer_scoring::received_message (std::shared_ptr const & channel) { auto & index = scoring.get (); - auto existing = index.find (channel.get ()); - if (existing != index.end ()) + if (auto existing = index.find (channel.get ()); existing != index.end ()) { if (existing->outstanding > 1) { @@ -79,11 +88,20 @@ std::size_t nano::bootstrap::peer_scoring::size () const return scoring.size (); } -void nano::bootstrap::peer_scoring::timeout () +std::size_t nano::bootstrap::peer_scoring::available () const { - auto & index = scoring.get (); + return std::count_if (scoring.begin (), scoring.end (), [this] (auto const & score) { + if (auto channel = score.shared ()) + { + return !limit_exceeded (channel); + } + return false; + }); +} - erase_if (index, [] (auto const & score) { +void nano::bootstrap::peer_scoring::timeout () +{ + erase_if (scoring, [] (auto const & score) { if (auto channel = score.shared ()) { if (channel->alive ()) @@ -120,6 +138,14 @@ void nano::bootstrap::peer_scoring::sync (std::deque channel); - void received_message (std::shared_ptr channel); + bool limit_exceeded (std::shared_ptr const & channel) const; + bool try_send_message (std::shared_ptr const & channel); + void received_message (std::shared_ptr const & channel); std::shared_ptr channel (); [[nodiscard]] std::size_t size () const; + std::size_t available () const; // Cleans up scores for closed channels // Decays scores which become inaccurate over time due to message drops void timeout (); void sync (std::deque> const & list); + nano::container_info container_info () const; + private: bootstrap_config const & config; nano::network_constants const & network_constants; @@ -71,14 +75,14 @@ namespace bootstrap // Indexes scores by the number of outstanding requests in ascending order class tag_outstanding {}; - using scoring_t = boost::multi_index_container, mi::member>, mi::ordered_non_unique, mi::member>>>; // clang-format on - scoring_t scoring; + ordered_scoring scoring; }; } } From 9dd8783ba30987c176ed2ace6887d96e71f52be0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 11 Nov 2024 20:11:51 +0100 Subject: [PATCH 09/14] Randomize peer scoring channels --- nano/node/bootstrap/bootstrap_service.cpp | 2 +- nano/node/bootstrap/peer_scoring.cpp | 38 ++++++----------------- nano/node/bootstrap/peer_scoring.hpp | 13 ++++++-- 3 files changed, 20 insertions(+), 33 deletions(-) diff --git a/nano/node/bootstrap/bootstrap_service.cpp b/nano/node/bootstrap/bootstrap_service.cpp index 2c4be83e4d..7a65307271 100644 --- a/nano/node/bootstrap/bootstrap_service.cpp +++ b/nano/node/bootstrap/bootstrap_service.cpp @@ -659,7 +659,7 @@ void nano::bootstrap_service::cleanup_and_sync () { debug_assert (!mutex.try_lock ()); - scoring.sync (network.list ()); + scoring.sync (network.list (/* all */ 0, network_constants.bootstrap_protocol_version_min)); scoring.timeout (); throttle.resize (compute_throttle_size ()); diff --git a/nano/node/bootstrap/peer_scoring.cpp b/nano/node/bootstrap/peer_scoring.cpp index d3a76ac311..a86bdc5745 100644 --- a/nano/node/bootstrap/peer_scoring.cpp +++ b/nano/node/bootstrap/peer_scoring.cpp @@ -66,17 +66,13 @@ void nano::bootstrap::peer_scoring::received_message (std::shared_ptr nano::bootstrap::peer_scoring::channel () { - auto & index = scoring.get (); - for (auto const & score : index) + for (auto const & channel : channels) { - if (auto channel = score.shared ()) + if (!channel->max (nano::transport::traffic_type::bootstrap)) { - if (!channel->max (nano::transport::traffic_type::bootstrap)) + if (!try_send_message (channel)) { - if (!try_send_message (channel)) - { - return channel; - } + return channel; } } } @@ -90,12 +86,8 @@ std::size_t nano::bootstrap::peer_scoring::size () const std::size_t nano::bootstrap::peer_scoring::available () const { - return std::count_if (scoring.begin (), scoring.end (), [this] (auto const & score) { - if (auto channel = score.shared ()) - { - return !limit_exceeded (channel); - } - return false; + return std::count_if (channels.begin (), channels.end (), [this] (auto const & channel) { + return !limit_exceeded (channel); }); } @@ -122,27 +114,15 @@ void nano::bootstrap::peer_scoring::timeout () void nano::bootstrap::peer_scoring::sync (std::deque> const & list) { - auto & index = scoring.get (); - for (auto const & channel : list) - { - if (channel->get_network_version () >= network_constants.bootstrap_protocol_version_min) - { - if (index.find (channel.get ()) == index.end ()) - { - if (!channel->max (nano::transport::traffic_type::bootstrap)) - { - index.emplace (channel, 1, 1, 0); - } - } - } - } + channels = list; } nano::container_info nano::bootstrap::peer_scoring::container_info () const { nano::container_info info; - info.put ("total", size ()); + info.put ("scores", size ()); info.put ("available", available ()); + info.put ("channels", channels.size ()); return info; } diff --git a/nano/node/bootstrap/peer_scoring.hpp b/nano/node/bootstrap/peer_scoring.hpp index 2431fc4bd2..9b41865393 100644 --- a/nano/node/bootstrap/peer_scoring.hpp +++ b/nano/node/bootstrap/peer_scoring.hpp @@ -26,13 +26,18 @@ namespace bootstrap bool limit_exceeded (std::shared_ptr const & channel) const; bool try_send_message (std::shared_ptr const & channel); void received_message (std::shared_ptr const & channel); + std::shared_ptr channel (); - [[nodiscard]] std::size_t size () const; - std::size_t available () const; + + // Synchronize channels with the network, passed channels should be shuffled + void sync (std::deque> const & list); + // Cleans up scores for closed channels // Decays scores which become inaccurate over time due to message drops void timeout (); - void sync (std::deque> const & list); + + std::size_t size () const; + std::size_t available () const; nano::container_info container_info () const; @@ -83,6 +88,8 @@ namespace bootstrap mi::member>>>; // clang-format on ordered_scoring scoring; + + std::deque> channels; }; } } From 7a97eb06030506fdbca3a3b4f43daaed45aa05c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 11 Nov 2024 20:00:16 +0100 Subject: [PATCH 10/14] Track bootstrap timeout type --- nano/lib/stats_enums.hpp | 1 + nano/node/bootstrap/bootstrap_service.cpp | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index 754790e7f1..9e3086d2e9 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -70,6 +70,7 @@ enum class type bootstrap_frontiers, bootstrap_account_sets, bootstrap_frontier_scan, + bootstrap_timeout, bootstrap_server, bootstrap_server_request, bootstrap_server_overfill, diff --git a/nano/node/bootstrap/bootstrap_service.cpp b/nano/node/bootstrap/bootstrap_service.cpp index 7a65307271..8cbdb1791e 100644 --- a/nano/node/bootstrap/bootstrap_service.cpp +++ b/nano/node/bootstrap/bootstrap_service.cpp @@ -673,8 +673,9 @@ void nano::bootstrap_service::cleanup_and_sync () while (!tags_by_order.empty () && should_timeout (tags_by_order.front ())) { auto tag = tags_by_order.front (); - tags_by_order.pop_front (); stats.inc (nano::stat::type::bootstrap, nano::stat::detail::timeout); + stats.inc (nano::stat::type::bootstrap_timeout, to_stat_detail (tag.type)); + tags_by_order.pop_front (); } if (sync_dependencies_interval.elapsed (60s)) From 7fec95988da2e6e5410744ace8478ff536722dee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 11 Nov 2024 21:43:00 +0100 Subject: [PATCH 11/14] Adjust bootstrap request timeout --- nano/node/bootstrap/bootstrap_config.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nano/node/bootstrap/bootstrap_config.hpp b/nano/node/bootstrap/bootstrap_config.hpp index 22284e6d55..2896803681 100644 --- a/nano/node/bootstrap/bootstrap_config.hpp +++ b/nano/node/bootstrap/bootstrap_config.hpp @@ -53,7 +53,7 @@ class bootstrap_config final std::size_t frontier_rate_limit{ 8 }; std::size_t database_warmup_ratio{ 10 }; std::size_t max_pull_count{ nano::bootstrap_server::max_blocks }; - std::chrono::milliseconds request_timeout{ 1000 * 5 }; + std::chrono::milliseconds request_timeout{ 1000 * 15 }; std::size_t throttle_coefficient{ 8 * 1024 }; std::chrono::milliseconds throttle_wait{ 100 }; std::size_t block_processor_threshold{ 1000 }; From be28a8ac912bd9ba20e53fd2c5d2055b1b47c45a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 11 Nov 2024 21:41:36 +0100 Subject: [PATCH 12/14] Collect bootstrap limiters info --- nano/node/bootstrap/bootstrap_service.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/nano/node/bootstrap/bootstrap_service.cpp b/nano/node/bootstrap/bootstrap_service.cpp index 8cbdb1791e..2a71ffb08e 100644 --- a/nano/node/bootstrap/bootstrap_service.cpp +++ b/nano/node/bootstrap/bootstrap_service.cpp @@ -1108,6 +1108,14 @@ nano::container_info nano::bootstrap_service::container_info () const { nano::lock_guard lock{ mutex }; + auto collect_limiters = [this] () { + nano::container_info info; + info.put ("total", limiter.size ()); + info.put ("database", database_limiter.size ()); + info.put ("frontiers", frontiers_limiter.size ()); + return info; + }; + nano::container_info info; info.put ("tags", tags); info.put ("throttle", throttle.size ()); @@ -1117,6 +1125,7 @@ nano::container_info nano::bootstrap_service::container_info () const info.add ("frontiers", frontiers.container_info ()); info.add ("workers", workers.container_info ()); info.add ("peers", scoring.container_info ()); + info.add ("limiters", collect_limiters ()); return info; } From b851b75ba2aa1b140963a46f99fdfda9c232d2a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Tue, 12 Nov 2024 13:21:56 +0100 Subject: [PATCH 13/14] Adjust max fails --- nano/node/bootstrap/account_sets.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nano/node/bootstrap/account_sets.hpp b/nano/node/bootstrap/account_sets.hpp index 061da0fe5e..0a9e5c0560 100644 --- a/nano/node/bootstrap/account_sets.hpp +++ b/nano/node/bootstrap/account_sets.hpp @@ -30,7 +30,7 @@ namespace bootstrap static double constexpr priority_divide = 2.0; static double constexpr priority_max = 128.0; static double constexpr priority_cutoff = 0.15; - static unsigned constexpr max_fails = 2; + static unsigned constexpr max_fails = 3; public: account_sets (account_sets_config const &, nano::stats &); From 694d975fe20573103ed90e36a750d5e21777c51d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Tue, 12 Nov 2024 13:33:01 +0100 Subject: [PATCH 14/14] Reinsert unblocked with default priority --- nano/core_test/bootstrap.cpp | 11 +++++---- nano/lib/stats_enums.hpp | 2 ++ nano/node/bootstrap/account_sets.cpp | 36 ++++++++++++---------------- nano/node/bootstrap/account_sets.hpp | 15 +++--------- nano/node/json_handler.cpp | 2 +- 5 files changed, 27 insertions(+), 39 deletions(-) diff --git a/nano/core_test/bootstrap.cpp b/nano/core_test/bootstrap.cpp index 2c13390a16..1073a83a39 100644 --- a/nano/core_test/bootstrap.cpp +++ b/nano/core_test/bootstrap.cpp @@ -55,6 +55,7 @@ TEST (account_sets, block) nano::account account{ 1 }; nano::account_sets_config config; nano::bootstrap::account_sets sets{ config, system.stats }; + sets.priority_up (account); sets.block (account, random_hash ()); ASSERT_TRUE (sets.blocked (account)); } @@ -67,7 +68,9 @@ TEST (account_sets, unblock) nano::account_sets_config config; nano::bootstrap::account_sets sets{ config, system.stats }; auto hash = random_hash (); + sets.priority_up (account); sets.block (account, hash); + ASSERT_TRUE (sets.blocked (account)); sets.unblock (account, hash); ASSERT_FALSE (sets.blocked (account)); } @@ -93,8 +96,7 @@ TEST (account_sets, priority_blocked) ASSERT_EQ (0.0, sets.priority (account)); } -// When account is unblocked, check that it retains it former priority -TEST (account_sets, priority_unblock_keep) +TEST (account_sets, priority_unblock) { nano::test::system system; @@ -102,13 +104,12 @@ TEST (account_sets, priority_unblock_keep) nano::account_sets_config config; nano::bootstrap::account_sets sets{ config, system.stats }; sets.priority_up (account); - sets.priority_up (account); - ASSERT_EQ (sets.priority (account), nano::bootstrap::account_sets::priority_initial + nano::bootstrap::account_sets::priority_increase); + ASSERT_EQ (sets.priority (account), nano::bootstrap::account_sets::priority_initial); auto hash = random_hash (); sets.block (account, hash); ASSERT_EQ (0.0, sets.priority (account)); sets.unblock (account, hash); - ASSERT_EQ (sets.priority (account), nano::bootstrap::account_sets::priority_initial + nano::bootstrap::account_sets::priority_increase); + ASSERT_EQ (sets.priority (account), nano::bootstrap::account_sets::priority_initial); } TEST (account_sets, priority_up_down) diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index 9e3086d2e9..ab51468092 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -467,6 +467,7 @@ enum class detail prioritize, prioritize_failed, block, + block_failed, unblock, unblock_failed, dependency_update, @@ -490,6 +491,7 @@ enum class detail blocking_overflow, priority_insert, priority_set, + priority_unblocked, erase_by_threshold, erase_by_blocking, priority_overflow, diff --git a/nano/node/bootstrap/account_sets.cpp b/nano/node/bootstrap/account_sets.cpp index 4093644f5d..98d705dbdd 100644 --- a/nano/node/bootstrap/account_sets.cpp +++ b/nano/node/bootstrap/account_sets.cpp @@ -105,18 +105,20 @@ void nano::bootstrap::account_sets::block (nano::account const & account, nano:: { debug_assert (!account.is_zero ()); - stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::block); - - auto existing = priorities.get ().find (account); - auto entry = (existing == priorities.get ().end ()) ? priority_entry{ account, 0 } : *existing; - - priorities.get ().erase (account); - stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::erase_by_blocking); - - blocking.get ().insert ({ entry, dependency }); - stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::blocking_insert); + auto erased = priorities.get ().erase (account); + if (erased > 0) + { + stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::erase_by_blocking); + stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::block); - trim_overflow (); + debug_assert (blocking.get ().count (account) == 0); + blocking.get ().insert ({ account, dependency }); + trim_overflow (); + } + else + { + stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::block_failed); + } } void nano::bootstrap::account_sets::unblock (nano::account const & account, std::optional const & hash) @@ -131,19 +133,11 @@ void nano::bootstrap::account_sets::unblock (nano::account const & account, std: if (existing != blocking.get ().end () && (!hash || existing->dependency == *hash)) { stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::unblock); + stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::priority_unblocked); debug_assert (priorities.get ().count (account) == 0); - if (!existing->original_entry.account.is_zero ()) - { - debug_assert (existing->original_entry.account == account); - priorities.get ().insert (existing->original_entry); - } - else - { - priorities.get ().insert ({ account, account_sets::priority_initial }); - } + priorities.get ().insert ({ account, account_sets::priority_initial }); blocking.get ().erase (account); - trim_overflow (); } else diff --git a/nano/node/bootstrap/account_sets.hpp b/nano/node/bootstrap/account_sets.hpp index 0a9e5c0560..8456efa3d8 100644 --- a/nano/node/bootstrap/account_sets.hpp +++ b/nano/node/bootstrap/account_sets.hpp @@ -95,25 +95,16 @@ namespace bootstrap nano::account account; double priority; unsigned fails{ 0 }; - id_t id{ generate_id () }; // Uniformly distributed, used for random querying std::chrono::steady_clock::time_point timestamp{}; + id_t id{ generate_id () }; // Uniformly distributed, used for random querying }; struct blocking_entry { - priority_entry original_entry; + nano::account account; nano::block_hash dependency; nano::account dependency_account{ 0 }; id_t id{ generate_id () }; // Uniformly distributed, used for random querying - - nano::account account () const - { - return original_entry.account; - } - double priority () const - { - return original_entry.priority; - } }; // clang-format off @@ -142,7 +133,7 @@ namespace bootstrap mi::indexed_by< mi::sequenced>, mi::ordered_unique, - mi::const_mem_fun>, + mi::member>, mi::ordered_non_unique, mi::member>, mi::ordered_non_unique, diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp index 43dcf3f02c..91dca52cd4 100644 --- a/nano/node/json_handler.cpp +++ b/nano/node/json_handler.cpp @@ -5182,7 +5182,7 @@ void nano::json_handler::debug_bootstrap_priority_info () boost::property_tree::ptree response_blocking; for (auto const & entry : blocking) { - const auto account = entry.account (); + const auto account = entry.account; const auto dependency = entry.dependency; response_blocking.put (account.to_account (), dependency.to_string ());