From 22794c2c1e4de1d9030e4eff61d6828535bc2826 Mon Sep 17 00:00:00 2001 From: Guilherme Lawless Date: Fri, 29 May 2020 13:09:20 +0100 Subject: [PATCH] Bisected election backtracking (#2778) * Add ledger::backtrack limited to 128 jumps * Bisect election dependencies when activating * Addressing special case where the winner may not have a loaded sideband * Simplify ledger::backtrack by specifying desired number of jumps instead. Limit is placed on the caller * Perform DB transactions without holding the active mutex by batch processing * Add a debug_assert on getting the previous block since it happens in the same tx * Add a pessimistic fallback by also starting an election for the first unconfirmed block * Set empty sideband, required by recent change (thanks Serg!) * Check if first unconfirmed block is not being processed by conf height * Already in active_transactions * Use std::min to avoid overflow (Wes review) * Use owns_lock from mutex (Wes) --- nano/core_test/confirmation_solicitor.cpp | 2 + nano/core_test/election.cpp | 79 +++++++++++++++++++++++ nano/core_test/ledger.cpp | 51 +++++++++++++++ nano/node/active_transactions.cpp | 76 ++++++++++++++++++++++ nano/node/active_transactions.hpp | 7 +- nano/node/election.cpp | 49 ++------------ nano/node/election.hpp | 3 + nano/secure/ledger.cpp | 13 ++++ nano/secure/ledger.hpp | 1 + 9 files changed, 236 insertions(+), 45 deletions(-) diff --git a/nano/core_test/confirmation_solicitor.cpp b/nano/core_test/confirmation_solicitor.cpp index 33c561a1c3..1d108712fc 100644 --- a/nano/core_test/confirmation_solicitor.cpp +++ b/nano/core_test/confirmation_solicitor.cpp @@ -29,6 +29,7 @@ TEST (confirmation_solicitor, batches) ASSERT_EQ (nano::test_genesis_key.pub, representatives.front ().account); ASSERT_TIMELY (3s, node2.network.size () == 1); auto send (std::make_shared (nano::genesis_hash, nano::keypair ().pub, nano::genesis_amount - 100, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (nano::genesis_hash))); + send->sideband_set ({}); { nano::lock_guard guard (node2.active.mutex); for (size_t i (0); i < nano::network::confirm_req_hashes_max; ++i) @@ -71,6 +72,7 @@ TEST (confirmation_solicitor, different_hash) ASSERT_EQ (nano::test_genesis_key.pub, representatives.front ().account); ASSERT_TIMELY (3s, node2.network.size () == 1); auto send (std::make_shared (nano::genesis_hash, nano::keypair ().pub, nano::genesis_amount - 100, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (nano::genesis_hash))); + send->sideband_set ({}); { nano::lock_guard guard (node2.active.mutex); auto election (std::make_shared (node2, send, nullptr, false)); diff --git a/nano/core_test/election.cpp b/nano/core_test/election.cpp index cb5e7cce88..61911e8f14 100644 --- a/nano/core_test/election.cpp +++ b/nano/core_test/election.cpp @@ -17,3 +17,82 @@ TEST (election, construction) election->transition_passive (); ASSERT_FALSE (election->idle ()); } + +namespace nano +{ +TEST (election, bisect_dependencies) +{ + nano::system system; + nano::node_flags flags; + flags.disable_request_loop = true; + auto & node = *system.add_node (flags); + nano::genesis genesis; + nano::confirmation_height_info conf_info; + ASSERT_FALSE (node.store.confirmation_height_get (node.store.tx_begin_read (), nano::test_genesis_key.pub, conf_info)); + ASSERT_EQ (1, conf_info.height); + std::vector> blocks; + blocks.push_back (nullptr); // idx == height + blocks.push_back (genesis.open); + nano::block_builder builder; + auto amount = nano::genesis_amount; + for (int i = 0; i < 299; ++i) + { + auto latest = blocks.back (); + blocks.push_back (builder.state () + .previous (latest->hash ()) + .account (nano::test_genesis_key.pub) + .representative (nano::test_genesis_key.pub) + .balance (--amount) + .link (nano::test_genesis_key.pub) + .sign (nano::test_genesis_key.prv, nano::test_genesis_key.pub) + .work (*system.work.generate (latest->hash ())) + .build ()); + ASSERT_EQ (nano::process_result::progress, node.process (*blocks.back ()).code); + } + ASSERT_EQ (301, blocks.size ()); + ASSERT_TRUE (node.active.empty ()); + { + auto election = node.active.insert (blocks.back ()).election; + ASSERT_NE (nullptr, election); + ASSERT_EQ (300, election->blocks.begin ()->second->sideband ().height); + nano::unique_lock lock (node.active.mutex); + election->activate_dependencies (); + node.active.activate_dependencies (lock); + } + // The first dependency activation also starts an election for the first unconfirmed block + ASSERT_EQ (3, node.active.size ()); + { + auto election = node.active.election (blocks[2]->qualified_root ()); + ASSERT_NE (nullptr, election); + ASSERT_EQ (2, election->blocks.begin ()->second->sideband ().height); + } + + auto check_height_and_activate_next = [&node, &blocks](uint64_t height_a) { + auto election = node.active.election (blocks[height_a]->qualified_root ()); + ASSERT_NE (nullptr, election); + ASSERT_EQ (height_a, election->blocks.begin ()->second->sideband ().height); + nano::unique_lock lock (node.active.mutex); + election->activate_dependencies (); + node.active.activate_dependencies (lock); + }; + check_height_and_activate_next (300 - 128); // ensure limited by 128 jumps + ASSERT_EQ (4, node.active.size ()); + check_height_and_activate_next (87); + ASSERT_EQ (5, node.active.size ()); + check_height_and_activate_next (44); + ASSERT_EQ (6, node.active.size ()); + check_height_and_activate_next (23); + ASSERT_EQ (7, node.active.size ()); + check_height_and_activate_next (12); + ASSERT_EQ (8, node.active.size ()); + check_height_and_activate_next (7); + ASSERT_EQ (9, node.active.size ()); + check_height_and_activate_next (4); + ASSERT_EQ (10, node.active.size ()); + check_height_and_activate_next (3); + ASSERT_EQ (10, node.active.size ()); // height 2 already inserted initially, no more blocks to activate + check_height_and_activate_next (2); + ASSERT_EQ (10, node.active.size ()); // conf height is 1, no more blocks to activate + ASSERT_EQ (node.active.blocks.size (), node.active.roots.size ()); +} +} diff --git a/nano/core_test/ledger.cpp b/nano/core_test/ledger.cpp index 181d64b41c..f29d82fd63 100644 --- a/nano/core_test/ledger.cpp +++ b/nano/core_test/ledger.cpp @@ -3132,3 +3132,54 @@ TEST (ledger, epoch_2_upgrade_callback) auto latest = upgrade_epoch (pool, ledger, nano::epoch::epoch_2); ASSERT_TRUE (cb_hit); } + +TEST (ledger, backtrack) +{ + nano::genesis genesis; + nano::stat stats; + nano::logger_mt logger; + auto store = nano::make_store (logger, nano::unique_path ()); + ASSERT_TRUE (!store->init_error ()); + bool cb_hit = false; + nano::ledger ledger (*store, stats, nano::generate_cache (), [&cb_hit]() { + cb_hit = true; + }); + { + auto transaction (store->tx_begin_write ()); + store->initialize (transaction, genesis, ledger.cache); + } + nano::work_pool pool (std::numeric_limits::max ()); + std::vector> blocks; + blocks.push_back (nullptr); // idx == height + blocks.push_back (genesis.open); + auto amount = nano::genesis_amount; + for (auto i = 0; i < 300; ++i) + { + nano::block_builder builder; + std::error_code ec; + auto latest = blocks.back (); + blocks.push_back (builder.state () + .previous (latest->hash ()) + .account (nano::test_genesis_key.pub) + .representative (nano::test_genesis_key.pub) + .balance (--amount) + .link (nano::test_genesis_key.pub) + .sign (nano::test_genesis_key.prv, nano::test_genesis_key.pub) + .work (*pool.generate (latest->hash ())) + .build (ec)); + ASSERT_FALSE (ec); + ASSERT_EQ (nano::process_result::progress, ledger.process (store->tx_begin_write (), *blocks.back ()).code); + } + ASSERT_EQ (302, blocks.size ()); + ASSERT_EQ (301, blocks[301]->sideband ().height); + auto transaction (store->tx_begin_read ()); + auto block_100 = ledger.backtrack (transaction, blocks[300], 200); + ASSERT_NE (nullptr, block_100); + ASSERT_EQ (*block_100, *blocks[100]); + ASSERT_NE (nullptr, ledger.backtrack (transaction, blocks[10], 10)); + ASSERT_NE (ledger.backtrack (transaction, blocks[10], 1), ledger.backtrack (transaction, blocks[11], 2)); + ASSERT_EQ (ledger.backtrack (transaction, blocks[1], 0), ledger.backtrack (transaction, blocks[1], 1)); + ASSERT_NE (ledger.backtrack (transaction, blocks[2], 0), ledger.backtrack (transaction, blocks[2], 1)); + ASSERT_EQ (nullptr, ledger.backtrack (transaction, nullptr, 0)); + ASSERT_EQ (nullptr, ledger.backtrack (transaction, nullptr, 10)); +} diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index beb2d6d9ba..5b5c0a2846 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -256,6 +256,7 @@ void nano::active_transactions::request_confirm (nano::unique_lock & solicitor.flush (); generator_session.flush (); lock_a.lock (); + activate_dependencies (lock_a); // This is updated after the loop to ensure slow machines don't do the full check often if (check_all_elections_l) @@ -294,6 +295,81 @@ void nano::active_transactions::frontiers_confirmation (nano::unique_lock & lock_a) +{ + debug_assert (lock_a.owns_lock ()); + decltype (pending_dependencies) pending_l; + pending_l.swap (pending_dependencies); + lock_a.unlock (); + // Store blocks to activate when the lock is re-acquired, adding the hash of the original election as a dependency + std::vector, nano::block_hash>> activate_l; + { + auto transaction = node.store.tx_begin_read (); + for (auto const & entry_l : pending_l) + { + auto const & block_l (entry_l.first); + auto const height_l (entry_l.second); + bool escalated_l (false); + auto previous_hash_l (block_l->previous ()); + if (!previous_hash_l.is_zero ()) + { + /* Insert first unconfirmed block (pessimistic) and bisect the chain (likelihood) */ + auto account (node.ledger.account (transaction, block_l->hash ())); + nano::confirmation_height_info conf_info_l; + if (!node.store.confirmation_height_get (transaction, account, conf_info_l)) + { + if (height_l > conf_info_l.height + 1 && !conf_info_l.frontier.is_zero ()) + { + auto successor_hash_l = node.store.block_successor (transaction, conf_info_l.frontier); + if (!confirmation_height_processor.is_processing_block (successor_hash_l)) + { + auto successor_l = node.store.block_get (transaction, successor_hash_l); + debug_assert (successor_l != nullptr); + if (successor_l != nullptr) + { + activate_l.emplace_back (successor_l, block_l->hash ()); + } + } + } + if (height_l > conf_info_l.height + 2) + { + auto const jumps_l = std::min (128, (height_l - conf_info_l.height) / 2); + auto backtracked_l (node.ledger.backtrack (transaction, block_l, jumps_l)); + if (backtracked_l != nullptr) + { + activate_l.emplace_back (backtracked_l, block_l->hash ()); + } + } + } + } + /* If previous block not existing/not commited yet, block_source can cause segfault for state blocks + So source check can be done only if previous != nullptr or previous is 0 (open account) */ + if (previous_hash_l.is_zero () || node.ledger.block_exists (previous_hash_l)) + { + auto source_hash_l (node.ledger.block_source (transaction, *block_l)); + if (!source_hash_l.is_zero () && source_hash_l != previous_hash_l && blocks.find (source_hash_l) == blocks.end ()) + { + auto source_l (node.store.block_get (transaction, source_hash_l)); + if (source_l != nullptr && !node.block_confirmed_or_being_confirmed (transaction, source_hash_l)) + { + activate_l.emplace_back (source_l, block_l->hash ()); + } + } + } + } + } + lock_a.lock (); + for (auto const & entry_l : activate_l) + { + auto election = insert_impl (entry_l.first); + if (election.inserted) + { + election.election->transition_active (); + election.election->dependent_blocks.insert (entry_l.second); + } + } +} + void nano::active_transactions::request_loop () { nano::unique_lock lock (mutex); diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index ddd3d96e2e..752423daf6 100644 --- a/nano/node/active_transactions.hpp +++ b/nano/node/active_transactions.hpp @@ -207,6 +207,8 @@ class active_transactions final void frontiers_confirmation (nano::unique_lock &); nano::account next_frontier_account{ 0 }; std::chrono::steady_clock::time_point next_frontier_check{ std::chrono::steady_clock::now () }; + void activate_dependencies (nano::unique_lock &); + std::vector, uint64_t>> pending_dependencies; nano::condition_variable condition; bool started{ false }; std::atomic stopped{ false }; @@ -262,6 +264,9 @@ class active_transactions final bool inactive_votes_bootstrap_check (std::vector const &, nano::block_hash const &, bool &); boost::thread thread; + friend class election; + friend std::unique_ptr collect_container_info (active_transactions &, const std::string &); + friend class active_transactions_dropped_cleanup_Test; friend class active_transactions_vote_replays_Test; friend class confirmation_height_prioritize_frontiers_Test; @@ -269,7 +274,7 @@ class active_transactions final friend class active_transactions_confirmation_consistency_Test; friend class active_transactions_vote_generator_session_Test; friend class node_vote_by_hash_bundle_Test; - friend std::unique_ptr collect_container_info (active_transactions &, const std::string &); + friend class election_bisect_dependencies_Test; }; std::unique_ptr collect_container_info (active_transactions & active_transactions, const std::string & name); diff --git a/nano/node/election.cpp b/nano/node/election.cpp index 7250aa3b32..7a0b54fcfb 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -27,7 +27,8 @@ nano::election::election (nano::node & node_a, std::shared_ptr bloc confirmation_action (confirmation_action_a), prioritized_m (prioritized_a), node (node_a), -status ({ block_a, 0, std::chrono::duration_cast (std::chrono::system_clock::now ().time_since_epoch ()), std::chrono::duration_values::zero (), 0, 1, 0, nano::election_status_type::ongoing }) +status ({ block_a, 0, std::chrono::duration_cast (std::chrono::system_clock::now ().time_since_epoch ()), std::chrono::duration_values::zero (), 0, 1, 0, nano::election_status_type::ongoing }), +height (block_a->sideband ().height) { last_votes.emplace (node.network_params.random.not_an_account, nano::vote_info{ std::chrono::steady_clock::now (), 0, block_a->hash () }); blocks.emplace (block_a->hash (), block_a); @@ -210,46 +211,8 @@ bool nano::election::confirmed () const void nano::election::activate_dependencies () { - auto transaction = node.store.tx_begin_read (); - bool escalated_l (false); - std::shared_ptr previous_l; - auto previous_hash_l (status.winner->previous ()); - if (!previous_hash_l.is_zero () && node.active.blocks.find (previous_hash_l) == node.active.blocks.end ()) - { - previous_l = node.store.block_get (transaction, previous_hash_l); - if (previous_l != nullptr && !node.block_confirmed_or_being_confirmed (transaction, previous_hash_l)) - { - auto election = node.active.insert_impl (previous_l); - if (election.inserted) - { - election.election->transition_active (); - escalated_l = true; - } - } - } - /* If previous block not existing/not commited yet, block_source can cause segfault for state blocks - So source check can be done only if previous != nullptr or previous is 0 (open account) */ - if (previous_hash_l.is_zero () || previous_l != nullptr) - { - auto source_hash_l (node.ledger.block_source (transaction, *status.winner)); - if (!source_hash_l.is_zero () && source_hash_l != previous_hash_l && node.active.blocks.find (source_hash_l) == node.active.blocks.end ()) - { - auto source_l (node.store.block_get (transaction, source_hash_l)); - if (source_l != nullptr && !node.block_confirmed_or_being_confirmed (transaction, source_hash_l)) - { - auto election = node.active.insert_impl (source_l); - if (election.inserted) - { - election.election->transition_active (); - escalated_l = true; - } - } - } - } - if (escalated_l) - { - update_dependent (); - } + debug_assert (!node.active.mutex.try_lock ()); + node.active.pending_dependencies.emplace_back (status.winner, height); } void nano::election::broadcast_block (nano::confirmation_solicitor & solicitor_a) @@ -266,7 +229,7 @@ void nano::election::broadcast_block (nano::confirmation_solicitor & solicitor_a bool nano::election::transition_time (nano::confirmation_solicitor & solicitor_a) { debug_assert (!node.active.mutex.try_lock ()); - nano::unique_lock lock (timepoints_mutex); + nano::lock_guard guard (timepoints_mutex); bool result = false; switch (state_m) { @@ -293,9 +256,7 @@ bool nano::election::transition_time (nano::confirmation_solicitor & solicitor_a if (base_latency () * active_broadcasting_duration_factor < std::chrono::steady_clock::now () - state_start) { state_change (nano::election::state_t::broadcasting, nano::election::state_t::backtracking); - lock.unlock (); activate_dependencies (); - lock.lock (); } break; case nano::election::state_t::backtracking: diff --git a/nano/node/election.hpp b/nano/node/election.hpp index 6ceb7f298b..a30df891ec 100644 --- a/nano/node/election.hpp +++ b/nano/node/election.hpp @@ -111,5 +111,8 @@ class election final : public std::enable_shared_from_this std::unordered_map last_tally; std::unordered_set dependent_blocks; std::chrono::seconds late_blocks_delay{ 5 }; + uint64_t const height; + + friend class election_bisect_dependencies_Test; }; } diff --git a/nano/secure/ledger.cpp b/nano/secure/ledger.cpp index 01ad49e06c..a8a4d8d499 100644 --- a/nano/secure/ledger.cpp +++ b/nano/secure/ledger.cpp @@ -1178,6 +1178,19 @@ std::shared_ptr nano::ledger::forked_block (nano::transaction const return result; } +std::shared_ptr nano::ledger::backtrack (nano::transaction const & transaction_a, std::shared_ptr const & start_a, uint64_t jumps_a) +{ + auto block = start_a; + while (jumps_a > 0 && block != nullptr && !block->previous ().is_zero ()) + { + block = store.block_get (transaction_a, block->previous ()); + debug_assert (block != nullptr); + --jumps_a; + } + debug_assert (block == nullptr || block->previous ().is_zero () || jumps_a == 0); + return block; +} + bool nano::ledger::block_confirmed (nano::transaction const & transaction_a, nano::block_hash const & hash_a) const { auto confirmed (false); diff --git a/nano/secure/ledger.hpp b/nano/secure/ledger.hpp index fa5027b808..3d83b28c54 100644 --- a/nano/secure/ledger.hpp +++ b/nano/secure/ledger.hpp @@ -25,6 +25,7 @@ class ledger final nano::uint128_t weight (nano::account const &); std::shared_ptr successor (nano::transaction const &, nano::qualified_root const &); std::shared_ptr forked_block (nano::transaction const &, nano::block const &); + std::shared_ptr backtrack (nano::transaction const &, std::shared_ptr const &, uint64_t); bool block_confirmed (nano::transaction const & transaction_a, nano::block_hash const & hash_a) const; bool block_not_confirmed_or_not_exists (nano::block const & block_a) const; nano::block_hash latest (nano::transaction const &, nano::account const &);