From ce11ced792901a3f7f959008d10d0943b0734ce1 Mon Sep 17 00:00:00 2001 From: RickiNano <81099017+RickiNano@users.noreply.github.com> Date: Sun, 12 Nov 2023 18:56:40 +0100 Subject: [PATCH 1/5] Set maximum unchecked blocks in config --- nano/core_test/block_store.cpp | 3 ++- nano/core_test/unchecked_map.cpp | 14 ++++++++------ nano/node/node.cpp | 2 +- nano/node/nodeconfig.cpp | 7 +++++++ nano/node/nodeconfig.hpp | 1 + nano/node/unchecked_map.cpp | 6 ++++-- nano/node/unchecked_map.hpp | 5 ++--- 7 files changed, 25 insertions(+), 13 deletions(-) diff --git a/nano/core_test/block_store.cpp b/nano/core_test/block_store.cpp index fe6d90c40c..4e622a2beb 100644 --- a/nano/core_test/block_store.cpp +++ b/nano/core_test/block_store.cpp @@ -436,7 +436,8 @@ TEST (block_store, empty_bootstrap) { nano::test::system system{}; nano::logger logger; - nano::unchecked_map unchecked{ system.stats, false }; + unsigned max_unchecked_blocks = 65536; + nano::unchecked_map unchecked{ max_unchecked_blocks, system.stats, false }; size_t count = 0; unchecked.for_each ([&count] (nano::unchecked_key const & key, nano::unchecked_info const & info) { ++count; diff --git a/nano/core_test/unchecked_map.cpp b/nano/core_test/unchecked_map.cpp index 6823c1f72d..6b3c799652 100644 --- a/nano/core_test/unchecked_map.cpp +++ b/nano/core_test/unchecked_map.cpp @@ -14,11 +14,13 @@ using namespace std::chrono_literals; namespace { +unsigned max_unchecked_blocks = 65536; + class context { public: context () : - unchecked{ stats, false } + unchecked{ max_unchecked_blocks, stats, false } { } nano::stats stats; @@ -54,7 +56,7 @@ TEST (unchecked_map, put_one) TEST (block_store, one_bootstrap) { nano::test::system system{}; - nano::unchecked_map unchecked{ system.stats, false }; + nano::unchecked_map unchecked{ max_unchecked_blocks, system.stats, false }; nano::block_builder builder; auto block1 = builder .send () @@ -87,7 +89,7 @@ TEST (block_store, one_bootstrap) TEST (unchecked, simple) { nano::test::system system{}; - nano::unchecked_map unchecked{ system.stats, false }; + nano::unchecked_map unchecked{ max_unchecked_blocks, system.stats, false }; nano::block_builder builder; auto block = builder .send () @@ -128,7 +130,7 @@ TEST (unchecked, multiple) // Don't test this in rocksdb mode GTEST_SKIP (); } - nano::unchecked_map unchecked{ system.stats, false }; + nano::unchecked_map unchecked{ max_unchecked_blocks, system.stats, false }; nano::block_builder builder; auto block = builder .send () @@ -158,7 +160,7 @@ TEST (unchecked, multiple) TEST (unchecked, double_put) { nano::test::system system{}; - nano::unchecked_map unchecked{ system.stats, false }; + nano::unchecked_map unchecked{ max_unchecked_blocks, system.stats, false }; nano::block_builder builder; auto block = builder .send () @@ -189,7 +191,7 @@ TEST (unchecked, double_put) TEST (unchecked, multiple_get) { nano::test::system system{}; - nano::unchecked_map unchecked{ system.stats, false }; + nano::unchecked_map unchecked{ max_unchecked_blocks, system.stats, false }; // Instantiates three blocks nano::block_builder builder; auto block1 = builder diff --git a/nano/node/node.cpp b/nano/node/node.cpp index 399fe821b9..cb854c326f 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -153,7 +153,7 @@ nano::node::node (boost::asio::io_context & io_ctx_a, std::filesystem::path cons distributed_work (*this), store_impl (nano::make_store (logger, application_path_a, network_params.ledger, flags.read_only, true, config_a.rocksdb_config, config_a.diagnostics_config.txn_tracking, config_a.block_processor_batch_max_time, config_a.lmdb_config, config_a.backup_before_upgrade)), store (*store_impl), - unchecked{ stats, flags.disable_block_processor_unchecked_deletion }, + unchecked{ config.max_unchecked_blocks, stats, flags.disable_block_processor_unchecked_deletion }, wallets_store_impl (std::make_unique (application_path_a / "wallets.ldb", config_a.lmdb_config)), wallets_store (*wallets_store_impl), gap_cache (*this), diff --git a/nano/node/nodeconfig.cpp b/nano/node/nodeconfig.cpp index 61179c685c..60c9fc50d1 100644 --- a/nano/node/nodeconfig.cpp +++ b/nano/node/nodeconfig.cpp @@ -128,6 +128,7 @@ nano::error nano::node_config::serialize_toml (nano::tomlconfig & toml) const toml.put ("max_work_generate_multiplier", max_work_generate_multiplier, "Maximum allowed difficulty multiplier for work generation.\ntype:double,[1..]"); toml.put ("frontiers_confirmation", serialize_frontiers_confirmation (frontiers_confirmation), "Mode controlling frontier confirmation rate.\ntype:string,{auto,always,disabled}"); toml.put ("max_queued_requests", max_queued_requests, "Limit for number of queued confirmation requests for one channel, after which new requests are dropped until the queue drops below this value.\ntype:uint32"); + toml.put ("max_unchecked_blocks", max_unchecked_blocks, "Maximum number of unchecked blocks to store in memory. Defaults to 65536. \ntype:uint64,[1..11]"); toml.put ("rep_crawler_weight_minimum", rep_crawler_weight_minimum.to_string_dec (), "Rep crawler minimum weight, if this is less than minimum principal weight then this is taken as the minimum weight a rep must have to be tracked. If you want to track all reps set this to 0. If you do not want this to influence anything then set it to max value. This is only useful for debugging or for people who really know what they are doing.\ntype:string,amount,raw"); toml.put ("backlog_scan_batch_size", backlog_scan_batch_size, "Number of accounts per second to process when doing backlog population scan. Increasing this value will help unconfirmed frontiers get into election prioritization queue faster, however it will also increase resource usage. \ntype:uint"); toml.put ("backlog_scan_frequency", backlog_scan_frequency, "Backlog scan divides the scan into smaller batches, number of which is controlled by this value. Higher frequency helps to utilize resources more uniformly, however it also introduces more overhead. The resulting number of accounts per single batch is `backlog_scan_batch_size / backlog_scan_frequency` \ntype:uint"); @@ -415,6 +416,8 @@ nano::error nano::node_config::deserialize_toml (nano::tomlconfig & toml) toml.get ("max_queued_requests", max_queued_requests); + toml.get ("max_unchecked_blocks", max_unchecked_blocks); + auto rep_crawler_weight_minimum_l (rep_crawler_weight_minimum.to_string_dec ()); if (toml.has_key ("rep_crawler_weight_minimum")) { @@ -462,6 +465,10 @@ nano::error nano::node_config::deserialize_toml (nano::tomlconfig & toml) { toml.get_error ().set ("active_elections_size must be greater than 250"); } + if (max_unchecked_blocks < 1) + { + toml.get_error ().set ("max_unchecked_blocks must be greater than 0"); + } if (bandwidth_limit > std::numeric_limits::max ()) { toml.get_error ().set ("bandwidth_limit unbounded = 0, default = 10485760, max = 18446744073709551615"); diff --git a/nano/node/nodeconfig.hpp b/nano/node/nodeconfig.hpp index d1a42fa370..8f7580534a 100644 --- a/nano/node/nodeconfig.hpp +++ b/nano/node/nodeconfig.hpp @@ -116,6 +116,7 @@ class node_config bool backup_before_upgrade{ false }; double max_work_generate_multiplier{ 64. }; uint32_t max_queued_requests{ 512 }; + unsigned max_unchecked_blocks{ 65536 }; std::chrono::seconds max_pruning_age{ !network_params.network.is_beta_network () ? std::chrono::seconds (24 * 60 * 60) : std::chrono::seconds (5 * 60) }; // 1 day; 5 minutes for beta network uint64_t max_pruning_depth{ 0 }; nano::rocksdb_config rocksdb_config; diff --git a/nano/node/unchecked_map.cpp b/nano/node/unchecked_map.cpp index 45d3b49563..9c9ccd53f1 100644 --- a/nano/node/unchecked_map.cpp +++ b/nano/node/unchecked_map.cpp @@ -5,7 +5,8 @@ #include #include -nano::unchecked_map::unchecked_map (nano::stats & stats, bool const & disable_delete) : +nano::unchecked_map::unchecked_map (unsigned & max_unchecked_blocks, nano::stats & stats, bool const & disable_delete) : + max_unchecked_blocks{ max_unchecked_blocks }, stats{ stats }, disable_delete{ disable_delete }, thread{ [this] () { run (); } } @@ -23,7 +24,8 @@ void nano::unchecked_map::put (nano::hash_or_account const & dependency, nano::u nano::lock_guard lock{ entries_mutex }; nano::unchecked_key key{ dependency, info.block->hash () }; entries.get ().insert ({ key, info }); - if (entries.size () > mem_block_count_max) + + if (entries.size () > max_unchecked_blocks) { entries.get ().pop_front (); } diff --git a/nano/node/unchecked_map.hpp b/nano/node/unchecked_map.hpp index b76e8ccb44..c733ba20c8 100644 --- a/nano/node/unchecked_map.hpp +++ b/nano/node/unchecked_map.hpp @@ -22,7 +22,7 @@ class stats; class unchecked_map { public: - unchecked_map (nano::stats &, bool const & do_delete); + unchecked_map (unsigned & max_unchecked_blocks, nano::stats &, bool const & do_delete); ~unchecked_map (); void put (nano::hash_or_account const & dependency, nano::unchecked_info const & info); @@ -62,11 +62,10 @@ class unchecked_map nano::condition_variable condition; nano::mutex mutex; std::thread thread; + unsigned max_unchecked_blocks; void process_queries (decltype (buffer) const & back_buffer); - static std::size_t constexpr mem_block_count_max = 64 * 1024; - private: struct entry { From e8df19506783a8217d7c25435c93866e1aee8f23 Mon Sep 17 00:00:00 2001 From: RickiNano <81099017+RickiNano@users.noreply.github.com> Date: Fri, 26 Jan 2024 15:40:53 +0100 Subject: [PATCH 2/5] Minimum value zero or higher --- nano/node/nodeconfig.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nano/node/nodeconfig.cpp b/nano/node/nodeconfig.cpp index 60c9fc50d1..a437a930a6 100644 --- a/nano/node/nodeconfig.cpp +++ b/nano/node/nodeconfig.cpp @@ -465,9 +465,9 @@ nano::error nano::node_config::deserialize_toml (nano::tomlconfig & toml) { toml.get_error ().set ("active_elections_size must be greater than 250"); } - if (max_unchecked_blocks < 1) + if (max_unchecked_blocks < 0) { - toml.get_error ().set ("max_unchecked_blocks must be greater than 0"); + toml.get_error ().set ("max_unchecked_blocks must be 0 or higher"); } if (bandwidth_limit > std::numeric_limits::max ()) { From 1d8e661ef1b2136f71f8d8a6b3edb547352e82be Mon Sep 17 00:00:00 2001 From: RickiNano <81099017+RickiNano@users.noreply.github.com> Date: Sun, 28 Jan 2024 20:15:57 +0100 Subject: [PATCH 3/5] Use const value instead of reference --- nano/node/unchecked_map.cpp | 2 +- nano/node/unchecked_map.hpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nano/node/unchecked_map.cpp b/nano/node/unchecked_map.cpp index 9c9ccd53f1..b20651d793 100644 --- a/nano/node/unchecked_map.cpp +++ b/nano/node/unchecked_map.cpp @@ -5,7 +5,7 @@ #include #include -nano::unchecked_map::unchecked_map (unsigned & max_unchecked_blocks, nano::stats & stats, bool const & disable_delete) : +nano::unchecked_map::unchecked_map (unsigned const max_unchecked_blocks, nano::stats & stats, bool const & disable_delete) : max_unchecked_blocks{ max_unchecked_blocks }, stats{ stats }, disable_delete{ disable_delete }, diff --git a/nano/node/unchecked_map.hpp b/nano/node/unchecked_map.hpp index c733ba20c8..9a548c203d 100644 --- a/nano/node/unchecked_map.hpp +++ b/nano/node/unchecked_map.hpp @@ -22,7 +22,7 @@ class stats; class unchecked_map { public: - unchecked_map (unsigned & max_unchecked_blocks, nano::stats &, bool const & do_delete); + unchecked_map (unsigned const max_unchecked_blocks, nano::stats &, bool const & do_delete); ~unchecked_map (); void put (nano::hash_or_account const & dependency, nano::unchecked_info const & info); @@ -62,7 +62,7 @@ class unchecked_map nano::condition_variable condition; nano::mutex mutex; std::thread thread; - unsigned max_unchecked_blocks; + unsigned const max_unchecked_blocks; void process_queries (decltype (buffer) const & back_buffer); From 52b6d38a3713518ed798cb89fdff11fffd9f88c3 Mon Sep 17 00:00:00 2001 From: RickiNano <81099017+RickiNano@users.noreply.github.com> Date: Sun, 28 Jan 2024 20:23:14 +0100 Subject: [PATCH 4/5] Fixed value range and removed always false if statement --- nano/node/nodeconfig.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/nano/node/nodeconfig.cpp b/nano/node/nodeconfig.cpp index a437a930a6..175d78477b 100644 --- a/nano/node/nodeconfig.cpp +++ b/nano/node/nodeconfig.cpp @@ -128,7 +128,7 @@ nano::error nano::node_config::serialize_toml (nano::tomlconfig & toml) const toml.put ("max_work_generate_multiplier", max_work_generate_multiplier, "Maximum allowed difficulty multiplier for work generation.\ntype:double,[1..]"); toml.put ("frontiers_confirmation", serialize_frontiers_confirmation (frontiers_confirmation), "Mode controlling frontier confirmation rate.\ntype:string,{auto,always,disabled}"); toml.put ("max_queued_requests", max_queued_requests, "Limit for number of queued confirmation requests for one channel, after which new requests are dropped until the queue drops below this value.\ntype:uint32"); - toml.put ("max_unchecked_blocks", max_unchecked_blocks, "Maximum number of unchecked blocks to store in memory. Defaults to 65536. \ntype:uint64,[1..11]"); + toml.put ("max_unchecked_blocks", max_unchecked_blocks, "Maximum number of unchecked blocks to store in memory. Defaults to 65536. \ntype:uint64,[0..]"); toml.put ("rep_crawler_weight_minimum", rep_crawler_weight_minimum.to_string_dec (), "Rep crawler minimum weight, if this is less than minimum principal weight then this is taken as the minimum weight a rep must have to be tracked. If you want to track all reps set this to 0. If you do not want this to influence anything then set it to max value. This is only useful for debugging or for people who really know what they are doing.\ntype:string,amount,raw"); toml.put ("backlog_scan_batch_size", backlog_scan_batch_size, "Number of accounts per second to process when doing backlog population scan. Increasing this value will help unconfirmed frontiers get into election prioritization queue faster, however it will also increase resource usage. \ntype:uint"); toml.put ("backlog_scan_frequency", backlog_scan_frequency, "Backlog scan divides the scan into smaller batches, number of which is controlled by this value. Higher frequency helps to utilize resources more uniformly, however it also introduces more overhead. The resulting number of accounts per single batch is `backlog_scan_batch_size / backlog_scan_frequency` \ntype:uint"); @@ -465,10 +465,6 @@ nano::error nano::node_config::deserialize_toml (nano::tomlconfig & toml) { toml.get_error ().set ("active_elections_size must be greater than 250"); } - if (max_unchecked_blocks < 0) - { - toml.get_error ().set ("max_unchecked_blocks must be 0 or higher"); - } if (bandwidth_limit > std::numeric_limits::max ()) { toml.get_error ().set ("bandwidth_limit unbounded = 0, default = 10485760, max = 18446744073709551615"); From c2d2c4f6dc74b164c4a25da891995e1e1ff8ec0a Mon Sep 17 00:00:00 2001 From: RickiNano <81099017+RickiNano@users.noreply.github.com> Date: Sun, 28 Jan 2024 20:31:53 +0100 Subject: [PATCH 5/5] Added unit test --- nano/core_test/toml.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nano/core_test/toml.cpp b/nano/core_test/toml.cpp index b28c875750..4e13e7b422 100644 --- a/nano/core_test/toml.cpp +++ b/nano/core_test/toml.cpp @@ -193,6 +193,7 @@ TEST (toml, daemon_config_deserialize_defaults) ASSERT_EQ (conf.node.work_peers, defaults.node.work_peers); ASSERT_EQ (conf.node.work_threads, defaults.node.work_threads); ASSERT_EQ (conf.node.max_queued_requests, defaults.node.max_queued_requests); + ASSERT_EQ (conf.node.max_unchecked_blocks, defaults.node.max_unchecked_blocks); ASSERT_EQ (conf.node.backlog_scan_batch_size, defaults.node.backlog_scan_batch_size); ASSERT_EQ (conf.node.backlog_scan_frequency, defaults.node.backlog_scan_frequency); @@ -421,6 +422,7 @@ TEST (toml, daemon_config_deserialize_no_defaults) work_threads = 999 max_work_generate_multiplier = 1.0 max_queued_requests = 999 + max_unchecked_blocks = 999 frontiers_confirmation = "always" backlog_scan_batch_size = 999 backlog_scan_frequency = 999 @@ -585,6 +587,7 @@ TEST (toml, daemon_config_deserialize_no_defaults) ASSERT_NE (conf.node.external_port, defaults.node.external_port); ASSERT_NE (conf.node.io_threads, defaults.node.io_threads); ASSERT_NE (conf.node.max_work_generate_multiplier, defaults.node.max_work_generate_multiplier); + ASSERT_NE (conf.node.max_unchecked_blocks, defaults.node.max_unchecked_blocks); ASSERT_NE (conf.node.frontiers_confirmation, defaults.node.frontiers_confirmation); ASSERT_NE (conf.node.network_threads, defaults.node.network_threads); ASSERT_NE (conf.node.background_threads, defaults.node.background_threads);