Merge pull request #737 from evoskuil/master

Complete validation off-strand, making backlog counter atomic.

Author: evoskuil

include/bitcoin/node/chasers/chaser_check.hpp

@@ -61,13 +61,13 @@ class BCN_API chaser_check
 
     virtual void do_bump(height_t height) NOEXCEPT;
     virtual void do_checked(height_t height) NOEXCEPT;
+    virtual void do_advanced(height_t height) NOEXCEPT;
     virtual void do_headers(height_t branch_point) NOEXCEPT;
     virtual void do_regressed(height_t branch_point) NOEXCEPT;
     virtual void do_handle_purged(const code& ec) NOEXCEPT;
     virtual void do_get_hashes(const map_handler& handler) NOEXCEPT;
     virtual void do_put_hashes(const map_ptr& map,
         const network::result_handler& handler) NOEXCEPT;
-    virtual void do_confirmable(height_t height) NOEXCEPT;
 
 private:
     typedef std::deque<map_ptr> maps;
@@ -89,7 +89,7 @@ class BCN_API chaser_check
     // These are protected by strand.
     size_t inventory_{};
     size_t requested_{};
-    size_t confirmed_{};
+    size_t advanced_{};
     job::ptr job_{};
     maps maps_{};
 };

include/bitcoin/node/chasers/chaser_validate.hpp

@@ -19,6 +19,7 @@
 #ifndef LIBBITCOIN_NODE_CHASERS_CHASER_VALIDATE_HPP
 #define LIBBITCOIN_NODE_CHASERS_CHASER_VALIDATE_HPP
 
+#include <atomic>
 #include <bitcoin/database.hpp>
 #include <bitcoin/network.hpp>
 #include <bitcoin/node/chasers/chaser.hpp>
@@ -59,8 +60,6 @@ class BCN_API chaser_validate
         const system::chain::context& ctx) NOEXCEPT;
     virtual code populate(bool bypass, const system::chain::block& block,
         const system::chain::context& ctx) NOEXCEPT;
-    virtual void tracked_complete_block(const code& ec,
-        const database::header_link& link, size_t height, bool bypassed) NOEXCEPT;
     virtual void complete_block(const code& ec,
         const database::header_link& link, size_t height, bool bypassed) NOEXCEPT;
 
@@ -74,11 +73,11 @@ class BCN_API chaser_validate
         return backlog_ < maximum_backlog_;
     }
 
-    // These are protected by strand.
-    size_t backlog_{};
+    // This is protected by strand.
     network::threadpool threadpool_;
 
     // These are thread safe.
+    std::atomic<size_t> backlog_{};
     network::asio::strand independent_strand_;
     const uint32_t subsidy_interval_;
     const uint64_t initial_subsidy_;

src/chasers/chaser_check.cpp

@@ -129,10 +129,11 @@ bool chaser_check::handle_event(const code&, chase event_,
             POST(do_headers, std::get<height_t>(value));
             break;
         }
-        case chase::confirmable:
+        ////case chase::confirmable:
+        case chase::valid:
         {
             BC_ASSERT(std::holds_alternative<height_t>(value));
-            POST(do_confirmable, std::get<height_t>(value));
+            POST(do_advanced, std::get<height_t>(value));
             break;
         }
         case chase::stop:
@@ -205,15 +206,15 @@ void chaser_check::do_regressed(height_t branch_point) NOEXCEPT
 // track downloaded in order (to move download window)
 // ----------------------------------------------------------------------------
 
-void chaser_check::do_confirmable(height_t height) NOEXCEPT
+void chaser_check::do_advanced(height_t height) NOEXCEPT
 {
     BC_ASSERT(stranded());
 
     // Confirmations are ordered and notification order is guaranteed.
-    confirmed_ = height;
+    advanced_ = height;
 
     // The full set of requested hashes has been confirmed.
-    if (confirmed_ == requested_)
+    if (advanced_ == requested_)
         do_headers(height);
 }
 
@@ -345,8 +346,7 @@ size_t chaser_check::set_unassociated() NOEXCEPT
         return {};
 
     // Defer new work issuance until gaps filled and confirmation caught up.
-    if (position() < requested_ ||
-        confirmed_ < requested_)
+    if (position() < requested_ || advanced_ < requested_)
         return {};
 
     // Inventory size gets set only once.

src/chasers/chaser_validate.cpp

@@ -18,6 +18,7 @@
  */
 #include <bitcoin/node/chasers/chaser_validate.hpp>
 
+#include <atomic>
 #include <bitcoin/system.hpp>
 #include <bitcoin/node/chasers/chaser.hpp>
 #include <bitcoin/node/define.hpp>
@@ -171,6 +172,7 @@ void chaser_validate::do_bump(height_t) NOEXCEPT
             return;
         }
 
+        // block_unknown allowed here (debug reset).
         const auto bypass =
             (ec == database::error::block_valid) ||
             (ec == database::error::block_confirmable) ||
@@ -182,7 +184,7 @@ void chaser_validate::do_bump(height_t) NOEXCEPT
         }
         else
         {
-            ++backlog_;
+            backlog_.fetch_add(one, std::memory_order_relaxed);
             PARALLEL(validate_block, link, bypass);
         }
 
@@ -225,8 +227,10 @@ void chaser_validate::validate_block(const header_link& link,
         ec = error::validate5;
     }
 
+    backlog_.fetch_sub(one, std::memory_order_relaxed);
+
     // Return to strand to handle result.
-    POST(tracked_complete_block, ec, link, ctx.height, bypass);
+    complete_block(ec, link, ctx.height, bypass);
 }
 
 code chaser_validate::populate(bool bypass, const chain::block& block,
@@ -267,30 +271,19 @@ code chaser_validate::validate(bool bypass, const chain::block& block,
     if ((ec = block.connect(ctx)))
         return ec;
 
-    if (!query.set_prevouts(link, block))
-        return error::validate8;
+    if ((ec = query.set_prevouts(link, block)))
+        return ec;
 
     if (!query.set_block_valid(link, block.fees()))
-        return error::validate9;
+        return error::validate6;
 
     return ec;
 }
 
-// The size of the job is not relevant to the backlog cost.
-void chaser_validate::tracked_complete_block(const code& ec,
-    const header_link& link, size_t height, bool bypassed) NOEXCEPT
-{
-    BC_ASSERT(stranded());
-
-    --backlog_;
-    complete_block(ec, link, height, bypassed);
-}
-
+// Completes off strand.
 void chaser_validate::complete_block(const code& ec, const header_link& link,
     size_t height, bool bypassed) NOEXCEPT
 {
-    BC_ASSERT(stranded());
-
     if (ec)
     {
         if (node::error::error_category::contains(ec))
@@ -307,7 +300,6 @@ void chaser_validate::complete_block(const code& ec, const header_link& link,
         // Stop the network in case of an unexpected invalidity (debugging).
         // This is considered a bug, not an invalid block arrival (for now).
         ////fault(ec);
-
         return;
     }
 
@@ -320,7 +312,7 @@ void chaser_validate::complete_block(const code& ec, const header_link& link,
     }
 
     // Prevent stall by posting internal event, avoid hitting external handlers.
-    if (is_zero(backlog_))
+    if (is_zero(backlog_.load(std::memory_order_relaxed)))
         handle_event(ec, chase::bump, height_t{});
 }