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

Remove active_elections::trim #4611

Merged
merged 5 commits into from
May 13, 2024
Merged

Remove active_elections::trim #4611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d241751
Remove election_behavior argument for manual scheduler insertion.
clemahieu May 9, 2024
f9d1b88
Splitting manual election_behavior from normal.
clemahieu May 9, 2024
433e0c9
Renaming election_behavior::normal to election_behavior::priority
clemahieu May 10, 2024
f0a8dea
Remove default nano::election_behavior arguments
clemahieu May 10, 2024
53589b3
Remove active_elections::trim
clemahieu May 10, 2024
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
107 changes: 12 additions & 95 deletions nano/core_test/active_elections.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ TEST (active_elections, dropped_cleanup)
ASSERT_FALSE (node.network.publish_filter.apply (block_bytes.data (), block_bytes.size ()));

// An election was recently dropped
ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::normal));
ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::manual));

// Block cleared from active
ASSERT_FALSE (node.vote_router.active (hash));
Expand All @@ -687,7 +687,7 @@ TEST (active_elections, dropped_cleanup)
ASSERT_TRUE (node.network.publish_filter.apply (block_bytes.data (), block_bytes.size ()));

// Not dropped
ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::normal));
ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::manual));

// Block cleared from active
ASSERT_FALSE (node.vote_router.active (hash));
Expand Down Expand Up @@ -1325,105 +1325,22 @@ TEST (active_elections, vacancy)
.build ();
node.active.vacancy_update = [&updated] () { updated = true; };
ASSERT_EQ (nano::block_status::progress, node.process (send));
ASSERT_EQ (1, node.active.vacancy ());
ASSERT_EQ (1, node.active.vacancy (nano::election_behavior::priority));
ASSERT_EQ (0, node.active.size ());
node.scheduler.priority.activate (node.ledger.tx_begin_read (), nano::dev::genesis_key.pub);
ASSERT_TIMELY (1s, updated);
updated = false;
ASSERT_EQ (0, node.active.vacancy ());
ASSERT_EQ (0, node.active.vacancy (nano::election_behavior::priority));
ASSERT_EQ (1, node.active.size ());
auto election1 = node.active.election (send->qualified_root ());
ASSERT_NE (nullptr, election1);
election1->force_confirm ();
ASSERT_TIMELY (1s, updated);
ASSERT_EQ (1, node.active.vacancy ());
ASSERT_EQ (1, node.active.vacancy (nano::election_behavior::priority));
ASSERT_EQ (0, node.active.size ());
}
}

// Ensure transactions in excess of capacity are removed in fifo order
TEST (active_elections, fifo)
{
nano::test::system system{};

nano::node_config config = system.default_config ();
config.active_elections.size = 1;
config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;

auto & node = *system.add_node (config);
auto latest_hash = nano::dev::genesis->hash ();
nano::keypair key0{};
nano::state_block_builder builder{};

// Construct two pending entries that can be received simultaneously
auto send1 = builder.make_block ()
.previous (latest_hash)
.account (nano::dev::genesis_key.pub)
.representative (nano::dev::genesis_key.pub)
.link (key0.pub)
.balance (nano::dev::constants.genesis_amount - 1)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (*system.work.generate (latest_hash))
.build ();
ASSERT_EQ (nano::block_status::progress, node.process (send1));
node.process_confirmed (nano::election_status{ send1 });
ASSERT_TIMELY (5s, node.block_confirmed (send1->hash ()));

nano::keypair key1{};
latest_hash = send1->hash ();
auto send2 = builder.make_block ()
.previous (latest_hash)
.account (nano::dev::genesis_key.pub)
.representative (nano::dev::genesis_key.pub)
.link (key1.pub)
.balance (nano::dev::constants.genesis_amount - 2)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (*system.work.generate (latest_hash))
.build ();
ASSERT_EQ (nano::block_status::progress, node.process (send2));
node.process_confirmed (nano::election_status{ send2 });
ASSERT_TIMELY (5s, node.block_confirmed (send2->hash ()));

auto receive1 = builder.make_block ()
.previous (0)
.account (key0.pub)
.representative (nano::dev::genesis_key.pub)
.link (send1->hash ())
.balance (1)
.sign (key0.prv, key0.pub)
.work (*system.work.generate (key0.pub))
.build ();
ASSERT_EQ (nano::block_status::progress, node.process (receive1));

auto receive2 = builder.make_block ()
.previous (0)
.account (key1.pub)
.representative (nano::dev::genesis_key.pub)
.link (send2->hash ())
.balance (1)
.sign (key1.prv, key1.pub)
.work (*system.work.generate (key1.pub))
.build ();
ASSERT_EQ (nano::block_status::progress, node.process (receive2));

// Ensure first transaction becomes active
node.scheduler.manual.push (receive1);
ASSERT_TIMELY (5s, node.active.election (receive1->qualified_root ()) != nullptr);

// Ensure second transaction becomes active
node.scheduler.manual.push (receive2);
ASSERT_TIMELY (5s, node.active.election (receive2->qualified_root ()) != nullptr);

// Ensure excess transactions get trimmed
ASSERT_TIMELY_EQ (5s, node.active.size (), 1);

// Ensure overflow stats have been incremented
ASSERT_EQ (1, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::normal));

// Ensure the surviving transaction is the least recently inserted
ASSERT_TIMELY (1s, node.active.election (receive2->qualified_root ()) != nullptr);
}

/*
* Ensures we limit the number of vote hinted elections in AEC
*/
Expand Down Expand Up @@ -1487,7 +1404,7 @@ TEST (active_elections, limit_vote_hinted_elections)
ASSERT_TIMELY (5s, nano::test::active (node, { open1 }));

// Ensure there was no overflow of elections
ASSERT_EQ (0, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::normal));
ASSERT_EQ (0, node.stats.count (nano::stat::type::active_dropped, nano::stat::detail::priority));
}

/*
Expand Down Expand Up @@ -1521,9 +1438,9 @@ TEST (active_elections, allow_limited_overflow)
}

// Ensure number of active elections reaches AEC limit and there is no overfill
ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit ());
ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit (nano::election_behavior::priority));
// And it stays that way without increasing
ASSERT_ALWAYS (1s, node.active.size () == node.active.limit ());
ASSERT_ALWAYS (1s, node.active.size () == node.active.limit (nano::election_behavior::priority));

// Insert votes for the second part of the blocks, so that those are scheduled as hinted elections
for (auto const & block : blocks2)
Expand All @@ -1534,9 +1451,9 @@ TEST (active_elections, allow_limited_overflow)
}

// Ensure active elections overfill AEC only up to normal + hinted limit
ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit () + node.active.limit (nano::election_behavior::hinted));
ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit (nano::election_behavior::priority) + node.active.limit (nano::election_behavior::hinted));
// And it stays that way without increasing
ASSERT_ALWAYS (1s, node.active.size () == node.active.limit () + node.active.limit (nano::election_behavior::hinted));
ASSERT_ALWAYS (1s, node.active.size () == node.active.limit (nano::election_behavior::priority) + node.active.limit (nano::election_behavior::hinted));
}

/*
Expand Down Expand Up @@ -1583,7 +1500,7 @@ TEST (active_elections, allow_limited_overflow_adapt)
}

// Ensure number of active elections reaches AEC limit and there is no overfill
ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit ());
ASSERT_TIMELY_EQ (5s, node.active.size (), node.active.limit (nano::election_behavior::priority));
// And it stays that way without increasing
ASSERT_ALWAYS (1s, node.active.size () == node.active.limit ());
ASSERT_ALWAYS (1s, node.active.size () == node.active.limit (nano::election_behavior::priority));
}
10 changes: 5 additions & 5 deletions nano/core_test/confirmation_solicitor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ TEST (confirmation_solicitor, batches)
nano::lock_guard<nano::mutex> guard (node2.active.mutex);
for (size_t i (0); i < nano::network::confirm_req_hashes_max; ++i)
{
auto election (std::make_shared<nano::election> (node2, send, nullptr, nullptr, nano::election_behavior::normal));
auto election (std::make_shared<nano::election> (node2, send, nullptr, nullptr, nano::election_behavior::priority));
ASSERT_FALSE (solicitor.add (*election));
}
// Reached the maximum amount of requests for the channel
auto election (std::make_shared<nano::election> (node2, send, nullptr, nullptr, nano::election_behavior::normal));
auto election (std::make_shared<nano::election> (node2, send, nullptr, nullptr, nano::election_behavior::priority));
// 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 Expand Up @@ -92,7 +92,7 @@ TEST (confirmation_solicitor, different_hash)
.work (*system.work.generate (nano::dev::genesis->hash ()))
.build ();
send->sideband_set ({});
auto election (std::make_shared<nano::election> (node2, send, nullptr, nullptr, nano::election_behavior::normal));
auto election (std::make_shared<nano::election> (node2, send, nullptr, nullptr, nano::election_behavior::priority));
// Add a vote for something else, not the winner
election->last_votes[representative.account] = { std::chrono::steady_clock::now (), 1, 1 };
// Ensure the request and broadcast goes through
Expand Down Expand Up @@ -136,7 +136,7 @@ TEST (confirmation_solicitor, bypass_max_requests_cap)
.work (*system.work.generate (nano::dev::genesis->hash ()))
.build ();
send->sideband_set ({});
auto election (std::make_shared<nano::election> (node2, send, nullptr, nullptr, nano::election_behavior::normal));
auto election (std::make_shared<nano::election> (node2, send, nullptr, nullptr, nano::election_behavior::priority));
// Add a vote for something else, not the winner
for (auto const & rep : representatives)
{
Expand All @@ -149,7 +149,7 @@ TEST (confirmation_solicitor, bypass_max_requests_cap)
ASSERT_TIMELY_EQ (6s, max_representatives + 1, node2.stats.count (nano::stat::type::message, nano::stat::detail::confirm_req, nano::stat::dir::out));

solicitor.prepare (representatives);
auto election2 (std::make_shared<nano::election> (node2, send, nullptr, nullptr, nano::election_behavior::normal));
auto election2 (std::make_shared<nano::election> (node2, send, nullptr, nullptr, nano::election_behavior::priority));
ASSERT_FALSE (solicitor.add (*election2));
ASSERT_FALSE (solicitor.broadcast (*election2));

Expand Down
4 changes: 2 additions & 2 deletions nano/core_test/election.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ TEST (election, construction)
nano::test::system system (1);
auto & node = *system.nodes[0];
auto election = std::make_shared<nano::election> (
node, nano::dev::genesis, [] (auto const &) {}, [] (auto const &) {}, nano::election_behavior::normal);
node, nano::dev::genesis, [] (auto const &) {}, [] (auto const &) {}, nano::election_behavior::priority);
}

TEST (election, behavior)
Expand All @@ -27,7 +27,7 @@ TEST (election, behavior)
auto chain = nano::test::setup_chain (system, *system.nodes[0], 1, nano::dev::genesis_key, false);
auto election = nano::test::start_election (system, *system.nodes[0], chain[0]->hash ());
ASSERT_NE (nullptr, election);
ASSERT_EQ (nano::election_behavior::normal, election->behavior ());
ASSERT_EQ (nano::election_behavior::manual, election->behavior ());
}

TEST (election, quorum_minimum_flip_success)
Expand Down
3 changes: 2 additions & 1 deletion nano/lib/stats_enums.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ enum class detail
broadcast_block_repeat,

// election types
normal,
manual,
priority,
hinted,
optimistic,

Expand Down
40 changes: 10 additions & 30 deletions nano/node/active_elections.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,11 @@ int64_t nano::active_elections::limit (nano::election_behavior behavior) const
{
switch (behavior)
{
case nano::election_behavior::normal:
case nano::election_behavior::manual:
{
return std::numeric_limits<int64_t>::max ();
}
case nano::election_behavior::priority:
{
return static_cast<int64_t> (config.size);
}
Expand All @@ -212,8 +216,10 @@ int64_t nano::active_elections::vacancy (nano::election_behavior behavior) const
nano::lock_guard<nano::mutex> guard{ mutex };
switch (behavior)
{
case nano::election_behavior::normal:
return limit () - static_cast<int64_t> (roots.size ());
case nano::election_behavior::manual:
return std::numeric_limits<int64_t>::max ();
case nano::election_behavior::priority:
return limit (nano::election_behavior::priority) - static_cast<int64_t> (roots.size ());
case nano::election_behavior::hinted:
case nano::election_behavior::optimistic:
return limit (behavior) - count_by_behavior[behavior];
Expand Down Expand Up @@ -359,20 +365,6 @@ void nano::active_elections::request_loop ()
}
}

void nano::active_elections::trim ()
{
/*
* Both normal and hinted election schedulers are well-behaved, meaning they first check for AEC vacancy before inserting new elections.
* However, it is possible that AEC will be temporarily overfilled in case it's running at full capacity and election hinting or manual queue kicks in.
* That case will lead to unwanted churning of elections, so this allows for AEC to be overfilled to 125% until erasing of elections happens.
*/
while (vacancy () < -(limit () / 4))
{
node.stats.inc (nano::stat::type::active, nano::stat::detail::erase_oldest);
erase_oldest ();
}
}

nano::election_insertion_result nano::active_elections::insert (std::shared_ptr<nano::block> const & block_a, nano::election_behavior election_behavior_a)
{
debug_assert (block_a);
Expand Down Expand Up @@ -443,8 +435,6 @@ nano::election_insertion_result nano::active_elections::insert (std::shared_ptr<
result.election->broadcast_vote ();
}

trim ();

return result;
}

Expand Down Expand Up @@ -489,16 +479,6 @@ bool nano::active_elections::erase (nano::qualified_root const & root_a)
return false;
}

void nano::active_elections::erase_oldest ()
{
nano::unique_lock<nano::mutex> lock{ mutex };
if (!roots.empty ())
{
auto item = roots.get<tag_sequenced> ().front ();
cleanup_election (lock, item.election);
}
}

bool nano::active_elections::empty () const
{
nano::lock_guard<nano::mutex> lock{ mutex };
Expand Down Expand Up @@ -557,7 +537,7 @@ std::unique_ptr<nano::container_info_component> nano::collect_container_info (ac
auto composite = std::make_unique<container_info_composite> (name);
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "roots", active_elections.roots.size (), sizeof (decltype (active_elections.roots)::value_type) }));
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "election_winner_details", active_elections.election_winner_details_size (), sizeof (decltype (active_elections.election_winner_details)::value_type) }));
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "normal", static_cast<std::size_t> (active_elections.count_by_behavior[nano::election_behavior::normal]), 0 }));
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "normal", static_cast<std::size_t> (active_elections.count_by_behavior[nano::election_behavior::priority]), 0 }));
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "hinted", static_cast<std::size_t> (active_elections.count_by_behavior[nano::election_behavior::hinted]), 0 }));
composite->add_component (std::make_unique<container_info_leaf> (container_info{ "optimistic", static_cast<std::size_t> (active_elections.count_by_behavior[nano::election_behavior::optimistic]), 0 }));

Expand Down
9 changes: 3 additions & 6 deletions nano/node/active_elections.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class active_elections final
/**
* Starts new election with a specified behavior type
*/
nano::election_insertion_result insert (std::shared_ptr<nano::block> const &, nano::election_behavior = nano::election_behavior::normal);
nano::election_insertion_result insert (std::shared_ptr<nano::block> const &, nano::election_behavior = nano::election_behavior::priority);
// Is the root of this block in the roots container
bool active (nano::block const &) const;
bool active (nano::qualified_root const &) const;
Expand All @@ -117,7 +117,6 @@ class active_elections final
std::vector<std::shared_ptr<nano::election>> list_active (std::size_t = std::numeric_limits<std::size_t>::max ());
bool erase (nano::block const &);
bool erase (nano::qualified_root const &);
void erase_oldest ();
bool empty () const;
std::size_t size () const;
bool publish (std::shared_ptr<nano::block> const &);
Expand All @@ -128,20 +127,18 @@ class active_elections final
* Maximum number of elections that should be present in this container
* NOTE: This is only a soft limit, it is possible for this container to exceed this count
*/
int64_t limit (nano::election_behavior behavior = nano::election_behavior::normal) const;
int64_t limit (nano::election_behavior behavior) const;
/**
* How many election slots are available for specified election type
*/
int64_t vacancy (nano::election_behavior behavior = nano::election_behavior::normal) const;
int64_t vacancy (nano::election_behavior behavior) const;
std::function<void ()> vacancy_update{ [] () {} };

std::size_t election_winner_details_size ();
void add_election_winner_details (nano::block_hash const &, std::shared_ptr<nano::election> const &);
std::shared_ptr<nano::election> remove_election_winner_details (nano::block_hash const &);

private:
// Erase elections if we're over capacity
void trim ();
void request_loop ();
void request_confirm (nano::unique_lock<nano::mutex> &);
// Erase all blocks from active and, if not confirmed, clear digests from network filters
Expand Down
6 changes: 4 additions & 2 deletions nano/node/election.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ std::chrono::milliseconds nano::election::confirm_req_time () const
{
switch (behavior ())
{
case election_behavior::normal:
case election_behavior::manual:
case election_behavior::priority:
case election_behavior::hinted:
return base_latency () * 5;
case election_behavior::optimistic:
Expand Down Expand Up @@ -295,7 +296,8 @@ std::chrono::milliseconds nano::election::time_to_live () const
{
switch (behavior ())
{
case election_behavior::normal:
case election_behavior::manual:
case election_behavior::priority:
return std::chrono::milliseconds (5 * 60 * 1000);
case election_behavior::hinted:
case election_behavior::optimistic:
Expand Down
Loading
Loading