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

Use correct index when iterating prioritized frontiers #2069

Merged
merged 5 commits into from Jun 10, 2019
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

Next

Use correct index when iterating multi-index container

  • Loading branch information...
wezrule committed Jun 7, 2019
commit 87b8aec289ca4ec6fdf004dcf5bb6b14d754dfce
@@ -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 ());
}
}
}

@@ -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))
{
@@ -56,33 +58,34 @@ void nano::active_transactions::confirm_frontiers (nano::transaction const & tra
prioritize_frontiers_for_confirmation (transaction_a, is_test_network ? std::chrono::milliseconds (50) : std::chrono::seconds (2));

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 = *priority_cementable_frontiers.get<1> ().begin ();
priority_cementable_frontiers.get<1> ().erase (priority_cementable_frontiers.get<1> ().begin ());
This conversation was marked as resolved by wezrule

This comment has been minimized.

Copy link
@SergiySW

SergiySW Jun 9, 2019

Collaborator

2 times get same info priority_cementable_frontiers.get<1> ().begin ()

This comment has been minimized.

Copy link
@wezrule

wezrule Jun 10, 2019

Author Collaborator

Made it a separate variable

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))
{
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));
lk.lock ();
auto root_exists = roots.find (block->qualified_root ()) != roots.end ();
lk.unlock ();
if (!root_exists && !start (block))
This conversation was marked as resolved by wezrule

This comment has been minimized.

Copy link
@SergiySW

SergiySW Jun 9, 2019

Collaborator

I don't think we should check root existence here because it's in start () / add () function

This comment has been minimized.

Copy link
@wezrule

wezrule Jun 10, 2019

Author Collaborator

ok removed

{
++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 ();

// 4 times slower check if all frontiers were confirmed
int fully_confirmed_factor = (elections_count < max_elections) ? 4 : 1;
// Calculate next check time
@@ -344,18 +347,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);
std::unique_lock<std::mutex> lk (mutex);
auto priority_cementable_frontiers_size = priority_cementable_frontiers.size ();
lk.unlock ();
for (; i != n && !stopped && (priority_cementable_frontiers_size < max_priority_cementable_frontiers); ++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 ())
{
@@ -372,7 +376,7 @@ void nano::active_transactions::prioritize_frontiers_for_confirmation (nano::tra
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;
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.