From ec15680d50ad80d43ba0636fa42db3e2f91e38d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20W=C3=B3jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 29 Jul 2024 19:01:45 +0200 Subject: [PATCH] Use chrono timestamps in ascending bootstrap code (#4687) * Use unique ptr * Use chrono timestamps --- nano/core_test/bootstrap_ascending.cpp | 6 ++-- nano/lib/tomlconfig.hpp | 10 ++++++ nano/node/bootstrap/bootstrap_config.cpp | 12 +++---- nano/node/bootstrap/bootstrap_config.hpp | 6 ++-- .../node/bootstrap_ascending/account_sets.cpp | 34 +++++++++---------- .../node/bootstrap_ascending/account_sets.hpp | 12 +++---- nano/node/bootstrap_ascending/service.cpp | 15 +++++--- nano/node/bootstrap_ascending/service.hpp | 3 +- nano/node/message_processor.cpp | 1 + nano/node/node.cpp | 4 ++- nano/node/node.hpp | 8 +++-- 11 files changed, 67 insertions(+), 44 deletions(-) diff --git a/nano/core_test/bootstrap_ascending.cpp b/nano/core_test/bootstrap_ascending.cpp index be946cb209..83abac19da 100644 --- a/nano/core_test/bootstrap_ascending.cpp +++ b/nano/core_test/bootstrap_ascending.cpp @@ -264,9 +264,9 @@ TEST (bootstrap_ascending, config_serialization) config1.requests_limit = 0x101; config1.database_requests_limit = 0x102; config1.pull_count = 0x103; - config1.timeout = 0x104; + config1.request_timeout = 0x104ms; config1.throttle_coefficient = 0x105; - config1.throttle_wait = 0x106; + config1.throttle_wait = 0x106ms; config1.block_wait_count = 0x107; nano::tomlconfig toml1; ASSERT_FALSE (config1.serialize (toml1)); @@ -281,7 +281,7 @@ TEST (bootstrap_ascending, config_serialization) ASSERT_EQ (config1.requests_limit, config2.requests_limit); ASSERT_EQ (config1.database_requests_limit, config2.database_requests_limit); ASSERT_EQ (config1.pull_count, config2.pull_count); - ASSERT_EQ (config1.timeout, config2.timeout); + ASSERT_EQ (config1.request_timeout, config2.request_timeout); ASSERT_EQ (config1.throttle_coefficient, config2.throttle_coefficient); ASSERT_EQ (config1.throttle_wait, config2.throttle_wait); ASSERT_EQ (config1.block_wait_count, config2.block_wait_count); diff --git a/nano/lib/tomlconfig.hpp b/nano/lib/tomlconfig.hpp index 36463e6122..f22d4482a5 100644 --- a/nano/lib/tomlconfig.hpp +++ b/nano/lib/tomlconfig.hpp @@ -143,6 +143,16 @@ class tomlconfig : public nano::configbase return *this; } + /** Get chrono duration */ + template + tomlconfig & get_duration (std::string const & key, Duration & target) + { + uint64_t value; + get (key, value); + target = Duration{ value }; + return *this; + } + /** * Get value of optional key. Use default value of data type if missing. */ diff --git a/nano/node/bootstrap/bootstrap_config.cpp b/nano/node/bootstrap/bootstrap_config.cpp index 762fb14d99..77c16e3eea 100644 --- a/nano/node/bootstrap/bootstrap_config.cpp +++ b/nano/node/bootstrap/bootstrap_config.cpp @@ -9,7 +9,7 @@ nano::error nano::account_sets_config::deserialize (nano::tomlconfig & toml) toml.get ("consideration_count", consideration_count); toml.get ("priorities_max", priorities_max); toml.get ("blocking_max", blocking_max); - toml.get ("cooldown", cooldown); + toml.get_duration ("cooldown", cooldown); return toml.get_error (); } @@ -19,7 +19,7 @@ nano::error nano::account_sets_config::serialize (nano::tomlconfig & toml) const toml.put ("consideration_count", consideration_count, "Limit the number of account candidates to consider and also the number of iterations.\ntype:uint64"); toml.put ("priorities_max", priorities_max, "Cutoff size limit for the priority list.\ntype:uint64"); toml.put ("blocking_max", blocking_max, "Cutoff size limit for the blocked accounts from the priority list.\ntype:uint64"); - toml.put ("cooldown", cooldown, "Waiting time for an account to become available.\ntype:milliseconds"); + toml.put ("cooldown", cooldown.count (), "Waiting time for an account to become available.\ntype:milliseconds"); return toml.get_error (); } @@ -32,9 +32,9 @@ nano::error nano::bootstrap_ascending_config::deserialize (nano::tomlconfig & to toml.get ("requests_limit", requests_limit); toml.get ("database_requests_limit", database_requests_limit); toml.get ("pull_count", pull_count); - toml.get ("timeout", timeout); + toml.get_duration ("timeout", request_timeout); toml.get ("throttle_coefficient", throttle_coefficient); - toml.get ("throttle_wait", throttle_wait); + toml.get_duration ("throttle_wait", throttle_wait); toml.get ("block_wait_count", block_wait_count); if (toml.has_key ("account_sets")) @@ -51,9 +51,9 @@ nano::error nano::bootstrap_ascending_config::serialize (nano::tomlconfig & toml toml.put ("requests_limit", requests_limit, "Request limit to ascending bootstrap after which requests will be dropped.\nNote: changing to unlimited (0) is not recommended.\ntype:uint64"); toml.put ("database_requests_limit", database_requests_limit, "Request limit for accounts from database after which requests will be dropped.\nNote: changing to unlimited (0) is not recommended as this operation competes for resources on querying the database.\ntype:uint64"); toml.put ("pull_count", pull_count, "Number of requested blocks for ascending bootstrap request.\ntype:uint64"); - toml.put ("timeout", timeout, "Timeout in milliseconds for incoming ascending bootstrap messages to be processed.\ntype:milliseconds"); + toml.put ("timeout", request_timeout.count (), "Timeout in milliseconds for incoming ascending bootstrap messages to be processed.\ntype:milliseconds"); toml.put ("throttle_coefficient", throttle_coefficient, "Scales the number of samples to track for bootstrap throttling.\ntype:uint64"); - toml.put ("throttle_wait", throttle_wait, "Length of time to wait between requests when throttled.\ntype:milliseconds"); + toml.put ("throttle_wait", throttle_wait.count (), "Length of time to wait between requests when throttled.\ntype:milliseconds"); toml.put ("block_wait_count", block_wait_count, "Asending bootstrap will wait while block processor has more than this many blocks queued.\ntype:uint64"); nano::tomlconfig account_sets_l; diff --git a/nano/node/bootstrap/bootstrap_config.hpp b/nano/node/bootstrap/bootstrap_config.hpp index 9b618580f0..abcb404c89 100644 --- a/nano/node/bootstrap/bootstrap_config.hpp +++ b/nano/node/bootstrap/bootstrap_config.hpp @@ -17,7 +17,7 @@ class account_sets_config final std::size_t consideration_count{ 4 }; std::size_t priorities_max{ 256 * 1024 }; std::size_t blocking_max{ 256 * 1024 }; - nano::millis_t cooldown{ 1000 * 3 }; + std::chrono::milliseconds cooldown{ 1000 * 3 }; }; class bootstrap_ascending_config final @@ -30,9 +30,9 @@ class bootstrap_ascending_config final std::size_t requests_limit{ 64 }; std::size_t database_requests_limit{ 1024 }; std::size_t pull_count{ nano::bootstrap_server::max_blocks }; - nano::millis_t timeout{ 1000 * 3 }; + std::chrono::milliseconds request_timeout{ 1000 * 5 }; std::size_t throttle_coefficient{ 16 }; - nano::millis_t throttle_wait{ 100 }; + std::chrono::milliseconds throttle_wait{ 100 }; std::size_t block_wait_count{ 1000 }; nano::account_sets_config account_sets; diff --git a/nano/node/bootstrap_ascending/account_sets.cpp b/nano/node/bootstrap_ascending/account_sets.cpp index 1348c5bebc..91948fd0fa 100644 --- a/nano/node/bootstrap_ascending/account_sets.cpp +++ b/nano/node/bootstrap_ascending/account_sets.cpp @@ -114,25 +114,36 @@ void nano::bootstrap_ascending::account_sets::unblock (nano::account const & acc } } -void nano::bootstrap_ascending::account_sets::timestamp (const nano::account & account, bool reset) +void nano::bootstrap_ascending::account_sets::timestamp_set (const nano::account & account) { - const nano::millis_t tstamp = reset ? 0 : nano::milliseconds_since_epoch (); + auto iter = priorities.get ().find (account); + if (iter != priorities.get ().end ()) + { + priorities.get ().modify (iter, [] (auto & entry) { + entry.timestamp = std::chrono::steady_clock::now (); + }); + } +} +void nano::bootstrap_ascending::account_sets::timestamp_reset (const nano::account & account) +{ auto iter = priorities.get ().find (account); if (iter != priorities.get ().end ()) { - priorities.get ().modify (iter, [tstamp] (auto & entry) { - entry.timestamp = tstamp; + priorities.get ().modify (iter, [] (auto & entry) { + entry.timestamp = {}; }); } } +// Returns false if the account is busy bool nano::bootstrap_ascending::account_sets::check_timestamp (const nano::account & account) const { auto iter = priorities.get ().find (account); if (iter != priorities.get ().end ()) { - if (nano::milliseconds_since_epoch () - iter->timestamp < config.cooldown) + auto const cutoff = std::chrono::steady_clock::now () - config.cooldown; + if (iter->timestamp > cutoff) { return false; } @@ -240,15 +251,4 @@ std::unique_ptr nano::bootstrap_ascending::accou composite->add_component (std::make_unique (container_info{ "priorities", priorities.size (), sizeof (decltype (priorities)::value_type) })); composite->add_component (std::make_unique (container_info{ "blocking", blocking.size (), sizeof (decltype (blocking)::value_type) })); return composite; -} - -/* - * priority_entry - */ - -nano::bootstrap_ascending::account_sets::priority_entry::priority_entry (nano::account account_a, float priority_a) : - account{ account_a }, - priority{ priority_a } -{ - id = nano::bootstrap_ascending::generate_id (); -} +} \ No newline at end of file diff --git a/nano/node/bootstrap_ascending/account_sets.hpp b/nano/node/bootstrap_ascending/account_sets.hpp index ebed04a465..2fc0ef76b1 100644 --- a/nano/node/bootstrap_ascending/account_sets.hpp +++ b/nano/node/bootstrap_ascending/account_sets.hpp @@ -41,7 +41,8 @@ namespace bootstrap_ascending void priority_down (nano::account const & account); void block (nano::account const & account, nano::block_hash const & dependency); void unblock (nano::account const & account, std::optional const & hash = std::nullopt); - void timestamp (nano::account const & account, bool reset = false); + void timestamp_set (nano::account const & account); + void timestamp_reset (nano::account const & account); nano::account next (); @@ -68,12 +69,11 @@ namespace bootstrap_ascending private: struct priority_entry { - nano::account account{ 0 }; - float priority{ 0 }; - nano::millis_t timestamp{ 0 }; - nano::bootstrap_ascending::id_t id{ 0 }; // Uniformly distributed, used for random querying + nano::account account; + float priority; - priority_entry (nano::account account, float priority); + id_t id{ generate_id () }; // Uniformly distributed, used for random querying + std::chrono::steady_clock::time_point timestamp{}; }; struct blocking_entry diff --git a/nano/node/bootstrap_ascending/service.cpp b/nano/node/bootstrap_ascending/service.cpp index 8bceda97d3..d64c657966 100644 --- a/nano/node/bootstrap_ascending/service.cpp +++ b/nano/node/bootstrap_ascending/service.cpp @@ -141,7 +141,7 @@ void nano::bootstrap_ascending::service::inspect (secure::transaction const & tx // If we've inserted any block in to an account, unmark it as blocked accounts.unblock (account); accounts.priority_up (account); - accounts.timestamp (account, /* reset timestamp */ true); + accounts.timestamp_reset (account); if (block.is_send ()) { @@ -230,7 +230,7 @@ nano::account nano::bootstrap_ascending::service::wait_available_account () auto account = available_account (); if (!account.is_zero ()) { - accounts.timestamp (account); + accounts.timestamp_set (account); return account; } else @@ -246,7 +246,6 @@ bool nano::bootstrap_ascending::service::request (nano::account & account, std:: async_tag tag{}; tag.id = nano::bootstrap_ascending::generate_id (); tag.account = account; - tag.time = nano::milliseconds_since_epoch (); // Check if the account picked has blocks, if it does, start the pull from the highest block auto info = ledger.store.account.get (ledger.store.tx_begin_read (), account); @@ -323,8 +322,14 @@ void nano::bootstrap_ascending::service::run_timeouts () scoring.sync (network.list ()); scoring.timeout (); throttle.resize (compute_throttle_size ()); + + auto const cutoff = std::chrono::steady_clock::now () - config.bootstrap_ascending.request_timeout; + auto should_timeout = [cutoff] (async_tag const & tag) { + return tag.timestamp < cutoff; + }; + auto & tags_by_order = tags.get (); - while (!tags_by_order.empty () && nano::time_difference (tags_by_order.front ().time, nano::milliseconds_since_epoch ()) > config.bootstrap_ascending.timeout) + while (!tags_by_order.empty () && should_timeout (tags_by_order.front ())) { auto tag = tags_by_order.front (); tags_by_order.pop_front (); @@ -350,7 +355,7 @@ void nano::bootstrap_ascending::service::process (nano::asc_pull_ack const & mes tags_by_id.erase (iterator); // Track bootstrap request response time - stats.sample (nano::stat::sample::bootstrap_tag_duration, nano::milliseconds_since_epoch () - tag.time, { 0, config.bootstrap_ascending.timeout }); + stats.sample (nano::stat::sample::bootstrap_tag_duration, nano::log::milliseconds_delta (tag.timestamp), { 0, config.bootstrap_ascending.request_timeout.count () }); scoring.received_message (channel); diff --git a/nano/node/bootstrap_ascending/service.hpp b/nano/node/bootstrap_ascending/service.hpp index ccb2ea66cd..a1470da2cb 100644 --- a/nano/node/bootstrap_ascending/service.hpp +++ b/nano/node/bootstrap_ascending/service.hpp @@ -82,8 +82,9 @@ namespace bootstrap_ascending query_type type{ query_type::invalid }; nano::bootstrap_ascending::id_t id{ 0 }; nano::hash_or_account start{ 0 }; - nano::millis_t time{ 0 }; nano::account account{ 0 }; + + std::chrono::steady_clock::time_point timestamp{ std::chrono::steady_clock::now () }; }; public: // Events diff --git a/nano/node/message_processor.cpp b/nano/node/message_processor.cpp index a479b7bf7b..dc1684789a 100644 --- a/nano/node/message_processor.cpp +++ b/nano/node/message_processor.cpp @@ -1,4 +1,5 @@ #include +#include #include #include diff --git a/nano/node/node.cpp b/nano/node/node.cpp index eaa141884d..f296ae5cca 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -216,7 +217,8 @@ nano::node::node (std::shared_ptr io_ctx_a, std::filesy aggregator{ *aggregator_impl }, wallets (wallets_store.init_error (), *this), backlog{ nano::backlog_population_config (config), scheduler, ledger, stats }, - ascendboot{ config, block_processor, ledger, network, stats }, + ascendboot_impl{ std::make_unique (config, block_processor, ledger, network, stats) }, + ascendboot{ *ascendboot_impl }, websocket{ config.websocket_config, observers, wallets, ledger, io_ctx, logger }, epoch_upgrader{ *this, ledger, store, network_params, logger }, local_block_broadcaster_impl{ std::make_unique (config.local_block_broadcaster, *this, block_processor, network, confirming_set, stats, logger, !flags.disable_block_processor_republishing) }, diff --git a/nano/node/node.hpp b/nano/node/node.hpp index 6b8d2eec94..020910bf1f 100644 --- a/nano/node/node.hpp +++ b/nano/node/node.hpp @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include @@ -62,6 +61,10 @@ namespace transport { class tcp_listener; } +namespace bootstrap_ascending +{ + class service; +} namespace rocksdb { } // Declare a namespace rocksdb inside nano so all references to the rocksdb library need to be globally scoped e.g. ::rocksdb::Slice @@ -209,7 +212,8 @@ class node final : public std::enable_shared_from_this nano::request_aggregator & aggregator; nano::wallets wallets; nano::backlog_population backlog; - nano::bootstrap_ascending::service ascendboot; + std::unique_ptr ascendboot_impl; + nano::bootstrap_ascending::service & ascendboot; nano::websocket_server websocket; nano::epoch_upgrader epoch_upgrader; std::unique_ptr local_block_broadcaster_impl;