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.

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 decreases the confirm_req_batches_max from 20 to 2 #3148

Merged
merged 1 commit into from
Mar 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 3 additions & 4 deletions nano/core_test/confirmation_solicitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ TEST (confirmation_solicitor, batches)
// Solicitor will only solicit from this representative
nano::representative representative (nano::dev_genesis_key.pub, nano::genesis_amount, channel1);
std::vector<nano::representative> representatives{ representative };
nano::confirmation_solicitor solicitor (node2.network, node2.network_params.network);
nano::confirmation_solicitor solicitor (node2.network, node2.config);
solicitor.prepare (representatives);
// Ensure the representatives are correct
ASSERT_EQ (1, representatives.size ());
Expand All @@ -37,7 +37,6 @@ TEST (confirmation_solicitor, batches)
auto election (std::make_shared<nano::election> (node2, send, nullptr, nullptr, false, nano::election_behavior::normal));
ASSERT_FALSE (solicitor.add (*election));
}
ASSERT_EQ (1, solicitor.max_confirm_req_batches);
// Reached the maximum amount of requests for the channel
auto election (std::make_shared<nano::election> (node2, send, nullptr, nullptr, false, nano::election_behavior::normal));
ASSERT_TRUE (solicitor.add (*election));
Expand Down Expand Up @@ -66,7 +65,7 @@ TEST (confirmation_solicitor, different_hash)
// Solicitor will only solicit from this representative
nano::representative representative (nano::dev_genesis_key.pub, nano::genesis_amount, channel1);
std::vector<nano::representative> representatives{ representative };
nano::confirmation_solicitor solicitor (node2.network, node2.network_params.network);
nano::confirmation_solicitor solicitor (node2.network, node2.config);
solicitor.prepare (representatives);
// Ensure the representatives are correct
ASSERT_EQ (1, representatives.size ());
Expand Down Expand Up @@ -96,7 +95,7 @@ TEST (confirmation_solicitor, bypass_max_requests_cap)
node_flags.disable_udp = false;
auto & node1 = *system.add_node (node_flags);
auto & node2 = *system.add_node (node_flags);
nano::confirmation_solicitor solicitor (node2.network, node2.network_params.network);
nano::confirmation_solicitor solicitor (node2.network, node2.config);
std::vector<nano::representative> representatives;
auto max_representatives = std::max<size_t> (solicitor.max_election_requests, solicitor.max_election_broadcasts);
representatives.reserve (max_representatives + 1);
Expand Down
19 changes: 18 additions & 1 deletion nano/core_test/toml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,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.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 @@ -371,7 +372,7 @@ TEST (toml, array)
config_node.push<std::string> ("items", "item 1");
config_node.push<std::string> ("items", "item 2");
int i = 1;
config_node.array_entries_required<std::string> ("items", [&i](std::string item) {
config_node.array_entries_required<std::string> ("items", [&i] (std::string item) {
ASSERT_TRUE (item == std::string ("item ") + std::to_string (i));
i++;
});
Expand Down Expand Up @@ -589,6 +590,7 @@ 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 @@ -821,6 +823,21 @@ 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");
}
}

TEST (toml, daemon_read_config)
Expand Down
2 changes: 1 addition & 1 deletion nano/node/active_transactions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ void nano::active_transactions::request_confirm (nano::unique_lock<nano::mutex>

lock_a.unlock ();

nano::confirmation_solicitor solicitor (node.network, node.network_params.network);
nano::confirmation_solicitor solicitor (node.network, node.config);
solicitor.prepare (node.rep_crawler.principal_representatives (std::numeric_limits<size_t>::max ()));
nano::vote_generator_session generator_session (generator);

Expand Down
11 changes: 6 additions & 5 deletions nano/node/confirmation_solicitor.cpp
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
#include <nano/node/confirmation_solicitor.hpp>
#include <nano/node/election.hpp>
#include <nano/node/nodeconfig.hpp>

using namespace std::chrono_literals;

nano::confirmation_solicitor::confirmation_solicitor (nano::network & network_a, nano::network_constants const & params_a) :
max_confirm_req_batches (params_a.is_dev_network () ? 1 : 20),
max_block_broadcasts (params_a.is_dev_network () ? 4 : 30),
nano::confirmation_solicitor::confirmation_solicitor (nano::network & network_a, nano::node_config const & config_a) :
max_block_broadcasts (config_a.network_params.network.is_dev_network () ? 4 : 30),
max_election_requests (50),
max_election_broadcasts (std::max<size_t> (network_a.fanout () / 2, 1)),
network (network_a)
network (network_a),
config (config_a)
{
}

Expand Down Expand Up @@ -56,7 +57,7 @@ bool nano::confirmation_solicitor::add (nano::election const & election_a)
debug_assert (prepared);
bool error (true);
unsigned count = 0;
auto const max_channel_requests (max_confirm_req_batches * nano::network::confirm_req_hashes_max);
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 Down
6 changes: 3 additions & 3 deletions nano/node/confirmation_solicitor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ namespace nano
{
class election;
class node;
class node_config;
/** This class accepts elections that need further votes before they can be confirmed and bundles them in to single confirm_req packets */
class confirmation_solicitor final
{
public:
confirmation_solicitor (nano::network &, nano::network_constants const &);
confirmation_solicitor (nano::network &, nano::node_config const &);
/** Prepare object for batching election confirmation requests*/
void prepare (std::vector<nano::representative> const &);
/** Broadcast the winner of an election if the broadcast limit has not been reached. Returns false if the broadcast was performed */
Expand All @@ -22,8 +23,6 @@ class confirmation_solicitor final
bool add (nano::election const &);
/** Dispatch bundled requests to each channel*/
void flush ();
/** Maximum amount of confirmation requests (batches) to be sent to each channel */
size_t const max_confirm_req_batches;
/** Global maximum amount of block broadcasts */
size_t const max_block_broadcasts;
/** Maximum amount of requests to be sent per election, bypassed if an existing vote is for a different hash*/
Expand All @@ -33,6 +32,7 @@ class confirmation_solicitor final

private:
nano::network & network;
nano::node_config const & config;

unsigned rebroadcasted{ 0 };
std::vector<nano::representative> representatives_requests;
Expand Down
6 changes: 6 additions & 0 deletions nano/node/nodeconfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,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 ("confirm_req_batches_max", confirm_req_batches_max, "Limit for the number of confirmation requests for one channel per request attempt\ntype:uint32");

auto work_peers_l (toml.create_array ("work_peers", "A list of \"address:port\" entries to identify work peers."));
for (auto i (work_peers.begin ()), n (work_peers.end ()); i != n; ++i)
Expand Down Expand Up @@ -372,6 +373,7 @@ 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);

if (toml.has_key ("frontiers_confirmation"))
{
Expand Down Expand Up @@ -441,6 +443,10 @@ 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");
}
}
catch (std::runtime_error const & ex)
{
Expand Down
2 changes: 2 additions & 0 deletions nano/node/nodeconfig.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ class node_config
std::chrono::seconds work_watcher_period{ std::chrono::seconds (5) };
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