Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This demonstrates a speedup in confirmation rate during initial boots… #3644

Merged
merged 3 commits into from
Apr 21, 2022
Merged

This demonstrates a speedup in confirmation rate during initial boots… #3644

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d25d943
This change limits confirmation requests by the status of the tcp cha…
clemahieu Apr 19, 2022
9e39e23
Removing confirm_req_batches_max as it's unused.
clemahieu Apr 20, 2022
4fb2b1d
Rename channel::full() to channel::max() to match socket::max()
dsiganos Apr 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion nano/core_test/confirmation_solicitor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ TEST (confirmation_solicitor, batches)
}
// Reached the maximum amount of requests for the channel
auto election (std::make_shared<nano::election> (node2, send, nullptr, nullptr, nano::election_behavior::normal));
ASSERT_TRUE (solicitor.add (*election));
// 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));
Expand Down
17 changes: 0 additions & 17 deletions nano/core_test/toml.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ 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.confirm_req_batches_max, defaults.node.confirm_req_batches_max);

ASSERT_EQ (conf.node.logging.bulk_pull_logging_value, defaults.node.logging.bulk_pull_logging_value);
ASSERT_EQ (conf.node.logging.flush, defaults.node.logging.flush);
Expand Down Expand Up @@ -594,7 +593,6 @@ TEST (toml, daemon_config_deserialize_no_defaults)
ASSERT_NE (conf.node.work_peers, defaults.node.work_peers);
ASSERT_NE (conf.node.work_threads, defaults.node.work_threads);
ASSERT_NE (conf.node.max_queued_requests, defaults.node.max_queued_requests);
ASSERT_EQ (conf.node.confirm_req_batches_max, defaults.node.confirm_req_batches_max);

ASSERT_NE (conf.node.logging.bulk_pull_logging_value, defaults.node.logging.bulk_pull_logging_value);
ASSERT_NE (conf.node.logging.flush, defaults.node.logging.flush);
Expand Down Expand Up @@ -830,21 +828,6 @@ TEST (toml, daemon_config_deserialize_errors)
ASSERT_EQ (toml.get_error ().get_message (), "election_hint_weight_percent must be a number between 5 and 50");
}

{
std::stringstream ss;
ss << R"toml(
[node]
confirm_req_batches_max = 0
)toml";

nano::tomlconfig toml;
toml.read (ss);
nano::daemon_config conf;
conf.deserialize_toml (toml);

ASSERT_EQ (toml.get_error ().get_message (), "confirm_req_batches_max must be between 1 and 100");
}

{
std::stringstream ss;
ss << R"toml(
Expand Down
3 changes: 1 addition & 2 deletions nano/node/confirmation_solicitor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ bool nano::confirmation_solicitor::add (nano::election const & election_a)
debug_assert (prepared);
bool error (true);
unsigned count = 0;
auto const max_channel_requests (config.confirm_req_batches_max * nano::network::confirm_req_hashes_max);
auto const & hash (election_a.status.winner->hash ());
for (auto i (representatives_requests.begin ()); i != representatives_requests.end () && count < max_election_requests;)
{
Expand All @@ -70,7 +69,7 @@ bool nano::confirmation_solicitor::add (nano::election const & election_a)
if (!exists || !is_final || different)
{
auto & request_queue (requests[rep.channel]);
if (request_queue.size () < max_channel_requests)
if (!rep.channel->full ())
Copy link
Contributor

@dsiganos dsiganos Apr 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely filling up the buffers is possibly not a good idea. It might starve other processes of network bandwidth. We are going from a value of 2 to thousands. How about half or quarter full?

{
request_queue.emplace_back (election_a.status.winner->hash (), election_a.status.winner->root ());
count += different ? 0 : 1;
Expand Down
6 changes: 0 additions & 6 deletions nano/node/nodeconfig.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ 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 ("confirm_req_batches_max", confirm_req_batches_max, "Limit for the number of confirmation requests for one channel per request attempt\ntype:uint32");
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");

auto work_peers_l (toml.create_array ("work_peers", "A list of \"address:port\" entries to identify work peers."));
Expand Down Expand Up @@ -370,7 +369,6 @@ nano::error nano::node_config::deserialize_toml (nano::tomlconfig & toml)
toml.get<double> ("max_work_generate_multiplier", max_work_generate_multiplier);

toml.get<uint32_t> ("max_queued_requests", max_queued_requests);
toml.get<uint32_t> ("confirm_req_batches_max", confirm_req_batches_max);

auto rep_crawler_weight_minimum_l (rep_crawler_weight_minimum.to_string_dec ());
if (toml.has_key ("rep_crawler_weight_minimum"))
Expand Down Expand Up @@ -445,10 +443,6 @@ nano::error nano::node_config::deserialize_toml (nano::tomlconfig & toml)
{
toml.get_error ().set ("max_pruning_age must be greater than or equal to 5 minutes");
}
if (confirm_req_batches_max < 1 || confirm_req_batches_max > 100)
{
toml.get_error ().set ("confirm_req_batches_max must be between 1 and 100");
}
if (bootstrap_frontier_request_count < 1024)
{
toml.get_error ().set ("bootstrap_frontier_request_count must be greater than or equal to 1024");
Expand Down
2 changes: 0 additions & 2 deletions nano/node/nodeconfig.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ class node_config
bool backup_before_upgrade{ false };
double max_work_generate_multiplier{ 64. };
uint32_t max_queued_requests{ 512 };
/** Maximum amount of confirmation requests (batches) to be sent to each channel */
uint32_t confirm_req_batches_max{ network_params.network.is_dev_network () ? 1u : 2u };
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;
Expand Down
10 changes: 10 additions & 0 deletions nano/node/transport/tcp.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@ namespace transport
return nano::transport::transport_type::tcp;
}

virtual bool full () override
{
bool result = true;
if (auto socket_l = socket.lock ())
{
result = socket_l->max ();
clemahieu marked this conversation as resolved.
Show resolved Hide resolved
}
return result;
}

private:
nano::tcp_endpoint endpoint{ boost::asio::ip::address_v6::any (), 0 };
};
Expand Down
4 changes: 4 additions & 0 deletions nano/node/transport/transport.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ namespace transport
virtual nano::endpoint get_endpoint () const = 0;
virtual nano::tcp_endpoint get_tcp_endpoint () const = 0;
virtual nano::transport::transport_type get_type () const = 0;
virtual bool full ()
{
return false;
}

std::chrono::steady_clock::time_point get_last_bootstrap_attempt () const
{
Expand Down