Skip to content

Commit

Permalink
Use correct index when iterating prioritized frontiers (#2069)
Browse files Browse the repository at this point in the history 
* Use correct index when iterating multi-index container

* Overwrite frontiers with low uncemented blocks with frontiers containing higher counts when max frontiers is reached

* Review comments

* Re-add removed stopped check in request loop

* Formatting
  • Loading branch information
@wezrule
wezrule authored Jun 10, 2019
1 parent efaed45 commit 1669fd8
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 25 deletions.
30 changes: 24 additions & 6 deletions nano/core_test/node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2837,24 +2837,42 @@ TEST (confirmation_height, prioritize_frontiers)
ASSERT_TRUE (priority_orders_match (desired_order_1) || priority_orders_match (desired_order_2));

// Check that accounts which already exist have their order modified when the uncemented count changes.
nano::send_block send12 (send9.hash (), nano::test_genesis_key.pub, 100, key3.prv, key3.pub, system.work.generate (send9.hash ()));
nano::send_block send13 (send12.hash (), nano::test_genesis_key.pub, 90, key3.prv, key3.pub, system.work.generate (send12.hash ()));
nano::send_block send14 (send13.hash (), nano::test_genesis_key.pub, 80, key3.prv, key3.pub, system.work.generate (send13.hash ()));
nano::send_block send15 (send14.hash (), nano::test_genesis_key.pub, 70, key3.prv, key3.pub, system.work.generate (send14.hash ()));
nano::send_block send16 (send15.hash (), nano::test_genesis_key.pub, 60, key3.prv, key3.pub, system.work.generate (send15.hash ()));
nano::send_block send17 (send16.hash (), nano::test_genesis_key.pub, 50, key3.prv, key3.pub, system.work.generate (send16.hash ()));
{
auto transaction = node->store.tx_begin_write ();
nano::send_block send12 (send9.hash (), nano::test_genesis_key.pub, 100, key3.prv, key3.pub, system.work.generate (send9.hash ()));
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, send12).code);
nano::send_block send13 (send12.hash (), nano::test_genesis_key.pub, 90, key3.prv, key3.pub, system.work.generate (send12.hash ()));
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, send13).code);
nano::send_block send14 (send13.hash (), nano::test_genesis_key.pub, 80, key3.prv, key3.pub, system.work.generate (send13.hash ()));
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, send14).code);
nano::send_block send15 (send14.hash (), nano::test_genesis_key.pub, 70, key3.prv, key3.pub, system.work.generate (send14.hash ()));
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, send15).code);
nano::send_block send16 (send15.hash (), nano::test_genesis_key.pub, 60, key3.prv, key3.pub, system.work.generate (send15.hash ()));
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, send16).code);
nano::send_block send17 (send16.hash (), nano::test_genesis_key.pub, 50, key3.prv, key3.pub, system.work.generate (send16.hash ()));
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, send17).code);
}
transaction.refresh ();
node->active.prioritize_frontiers_for_confirmation (transaction, std::chrono::seconds (1));
ASSERT_TRUE (priority_orders_match ({ key3.pub, nano::genesis_account, key4.pub, key1.pub, key2.pub }));

// Check that the active transactions roots contains the frontiers
{
std::lock_guard<std::mutex> guard (node->active.mutex);
node->active.next_frontier_check = std::chrono::steady_clock::now ();
}

system.deadline_set (std::chrono::seconds (10));
while (node->active.size () != num_accounts)
{
ASSERT_NO_ERROR (system.poll ());
}

std::array<nano::qualified_root, num_accounts> frontiers{ send17.qualified_root (), send6.qualified_root (), send7.qualified_root (), open2.qualified_root (), send11.qualified_root () };
for (auto & frontier : frontiers)
{
ASSERT_NE (node->active.roots.find (frontier), node->active.roots.end ());
}
}
}

Expand Down
64 changes: 45 additions & 19 deletions nano/node/active_transactions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ void nano::active_transactions::confirm_frontiers (nano::transaction const & tra
int test_network_factor = is_test_network ? 1000 : 1;
auto roots_size = size ();
auto max_elections = (max_broadcast_queue / 4);
std::unique_lock<std::mutex> lk (mutex);
auto check_time_exceeded = std::chrono::steady_clock::now () >= next_frontier_check;
lk.unlock ();
auto low_active_elections = roots_size < max_elections;
if (check_time_exceeded || (!is_test_network && low_active_elections))
{
Expand All @@ -53,38 +55,39 @@ void nano::active_transactions::confirm_frontiers (nano::transaction const & tra
}

// Spend time prioritizing accounts to reduce voting traffic
prioritize_frontiers_for_confirmation (transaction_a, is_test_network ? std::chrono::milliseconds (50) : std::chrono::seconds (2));
auto time_to_spend_prioritizing = (frontiers_fully_confirmed ? std::chrono::milliseconds (200) : std::chrono::seconds (2));
prioritize_frontiers_for_confirmation (transaction_a, is_test_network ? std::chrono::milliseconds (50) : time_to_spend_prioritizing);

size_t elections_count (0);
std::unique_lock<std::mutex> lock (mutex);
lk.lock ();
while (!priority_cementable_frontiers.empty () && !stopped && elections_count < max_elections)
{
auto cementable_account = *priority_cementable_frontiers.begin ();
priority_cementable_frontiers.erase (priority_cementable_frontiers.begin ());
lock.unlock ();
auto cementable_account_front_it = priority_cementable_frontiers.get<1> ().begin ();
auto cementable_account = *cementable_account_front_it;
priority_cementable_frontiers.get<1> ().erase (cementable_account_front_it);
lk.unlock ();
nano::account_info info;
auto error = node.store.account_get (transaction_a, cementable_account.account, info);
release_assert (!error);

if (info.block_count > info.confirmation_height && !this->node.pending_confirmation_height.is_processing_block (info.head))
if (info.block_count > info.confirmation_height && !node.pending_confirmation_height.is_processing_block (info.head))
{
auto block (this->node.store.block_get (transaction_a, info.head));
if (!this->start (block))
auto block (node.store.block_get (transaction_a, info.head));
if (!start (block))
{
++elections_count;
// Calculate votes for local representatives
if (representative)
{
this->node.block_processor.generator.add (block->hash ());
node.block_processor.generator.add (block->hash ());
}
}
}
lock.lock ();
lk.lock ();
}
lock.unlock ();

frontiers_fully_confirmed = (elections_count < max_elections);
// 4 times slower check if all frontiers were confirmed
int fully_confirmed_factor = (elections_count < max_elections) ? 4 : 1;
auto fully_confirmed_factor = frontiers_fully_confirmed ? 4 : 1;
// Calculate next check time
next_frontier_check = steady_clock::now () + (representative_factor * fully_confirmed_factor / test_network_factor);
}
Expand Down Expand Up @@ -330,6 +333,12 @@ void nano::active_transactions::request_loop ()
{
request_confirm (lock);
update_active_difficulty (lock);

// This prevents unnecessary waiting if stopped is set in-between the above check and now
if (stopped)
{
break;
}
const auto extra_delay (std::min (roots.size (), max_broadcast_queue) * node.network.broadcast_interval_ms * 2);
condition.wait_for (lock, std::chrono::milliseconds (node.network_params.network.request_interval_ms + extra_delay));
}
Expand All @@ -342,18 +351,19 @@ void nano::active_transactions::prioritize_frontiers_for_confirmation (nano::tra
timer.start ();
if (node.pending_confirmation_height.size () < confirmed_frontiers_max_pending_cut_off)
{
auto priority_cementable_frontiers_size = priority_cementable_frontiers.size ();
auto i (node.store.latest_begin (transaction_a, next_frontier_account));
auto n (node.store.latest_end ());
std::unique_lock<std::mutex> lock_a (mutex, std::defer_lock);
for (; i != n && !stopped && (priority_cementable_frontiers_size < max_priority_cementable_frontiers); ++i)
std::unique_lock<std::mutex> lk (mutex);
auto priority_cementable_frontiers_size = priority_cementable_frontiers.size ();
lk.unlock ();
for (; i != n && !stopped; ++i)
{
auto const & account (i->first);
auto const & info (i->second);
if (info.block_count > info.confirmation_height && !node.pending_confirmation_height.is_processing_block (info.head))
{
lock_a.lock ();
auto num_uncemented = info.block_count - info.confirmation_height;
lk.lock ();
auto it = priority_cementable_frontiers.find (account);
if (it != priority_cementable_frontiers.end ())
{
Expand All @@ -367,10 +377,26 @@ void nano::active_transactions::prioritize_frontiers_for_confirmation (nano::tra
}
else
{
priority_cementable_frontiers.emplace (account, num_uncemented);
assert (priority_cementable_frontiers_size <= max_priority_cementable_frontiers);
if (priority_cementable_frontiers_size == max_priority_cementable_frontiers)
{
// The maximum amount of frontiers stored has been reached. Check if the current frontier
// has more uncemented blocks than the lowest uncemented frontier in the collection if so replace it.
auto least_uncemented_frontier_it = priority_cementable_frontiers.get<1> ().end ();
--least_uncemented_frontier_it;
if (num_uncemented > least_uncemented_frontier_it->blocks_uncemented)
{
priority_cementable_frontiers.get<1> ().erase (least_uncemented_frontier_it);
priority_cementable_frontiers.emplace (account, num_uncemented);
}
}
else
{
priority_cementable_frontiers.emplace (account, num_uncemented);
}
}
priority_cementable_frontiers_size = priority_cementable_frontiers.size ();
lock_a.unlock ();
lk.unlock ();
}

next_frontier_account = account.number () + 1;
Expand Down
2 changes: 2 additions & 0 deletions nano/node/active_transactions.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,13 @@ class active_transactions final
boost::multi_index::member<nano::cementable_account, uint64_t, &nano::cementable_account::blocks_uncemented>,
std::greater<uint64_t>>>>
priority_cementable_frontiers;
bool frontiers_fully_confirmed{ false };
static size_t constexpr max_priority_cementable_frontiers{ 100000 };
static size_t constexpr confirmed_frontiers_max_pending_cut_off{ 1000 };
boost::thread thread;

friend class confirmation_height_prioritize_frontiers_Test;
friend class confirmation_height_prioritize_frontiers_overwrite_Test;
};

std::unique_ptr<seq_con_info_component> collect_seq_con_info (active_transactions & active_transactions, const std::string & name);
Expand Down
85 changes: 85 additions & 0 deletions nano/slow_test/node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -594,3 +594,88 @@ TEST (confirmation_height, long_chains)

ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in), num_blocks * 2 + 2);
}

namespace nano
{
TEST (confirmation_height, prioritize_frontiers_overwrite)
{
bool delay_frontier_confirmation_height_updating = true;
nano::system system;
nano::node_config node_config (24000, system.logging);
auto node = system.add_node (node_config, delay_frontier_confirmation_height_updating);
system.wallet (0)->insert_adhoc (nano::test_genesis_key.prv);

// As this test can take a while extend the next frontier check
{
std::lock_guard<std::mutex> guard (node->active.mutex);
node->active.next_frontier_check = std::chrono::steady_clock::now () + 3600s;
}

auto num_accounts = node->active.max_priority_cementable_frontiers;
nano::keypair last_keypair = nano::test_genesis_key;
auto last_open_hash = node->latest (nano::test_genesis_key.pub);
// Clear confirmation height so that the genesis account has the same amount of uncemented blocks as the other frontiers
{
auto transaction = node->store.tx_begin_write ();
node->store.confirmation_height_clear (transaction);
}

{
auto transaction = node->store.tx_begin_write ();
for (auto i = num_accounts - 1; i > 0; --i)
{
nano::keypair key;
system.wallet (0)->insert_adhoc (key.prv);

nano::send_block send (last_open_hash, key.pub, nano::Gxrb_ratio - 1, last_keypair.prv, last_keypair.pub, system.work.generate (last_open_hash));
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, send).code);
nano::open_block open (send.hash (), last_keypair.pub, key.pub, key.prv, key.pub, system.work.generate (key.pub));
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, open).code);
last_open_hash = open.hash ();
last_keypair = key;
}
}

auto transaction = node->store.tx_begin_read ();
{
node->active.prioritize_frontiers_for_confirmation (transaction, std::chrono::seconds (60));
ASSERT_EQ (node->active.priority_cementable_frontiers_size (), num_accounts);
auto last_frontier_it = node->active.priority_cementable_frontiers.get<1> ().end ();
--last_frontier_it;
ASSERT_EQ (last_frontier_it->account, last_keypair.pub);
ASSERT_EQ (last_frontier_it->blocks_uncemented, 1);
}

// Add a new frontier with 1 block, it should not be added to the frontier container
nano::keypair key;
auto latest_genesis = node->latest (nano::test_genesis_key.pub);
nano::send_block send (latest_genesis, key.pub, nano::Gxrb_ratio - 1, nano::test_genesis_key.prv, nano::test_genesis_key.pub, system.work.generate (latest_genesis));
nano::open_block open (send.hash (), nano::test_genesis_key.pub, key.pub, key.prv, key.pub, system.work.generate (key.pub));
{
auto transaction = node->store.tx_begin_write ();
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, send).code);
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, open).code);
}
transaction.refresh ();
node->active.prioritize_frontiers_for_confirmation (transaction, std::chrono::seconds (60));
ASSERT_EQ (node->active.priority_cementable_frontiers_size (), num_accounts);
auto last_frontier_it = node->active.priority_cementable_frontiers.get<1> ().end ();
--last_frontier_it;
ASSERT_EQ (last_frontier_it->account, last_keypair.pub);
ASSERT_EQ (last_frontier_it->blocks_uncemented, 1);

nano::send_block send1 (send.hash (), key.pub, nano::Gxrb_ratio - 2, nano::test_genesis_key.prv, nano::test_genesis_key.pub, system.work.generate (send.hash ()));
nano::receive_block receive (open.hash (), send1.hash (), key.prv, key.pub, system.work.generate (open.hash ()));
{
auto transaction = node->store.tx_begin_write ();
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, send1).code);
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, receive).code);
}

transaction.refresh ();
node->active.prioritize_frontiers_for_confirmation (transaction, std::chrono::seconds (5));
ASSERT_EQ (node->active.priority_cementable_frontiers_size (), num_accounts);
ASSERT_EQ (node->active.priority_cementable_frontiers.find (last_keypair.pub), node->active.priority_cementable_frontiers.end ());
ASSERT_NE (node->active.priority_cementable_frontiers.find (key.pub), node->active.priority_cementable_frontiers.end ());
}
}

0 comments on commit 1669fd8

Please sign in to comment.