From f14d30e4a71a8e47c040000cfc5c6c0dcc014baf Mon Sep 17 00:00:00 2001 From: clemahieu Date: Thu, 20 May 2021 12:43:38 +0100 Subject: [PATCH 01/10] This change simplifies logic in the election scheduler. It extracts predicate functions so identical logic can be checked in the condition_variable wait function and also inside the election_scheduler processing loop. This allows new predicates to be easily added and also eliminates risk of disjoin checks between the loop and the condition variable. --- nano/node/election_scheduler.cpp | 33 ++++++++++++++++++-------------- nano/node/election_scheduler.hpp | 4 ++-- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/nano/node/election_scheduler.cpp b/nano/node/election_scheduler.cpp index 71a9991ad7..577cf42d6d 100644 --- a/nano/node/election_scheduler.cpp +++ b/nano/node/election_scheduler.cpp @@ -87,6 +87,16 @@ size_t nano::election_scheduler::priority_queue_size () const return priority.size (); } +bool nano::election_scheduler::priority_queue_predicate () const +{ + return node.active.vacancy () > 0 && !priority.empty (); +} + +bool nano::election_scheduler::manual_queue_predicate () const +{ + return node.active.vacancy () > 0 && !manual_queue.empty (); +} + void nano::election_scheduler::run () { nano::thread_role::set (nano::thread_role::name::election_scheduler); @@ -94,15 +104,19 @@ void nano::election_scheduler::run () while (!stopped) { condition.wait (lock, [this] () { - auto vacancy = node.active.vacancy (); - auto has_vacancy = vacancy > 0; - auto available = !priority.empty () || !manual_queue.empty (); - return stopped || (has_vacancy && available); + return stopped || priority_queue_predicate () || manual_queue_predicate (); }); debug_assert ((std::this_thread::yield (), true)); // Introduce some random delay in debug builds if (!stopped) { - if (!priority.empty ()) + if (manual_queue_predicate ()) + { + auto const [block, previous_balance, election_behavior, confirmation_action] = manual_queue.front (); + nano::unique_lock lock2 (node.active.mutex); + node.active.insert_impl (lock2, block, previous_balance, election_behavior, confirmation_action); + manual_queue.pop_front (); + } + else if (priority_queue_predicate ()) { auto block = priority.top (); std::shared_ptr election; @@ -113,15 +127,6 @@ void nano::election_scheduler::run () election->transition_active (); } priority.pop (); - ++priority_queued; - } - if (!manual_queue.empty ()) - { - auto const [block, previous_balance, election_behavior, confirmation_action] = manual_queue.front (); - nano::unique_lock lock2 (node.active.mutex); - node.active.insert_impl (lock2, block, previous_balance, election_behavior, confirmation_action); - manual_queue.pop_front (); - ++manual_queued; } notify (); } diff --git a/nano/node/election_scheduler.hpp b/nano/node/election_scheduler.hpp index 8baf7a7e10..d90fa67ac7 100644 --- a/nano/node/election_scheduler.hpp +++ b/nano/node/election_scheduler.hpp @@ -36,10 +36,10 @@ class election_scheduler final private: void run (); bool empty_locked () const; + bool priority_queue_predicate () const; + bool manual_queue_predicate () const; nano::prioritization priority; - uint64_t priority_queued{ 0 }; std::deque, boost::optional, nano::election_behavior, std::function)>>> manual_queue; - uint64_t manual_queued{ 0 }; nano::node & node; bool stopped; nano::condition_variable condition; From 20543f2f568373380ab1f3c394d4b2adfe337f23 Mon Sep 17 00:00:00 2001 From: clemahieu Date: Thu, 20 May 2021 15:10:14 +0100 Subject: [PATCH 02/10] Rewrite test to use election_scheduler::activate instead of election_scheduler::manual since the semantics of ::manual will be changed to not wait for vacancy before inserting. --- nano/core_test/election_scheduler.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nano/core_test/election_scheduler.cpp b/nano/core_test/election_scheduler.cpp index 98ba24993b..c95dc39292 100644 --- a/nano/core_test/election_scheduler.cpp +++ b/nano/core_test/election_scheduler.cpp @@ -142,7 +142,8 @@ TEST (election_scheduler, flush_vacancy) .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub) .work (*system.work.generate (nano::genesis_hash)) .build_shared (); - node.scheduler.manual (send); + ASSERT_EQ (nano::process_result::progress, node.process (*send).code); + node.scheduler.activate (nano::dev_genesis_key.pub, node.store.tx_begin_read ()); // Ensure this call does not block, even though no elections can be activated. node.scheduler.flush (); ASSERT_EQ (0, node.active.size ()); From 39c4f5bdbb29eb5da5727e055b17d2f8c363980f Mon Sep 17 00:00:00 2001 From: clemahieu Date: Thu, 20 May 2021 15:24:07 +0100 Subject: [PATCH 03/10] - Changes the semantics of election_scheduler::manual_queue such that vacancy is not considered and elections are started for any blocks passed in. - Moves responsibility for trimming down election count from active_transaction in to election_scheduler. This is done so that more advanced filling/spilling can be done by the election scheduler in the future. - Erases from the active_transactions container by oldest transaction rather than by newest. --- nano/core_test/active_transactions.cpp | 70 ++++++++++++++++++++++++++ nano/node/active_transactions.cpp | 16 ++++-- nano/node/active_transactions.hpp | 1 + nano/node/election_scheduler.cpp | 15 ++++-- nano/node/election_scheduler.hpp | 1 + 5 files changed, 97 insertions(+), 6 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index fc3e1d97b5..d0df474ada 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -1458,3 +1458,73 @@ TEST (active_transactions, vacancy) ASSERT_EQ (1, node.active.vacancy ()); ASSERT_EQ (0, node.active.size ()); } + +// Ensure transactions in excess of capacity are removed in fifo order +TEST (active_transactions, fifo) +{ + nano::system system; + nano::node_config config{ nano::get_available_port (), system.logging }; + config.active_elections_size = 1; + auto & node = *system.add_node (config); + nano::keypair key0; + nano::keypair key1; + nano::state_block_builder builder; + // Construct two pending entries that can be received simultaneously + auto send0 = builder.make_block () + .account (nano::dev_genesis_key.pub) + .previous (nano::genesis_hash) + .representative (nano::dev_genesis_key.pub) + .link (key0.pub) + .balance (nano::genesis_amount - 1) + .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub) + .work (*system.work.generate (nano::genesis_hash)) + .build_shared (); + ASSERT_EQ (nano::process_result::progress, node.process (*send0).code); + nano::blocks_confirm (node, { send0 }, true); + ASSERT_TIMELY (1s, node.block_confirmed (send0->hash ())); + ASSERT_TIMELY (1s, node.active.empty ()); + auto send1 = builder.make_block () + .account (nano::dev_genesis_key.pub) + .previous (send0->hash ()) + .representative (nano::dev_genesis_key.pub) + .link (key1.pub) + .balance (nano::genesis_amount - 2) + .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub) + .work (*system.work.generate (send0->hash ())) + .build_shared (); + ASSERT_EQ (nano::process_result::progress, node.process (*send1).code); + nano::blocks_confirm (node, { send1 }, true); + ASSERT_TIMELY (1s, node.block_confirmed (send1->hash ())); + ASSERT_TIMELY (1s, node.active.empty ()); + + auto receive0 = builder.make_block () + .account (key0.pub) + .previous (0) + .representative (nano::dev_genesis_key.pub) + .link (send0->hash ()) + .balance (1) + .sign (key0.prv, key0.pub) + .work (*system.work.generate (key0.pub)) + .build_shared (); + ASSERT_EQ (nano::process_result::progress, node.process (*receive0).code); + auto receive1 = builder.make_block () + .account (key1.pub) + .previous (0) + .representative (nano::dev_genesis_key.pub) + .link (send1->hash ()) + .balance (1) + .sign (key1.prv, key1.pub) + .work (*system.work.generate (key1.pub)) + .build_shared (); + ASSERT_EQ (nano::process_result::progress, node.process (*receive1).code); + node.scheduler.manual (receive0); + // Ensure first transaction becomes active + ASSERT_TIMELY (1s, node.active.election (receive0->qualified_root ()) != nullptr); + node.scheduler.manual (receive1); + // Ensure second transaction becomes active + ASSERT_TIMELY (1s, node.active.election (receive1->qualified_root ()) != nullptr); + // Ensure excess transactions get trimmed + ASSERT_TIMELY (1s, node.active.size () == 1); + // Ensure the surviving transaction is the least recently inserted + ASSERT_TIMELY (1s, node.active.election (receive1->qualified_root ()) != nullptr); +} diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index b86596e88d..40bc889fa3 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -317,9 +317,7 @@ void nano::active_transactions::request_confirm (nano::unique_lock { bool const confirmed_l (election_l->confirmed ()); - unconfirmed_count_l += !confirmed_l; - bool const overflow_l (unconfirmed_count_l > node.config.active_elections_size && election_l->election_start < election_ttl_cutoff_l); - if (overflow_l || election_l->transition_time (solicitor)) + if (election_l->transition_time (solicitor)) { if (election_l->optimistic () && election_l->failed ()) { @@ -1058,6 +1056,18 @@ void nano::active_transactions::erase_hash (nano::block_hash const & hash_a) debug_assert (erased == 1); } +void nano::active_transactions::erase_oldest () +{ + nano::unique_lock lock (mutex); + if (!roots.empty ()) + { + auto item = roots.get ().front (); + cleanup_election (lock, item.election->cleanup_info ()); + roots.get ().erase (item.election->qualified_root); + vacancy_update (); + } +} + bool nano::active_transactions::empty () { nano::lock_guard lock (mutex); diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index 0669ad9b63..9f2341de8b 100644 --- a/nano/node/active_transactions.hpp +++ b/nano/node/active_transactions.hpp @@ -167,6 +167,7 @@ class active_transactions final std::vector> list_active (size_t = std::numeric_limits::max ()); void erase (nano::block const &); void erase_hash (nano::block_hash const &); + void erase_oldest (); bool empty (); size_t size (); void stop (); diff --git a/nano/node/election_scheduler.cpp b/nano/node/election_scheduler.cpp index 577cf42d6d..b7fc5f1fac 100644 --- a/nano/node/election_scheduler.cpp +++ b/nano/node/election_scheduler.cpp @@ -94,7 +94,12 @@ bool nano::election_scheduler::priority_queue_predicate () const bool nano::election_scheduler::manual_queue_predicate () const { - return node.active.vacancy () > 0 && !manual_queue.empty (); + return !manual_queue.empty (); +} + +bool nano::election_scheduler::overfill_predicate () const +{ + return node.active.vacancy () < 0; } void nano::election_scheduler::run () @@ -104,12 +109,16 @@ void nano::election_scheduler::run () while (!stopped) { condition.wait (lock, [this] () { - return stopped || priority_queue_predicate () || manual_queue_predicate (); + return stopped || priority_queue_predicate () || manual_queue_predicate () || overfill_predicate (); }); debug_assert ((std::this_thread::yield (), true)); // Introduce some random delay in debug builds if (!stopped) { - if (manual_queue_predicate ()) + if (overfill_predicate ()) + { + node.active.erase_oldest (); + } + else if (manual_queue_predicate ()) { auto const [block, previous_balance, election_behavior, confirmation_action] = manual_queue.front (); nano::unique_lock lock2 (node.active.mutex); diff --git a/nano/node/election_scheduler.hpp b/nano/node/election_scheduler.hpp index d90fa67ac7..1a2889f2d1 100644 --- a/nano/node/election_scheduler.hpp +++ b/nano/node/election_scheduler.hpp @@ -38,6 +38,7 @@ class election_scheduler final bool empty_locked () const; bool priority_queue_predicate () const; bool manual_queue_predicate () const; + bool overfill_predicate () const; nano::prioritization priority; std::deque, boost::optional, nano::election_behavior, std::function)>>> manual_queue; nano::node & node; From 5f5577b260da4533141b67f391599a99b14352d0 Mon Sep 17 00:00:00 2001 From: clemahieu Date: Thu, 20 May 2021 18:25:09 +0100 Subject: [PATCH 04/10] - Changes the semantics of election_scheduler::manual_queue such that vacancy is not considered and elections are started for any blocks passed in. - Moves responsibility for trimming down election count from active_transaction in to election_scheduler. This is done so that more advanced filling/spilling can be done by the election scheduler in the future. - Erases from the active_transactions container by oldest transaction rather than by newest. --- nano/core_test/active_transactions.cpp | 64 +++++++++++++------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index d0df474ada..f09117c6e8 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -1471,51 +1471,51 @@ TEST (active_transactions, fifo) nano::state_block_builder builder; // Construct two pending entries that can be received simultaneously auto send0 = builder.make_block () - .account (nano::dev_genesis_key.pub) - .previous (nano::genesis_hash) - .representative (nano::dev_genesis_key.pub) - .link (key0.pub) - .balance (nano::genesis_amount - 1) - .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub) - .work (*system.work.generate (nano::genesis_hash)) - .build_shared (); + .account (nano::dev_genesis_key.pub) + .previous (nano::genesis_hash) + .representative (nano::dev_genesis_key.pub) + .link (key0.pub) + .balance (nano::genesis_amount - 1) + .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub) + .work (*system.work.generate (nano::genesis_hash)) + .build_shared (); ASSERT_EQ (nano::process_result::progress, node.process (*send0).code); nano::blocks_confirm (node, { send0 }, true); ASSERT_TIMELY (1s, node.block_confirmed (send0->hash ())); ASSERT_TIMELY (1s, node.active.empty ()); auto send1 = builder.make_block () - .account (nano::dev_genesis_key.pub) - .previous (send0->hash ()) - .representative (nano::dev_genesis_key.pub) - .link (key1.pub) - .balance (nano::genesis_amount - 2) - .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub) - .work (*system.work.generate (send0->hash ())) - .build_shared (); + .account (nano::dev_genesis_key.pub) + .previous (send0->hash ()) + .representative (nano::dev_genesis_key.pub) + .link (key1.pub) + .balance (nano::genesis_amount - 2) + .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub) + .work (*system.work.generate (send0->hash ())) + .build_shared (); ASSERT_EQ (nano::process_result::progress, node.process (*send1).code); nano::blocks_confirm (node, { send1 }, true); ASSERT_TIMELY (1s, node.block_confirmed (send1->hash ())); ASSERT_TIMELY (1s, node.active.empty ()); auto receive0 = builder.make_block () - .account (key0.pub) - .previous (0) - .representative (nano::dev_genesis_key.pub) - .link (send0->hash ()) - .balance (1) - .sign (key0.prv, key0.pub) - .work (*system.work.generate (key0.pub)) - .build_shared (); + .account (key0.pub) + .previous (0) + .representative (nano::dev_genesis_key.pub) + .link (send0->hash ()) + .balance (1) + .sign (key0.prv, key0.pub) + .work (*system.work.generate (key0.pub)) + .build_shared (); ASSERT_EQ (nano::process_result::progress, node.process (*receive0).code); auto receive1 = builder.make_block () - .account (key1.pub) - .previous (0) - .representative (nano::dev_genesis_key.pub) - .link (send1->hash ()) - .balance (1) - .sign (key1.prv, key1.pub) - .work (*system.work.generate (key1.pub)) - .build_shared (); + .account (key1.pub) + .previous (0) + .representative (nano::dev_genesis_key.pub) + .link (send1->hash ()) + .balance (1) + .sign (key1.prv, key1.pub) + .work (*system.work.generate (key1.pub)) + .build_shared (); ASSERT_EQ (nano::process_result::progress, node.process (*receive1).code); node.scheduler.manual (receive0); // Ensure first transaction becomes active From 78d327acc117294e8cceaa8add87d0e3e4304795 Mon Sep 17 00:00:00 2001 From: Matthew King Date: Thu, 20 May 2021 19:38:31 +0100 Subject: [PATCH 05/10] Split election drop stats to be either overflow or expired --- nano/lib/stats.cpp | 7 +++++-- nano/lib/stats.hpp | 3 ++- nano/node/active_transactions.cpp | 10 +++++----- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/nano/lib/stats.cpp b/nano/lib/stats.cpp index f1db603667..4f53f59a08 100644 --- a/nano/lib/stats.cpp +++ b/nano/lib/stats.cpp @@ -739,8 +739,11 @@ std::string nano::stat::detail_to_string (uint32_t key) case nano::stat::detail::election_difficulty_update: res = "election_difficulty_update"; break; - case nano::stat::detail::election_drop: - res = "election_drop"; + case nano::stat::detail::election_drop_expired: + res = "election_drop_expired"; + break; + case nano::stat::detail::election_drop_overflow: + res = "election_drop_overflow"; break; case nano::stat::detail::election_restart: res = "election_restart"; diff --git a/nano/lib/stats.hpp b/nano/lib/stats.hpp index 1d28bc6085..b4ca560b9b 100644 --- a/nano/lib/stats.hpp +++ b/nano/lib/stats.hpp @@ -318,7 +318,8 @@ class stat final election_start, election_block_conflict, election_difficulty_update, - election_drop, + election_drop_expired, + election_drop_overflow, election_restart, // udp diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 40bc889fa3..466d7f91dc 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -330,6 +330,10 @@ void nano::active_transactions::request_confirm (nano::unique_lock } // Locks active mutex, cleans up the election and erases it from the main container + if (!election_l.confirmed) + { + node.stats.inc (nano::stat::type::election, nano::stat::detail::election_drop_expired); + } erase (election_l->qualified_root); } } @@ -349,11 +353,6 @@ void nano::active_transactions::cleanup_election (nano::unique_lock { debug_assert (lock_a.owns_lock ()); - if (!info_a.confirmed) - { - node.stats.inc (nano::stat::type::election, nano::stat::detail::election_drop); - } - for (auto const & [hash, block] : info_a.blocks) { auto erased (blocks.erase (hash)); @@ -1061,6 +1060,7 @@ void nano::active_transactions::erase_oldest () nano::unique_lock lock (mutex); if (!roots.empty ()) { + node.stats.inc (nano::stat::type::election, nano::stat::detail::election_drop_overflow); auto item = roots.get ().front (); cleanup_election (lock, item.election->cleanup_info ()); roots.get ().erase (item.election->qualified_root); From f5000efe97cea98a9a83f28a8ed9d9b20bc120e1 Mon Sep 17 00:00:00 2001 From: Matthew King Date: Thu, 20 May 2021 19:41:26 +0100 Subject: [PATCH 06/10] Fix typo --- nano/node/active_transactions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 466d7f91dc..fcfc46cc2a 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -330,7 +330,7 @@ void nano::active_transactions::request_confirm (nano::unique_lock } // Locks active mutex, cleans up the election and erases it from the main container - if (!election_l.confirmed) + if (!election_l.confirmed()) { node.stats.inc (nano::stat::type::election, nano::stat::detail::election_drop_expired); } From 6893b22bf75377849be9bea54220ce288f782760 Mon Sep 17 00:00:00 2001 From: Matthew King Date: Thu, 20 May 2021 19:43:12 +0100 Subject: [PATCH 07/10] Lose old php habbits --- nano/node/active_transactions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index fcfc46cc2a..29b4da58f2 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -330,7 +330,7 @@ void nano::active_transactions::request_confirm (nano::unique_lock } // Locks active mutex, cleans up the election and erases it from the main container - if (!election_l.confirmed()) + if (!election_l->confirmed()) { node.stats.inc (nano::stat::type::election, nano::stat::detail::election_drop_expired); } From 90364893c2630915d56a1cab515f9504cbf2ada1 Mon Sep 17 00:00:00 2001 From: Matthew King Date: Thu, 20 May 2021 21:36:11 +0100 Subject: [PATCH 08/10] Clang-format changes --- nano/node/active_transactions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 29b4da58f2..acb299dd5a 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -330,7 +330,7 @@ void nano::active_transactions::request_confirm (nano::unique_lock } // Locks active mutex, cleans up the election and erases it from the main container - if (!election_l->confirmed()) + if (!election_l->confirmed ()) { node.stats.inc (nano::stat::type::election, nano::stat::detail::election_drop_expired); } From 7d7297b224923f686cc0a4905b6278a16b37754a Mon Sep 17 00:00:00 2001 From: Matthew King Date: Fri, 21 May 2021 12:15:06 +0100 Subject: [PATCH 09/10] Fix tests and track all drops from erase function, incase of further entry points to .erase in future --- nano/core_test/active_transactions.cpp | 6 ++++-- nano/lib/stats.cpp | 3 +++ nano/lib/stats.hpp | 1 + nano/node/active_transactions.cpp | 5 +++++ 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index f09117c6e8..8646ab4a25 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -613,7 +613,7 @@ TEST (active_transactions, 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::election, nano::stat::detail::election_drop)); + ASSERT_EQ (1, node.stats.count (nano::stat::type::election, nano::stat::detail::election_drop_all)); // Block cleared from active ASSERT_EQ (0, node.active.blocks.count (block->hash ())); @@ -632,7 +632,7 @@ TEST (active_transactions, 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::election, nano::stat::detail::election_drop)); + ASSERT_EQ (1, node.stats.count (nano::stat::type::election, nano::stat::detail::election_drop_all)); // Block cleared from active ASSERT_EQ (0, node.active.blocks.count (block->hash ())); @@ -1525,6 +1525,8 @@ TEST (active_transactions, fifo) ASSERT_TIMELY (1s, node.active.election (receive1->qualified_root ()) != nullptr); // Ensure excess transactions get trimmed ASSERT_TIMELY (1s, node.active.size () == 1); + // Ensure overflow stats have been incremented + ASSERT_EQ (1, node.stats.count (nano::stat::type::election, nano::stat::detail::election_drop_overflow)); // Ensure the surviving transaction is the least recently inserted ASSERT_TIMELY (1s, node.active.election (receive1->qualified_root ()) != nullptr); } diff --git a/nano/lib/stats.cpp b/nano/lib/stats.cpp index 4f53f59a08..0b02dec981 100644 --- a/nano/lib/stats.cpp +++ b/nano/lib/stats.cpp @@ -745,6 +745,9 @@ std::string nano::stat::detail_to_string (uint32_t key) case nano::stat::detail::election_drop_overflow: res = "election_drop_overflow"; break; + case nano::stat::detail::election_drop_all: + res = "election_drop_all"; + break; case nano::stat::detail::election_restart: res = "election_restart"; break; diff --git a/nano/lib/stats.hpp b/nano/lib/stats.hpp index b4ca560b9b..9a72089e66 100644 --- a/nano/lib/stats.hpp +++ b/nano/lib/stats.hpp @@ -320,6 +320,7 @@ class stat final election_difficulty_update, election_drop_expired, election_drop_overflow, + election_drop_all, election_restart, // udp diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index acb299dd5a..975e174a02 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -353,6 +353,11 @@ void nano::active_transactions::cleanup_election (nano::unique_lock { debug_assert (lock_a.owns_lock ()); + if (!info_a.confirmed) + { + node.stats.inc (nano::stat::type::election, nano::stat::detail::election_drop_all); + } + for (auto const & [hash, block] : info_a.blocks) { auto erased (blocks.erase (hash)); From 9dfe1cfb41b653c4ebae3ede5616a891b82af687 Mon Sep 17 00:00:00 2001 From: Matthew King Date: Thu, 27 May 2021 13:13:50 +0100 Subject: [PATCH 10/10] Use local variable and fix logging issue introduced in #3296 --- nano/node/active_transactions.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index e3a5d8be85..248b659778 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -316,6 +316,7 @@ void nano::active_transactions::request_confirm (nano::unique_lock for (auto const & election_l : elections_l) { bool const confirmed_l (election_l->confirmed ()); + unconfirmed_count_l += !confirmed_l; if (election_l->transition_time (solicitor)) { @@ -330,7 +331,7 @@ void nano::active_transactions::request_confirm (nano::unique_lock } // Locks active mutex, cleans up the election and erases it from the main container - if (!election_l->confirmed ()) + if (!confirmed_l) { node.stats.inc (nano::stat::type::election, nano::stat::detail::election_drop_expired); }