From a60227f6c3411e5259dd4a70ff6dda1e39cea7a8 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 13 Sep 2024 07:48:19 -0500 Subject: [PATCH 01/14] GH-773 Add test cases for irreversible mode where sync-fetch-span less than head-LIB --- tests/nodeos_startup_catchup.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/nodeos_startup_catchup.py b/tests/nodeos_startup_catchup.py index dabedb2120..feca64f4dc 100755 --- a/tests/nodeos_startup_catchup.py +++ b/tests/nodeos_startup_catchup.py @@ -29,7 +29,7 @@ errorExit=Utils.errorExit appArgs=AppArgs() -extraArgs = appArgs.add(flag="--catchup-count", type=int, help="How many catchup-nodes to launch", default=12) +extraArgs = appArgs.add(flag="--catchup-count", type=int, help="How many catchup-nodes to launch", default=16) extraArgs = appArgs.add(flag="--txn-gen-nodes", type=int, help="How many transaction generator nodes", default=2) args = TestHelper.parse_args({"--dump-error-details","--keep-logs","-v","--leave-running", "--activate-if","-p","--wallet-port","--unshared"}, applicationSpecificArgs=appArgs) @@ -64,11 +64,15 @@ specificExtraNodeosArgs[pnodes+4] = f' --sync-fetch-span 89 ' specificExtraNodeosArgs[pnodes+5] = f' --sync-fetch-span 377 ' specificExtraNodeosArgs[pnodes+6] = f' --sync-fetch-span 1597 ' - specificExtraNodeosArgs[pnodes+7] = f' --sync-fetch-span 1597 ' + specificExtraNodeosArgs[pnodes+7] = f' --sync-fetch-span 2500 ' specificExtraNodeosArgs[pnodes+8] = f' --sync-fetch-span 6765 ' specificExtraNodeosArgs[pnodes+9] = f' --sync-fetch-span 28657 ' - specificExtraNodeosArgs[pnodes+10] = f' --sync-fetch-span 89 --read-mode irreversible ' - specificExtraNodeosArgs[pnodes+11] = f' --sync-fetch-span 377 --read-mode irreversible ' + specificExtraNodeosArgs[pnodes+10] = f' ' # default + specificExtraNodeosArgs[pnodes+11] = f' --sync-fetch-span 1 --read-mode irreversible ' + specificExtraNodeosArgs[pnodes+12] = f' --sync-fetch-span 5 --read-mode irreversible ' + specificExtraNodeosArgs[pnodes+13] = f' --sync-fetch-span 89 --read-mode irreversible ' + specificExtraNodeosArgs[pnodes+14] = f' --sync-fetch-span 200 --read-mode irreversible ' + specificExtraNodeosArgs[pnodes+15] = f' --sync-fetch-span 2500 --read-mode irreversible ' if cluster.launch(prodCount=prodCount, specificExtraNodeosArgs=specificExtraNodeosArgs, activateIF=activateIF, onlyBios=False, pnodes=pnodes, totalNodes=totalNodes, totalProducers=pnodes*prodCount, unstartedNodes=catchupCount, loadSystemContract=True, maximumP2pPerHost=totalNodes+trxGeneratorCnt) is False: From 88a1626ae51a99ae6bfd92bb81240c2a86bce131 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 13 Sep 2024 07:48:56 -0500 Subject: [PATCH 02/14] GH-773 Allow irreversible mode to keep syncing if LIB not moving --- plugins/net_plugin/net_plugin.cpp | 32 ++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 62e111ecfc..64fe8a2618 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2182,6 +2182,7 @@ namespace eosio { sync_next_expected_num < sync_last_requested_num ); } + // called from c's connection strand bool sync_manager::is_sync_request_ahead_allowed(block_num_type blk_num) const REQUIRES(sync_mtx) { if (blk_num >= sync_last_requested_num) { // do not allow to get too far ahead (sync_fetch_span) of chain head @@ -2189,7 +2190,16 @@ namespace eosio { uint32_t head = my_impl->get_chain_head_num(); if (blk_num < head + sync_fetch_span) return true; + + // might be in irreversible mode + controller& cc = my_impl->chain_plug->chain(); + auto calculated_lib = cc.fork_db_head().irreversible_blocknum(); + if (calculated_lib <= my_impl->get_chain_lib_num()) + return true; } + + fc_dlog(logger, "sync ahead not allowed ${bn} < sync_last_requested_num ${lrn}", + ("bn", blk_num)("lrn", sync_last_requested_num)); return false; } @@ -2208,7 +2218,10 @@ namespace eosio { return; } - if( sync_state != lib_catchup || !sync_recently_active()) { + stages current_sync_state = sync_state; + if( current_sync_state != lib_catchup || !sync_recently_active()) { + peer_dlog(c, "requesting next chuck, set to lib_catchup and request_next_chunk, sync_state ${s}, sync_next_expected_num ${nen}", + ("s", stage_str(current_sync_state))("nen", sync_next_expected_num)); set_state( lib_catchup ); sync_last_requested_num = 0; sync_next_expected_num = chain_info.lib_num + 1; @@ -2577,14 +2590,15 @@ namespace eosio { } } else { // blk_applied if (blk_num >= sync_last_requested_num) { - // Did not request blocks ahead, likely because too far ahead of head - // Do not restrict sync_fetch_span as we want max-reversible-blocks to shut down the node for applied blocks - fc_dlog(logger, "Requesting blocks, head: ${h} fhead ${fh} blk_num: ${bn} sync_next_expected_num ${nen} " - "sync_last_requested_num: ${lrn}, sync_last_requested_block: ${lrb}", - ("h", my_impl->get_chain_head_num())("fh", my_impl->get_fork_head_num()) - ("bn", blk_num)("nen", sync_next_expected_num) - ("lrn", sync_last_requested_num)("lrb", c->sync_last_requested_block)); - request_next_chunk(); + if (is_sync_request_ahead_allowed(blk_num)) { + // Did not request blocks ahead, likely because too far ahead of head, or in irreversible mode + fc_dlog(logger, "Requesting blocks, head: ${h} fhead ${fh} blk_num: ${bn} sync_next_expected_num ${nen} " + "sync_last_requested_num: ${lrn}, sync_last_requested_block: ${lrb}", + ("h", my_impl->get_chain_head_num())("fh", my_impl->get_fork_head_num()) + ("bn", blk_num)("nen", sync_next_expected_num) + ("lrn", sync_last_requested_num)("lrb", c->sync_last_requested_block)); + request_next_chunk(); + } } } } From d9a224b7d9b983d6528684cdeb4160846aa8af57 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 13 Sep 2024 08:29:36 -0500 Subject: [PATCH 03/14] GH-773 Log when syncing ahead even though LIB is not advancing --- plugins/net_plugin/net_plugin.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 64fe8a2618..90fbba7cc1 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2194,8 +2194,11 @@ namespace eosio { // might be in irreversible mode controller& cc = my_impl->chain_plug->chain(); auto calculated_lib = cc.fork_db_head().irreversible_blocknum(); - if (calculated_lib <= my_impl->get_chain_lib_num()) + if (calculated_lib <= my_impl->get_chain_lib_num()) { + fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp} for paused LIB ${l}, forkdb size ${s}", + ("sp", sync_fetch_span)("l", calculated_lib)("s", cc.fork_db_size())); return true; + } } fc_dlog(logger, "sync ahead not allowed ${bn} < sync_last_requested_num ${lrn}", From bc0bc8af2b1d79201aea1aa112ebf57c6acdf130 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 13 Sep 2024 11:05:17 -0500 Subject: [PATCH 04/14] GH-773 Reword confusing sync timeout log messages --- plugins/net_plugin/net_plugin.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 90fbba7cc1..5b70522207 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2257,14 +2257,14 @@ namespace eosio { // called from connection strand void sync_manager::sync_timeout(const connection_ptr& c, const boost::system::error_code& ec) { if( !ec ) { - peer_dlog(c, "sync timeout"); + peer_dlog(c, "sync timed out"); sync_reassign_fetch( c ); close(true); } else if( ec != boost::asio::error::operation_aborted ) { // don't log on operation_aborted, called on destroy peer_elog( c, "setting timer for sync request got error ${ec}", ("ec", ec.message()) ); } --sync_timers_active; - peer_dlog(c, "sync timeout, active_timers ${t}", ("t", sync_timers_active.load())); + peer_dlog(c, "sync active_timers ${t}", ("t", sync_timers_active.load())); } // called from connection strand From 4bceee07695d9b5052cf9cf24260bac4a6843f72 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 13 Sep 2024 14:36:45 -0500 Subject: [PATCH 05/14] GH-773 Make it obvious that controller.get_read_mode() is thread-safe. --- libraries/chain/controller.cpp | 2 +- libraries/chain/include/eosio/chain/controller.hpp | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index eb60ec697d..7a35b8a772 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -953,7 +953,7 @@ struct controller_impl { const chain_id_type chain_id; // read by thread_pool threads, value will not be changed bool replaying = false; bool is_producer_node = false; // true if node is configured as a block producer - db_read_mode read_mode = db_read_mode::HEAD; + const db_read_mode read_mode; bool in_trx_requiring_checks = false; ///< if true, checks that are normally skipped on replay (e.g. auth checks) cannot be skipped std::optional subjective_cpu_leeway; bool trusted_producer_light_validation = false; diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index 599800e753..73973b82fa 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -394,8 +394,10 @@ namespace eosio::chain { chain_id_type get_chain_id()const; + // thread safe db_read_mode get_read_mode()const; validation_mode get_validation_mode()const; + /// @return true if terminate-at-block reached /// not-thread-safe bool should_terminate() const; From e82613a4f93b3e1a9df0cd48102adf9b061a071f Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 13 Sep 2024 14:40:05 -0500 Subject: [PATCH 06/14] GH-773 Only check lib not moving in irreversible mode since non-irreversible mode the chain_head will still be moving and not cause net_plugin to stop syncing. Only consider if LIB not moving when the span is greater than sync_fetch_span. Otherwise normal operation will cause net_plugin to sync ahead when fork db is just up to date. --- plugins/net_plugin/net_plugin.cpp | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 5b70522207..65c55d879d 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2187,17 +2187,23 @@ namespace eosio { if (blk_num >= sync_last_requested_num) { // do not allow to get too far ahead (sync_fetch_span) of chain head // use chain head instead of fork head so we do not get too far ahead of applied blocks - uint32_t head = my_impl->get_chain_head_num(); - if (blk_num < head + sync_fetch_span) + uint32_t head_num = my_impl->get_chain_head_num(); + if (blk_num < head_num + sync_fetch_span) return true; // might be in irreversible mode controller& cc = my_impl->chain_plug->chain(); - auto calculated_lib = cc.fork_db_head().irreversible_blocknum(); - if (calculated_lib <= my_impl->get_chain_lib_num()) { - fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp} for paused LIB ${l}, forkdb size ${s}", - ("sp", sync_fetch_span)("l", calculated_lib)("s", cc.fork_db_size())); - return true; + if (cc.get_read_mode() == db_read_mode::IRREVERSIBLE) { + // chain head == lib == fork_db_root in irreversible + auto forkdb_head = cc.fork_db_head(); + if (forkdb_head.block_num() - head_num >= sync_fetch_span) { + auto calculated_lib = forkdb_head.irreversible_blocknum(); + if (calculated_lib <= head_num) { + fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp} for paused LIB ${l}, chain_lib ${cl}, forkdb size ${s}", + ("sp", sync_fetch_span)("l", calculated_lib)("cl", head_num)("s", cc.fork_db_size())); + return true; + } + } } } From 1d2ceb14dd4b09897be94eef3c6968c6be4b918a Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 13 Sep 2024 14:43:26 -0500 Subject: [PATCH 07/14] GH-773 Fix should_sync_from being way to picky. It would not allow a sync when the peer reported even slightly less than sync_known_lib_num which is a very normal condition. For example sync_known_lib_num: 373330666, while peer_fhead: 373330647 when trying to sync block 129728. --- plugins/net_plugin/net_plugin.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 65c55d879d..e99d90f137 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -1007,7 +1007,7 @@ namespace eosio { bool connected() const; bool closed() const; // socket is not open or is closed or closing, thread safe bool current() const; - bool should_sync_from(uint32_t sync_next_expected_num, uint32_t sync_known_lib_num) const; + bool should_sync_from(uint32_t sync_next_expected_num, uint32_t sync_known_lib_num, uint32_t sync_fetch_span) const; /// @param reconnect true if we should try and reconnect immediately after close /// @param shutdown true only if plugin is shutting down @@ -1429,7 +1429,7 @@ namespace eosio { } // thread safe - bool connection::should_sync_from(uint32_t sync_next_expected_num, uint32_t sync_known_lib_num) const { + bool connection::should_sync_from(uint32_t sync_next_expected_num, uint32_t sync_known_lib_num, uint32_t sync_fetch_span) const { fc_dlog(logger, "id: ${id} blocks conn: ${t} current: ${c} socket_open: ${so} syncing from us: ${s} state: ${con} peer_start_block: ${sb} peer_fhead: ${h} ping: ${p}us no_retry: ${g}", ("id", connection_id)("t", is_blocks_connection()) ("c", current())("so", socket_is_open())("s", peer_syncing_from_us.load())("con", state_str(state())) @@ -1437,7 +1437,8 @@ namespace eosio { if (is_blocks_connection() && current()) { if (no_retry == go_away_reason::no_reason) { if (peer_start_block_num <= sync_next_expected_num) { // has blocks we want - if (peer_fork_head_block_num >= sync_known_lib_num) { // is in sync + auto needed_end = std::min(sync_next_expected_num + sync_fetch_span, sync_known_lib_num); + if (peer_fork_head_block_num >= needed_end) { // has lib blocks return true; } } @@ -2060,8 +2061,9 @@ namespace eosio { deque conns; my_impl->connections.for_each_block_connection([sync_next_expected_num = sync_next_expected_num, sync_known_lib_num = sync_known_lib_num, + sync_fetch_span = sync_fetch_span, &conns](const auto& c) { - if (c->should_sync_from(sync_next_expected_num, sync_known_lib_num)) { + if (c->should_sync_from(sync_next_expected_num, sync_known_lib_num, sync_fetch_span)) { conns.push_back(c); } }); From 8737c4e0000e9a97a2302937f2ed0fc3fda64686 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 13 Sep 2024 15:26:36 -0500 Subject: [PATCH 08/14] GH-773 Add additional debug logging --- plugins/net_plugin/net_plugin.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index e99d90f137..618ab6a3ed 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2206,11 +2206,15 @@ namespace eosio { return true; } } + fc_dlog(logger, "sync ahead not allowed. head ${h}, fhead ${fh}, fhead->lib ${fl}, sync-fetch-span ${s}", + ("h", head_num)("fh", forkdb_head.block_num())("fl", forkdb_head.irreversible_blocknum()) + ("s", sync_fetch_span)); + return false; } } - fc_dlog(logger, "sync ahead not allowed ${bn} < sync_last_requested_num ${lrn}", - ("bn", blk_num)("lrn", sync_last_requested_num)); + fc_dlog(logger, "sync ahead not allowed. block ${bn}, sync_last_requested_num ${lrn}, head ${h}, sync-fetch-span ${s}", + ("bn", blk_num)("lrn", sync_last_requested_num)("s", sync_fetch_span)); return false; } From 9a72b28410d8cee0dcc2eec8acd6d2cf4c4c7e86 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Sat, 14 Sep 2024 07:37:15 -0500 Subject: [PATCH 09/14] GH-773 Guard against underflow --- plugins/net_plugin/net_plugin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 618ab6a3ed..4c76d08ae6 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2198,7 +2198,7 @@ namespace eosio { if (cc.get_read_mode() == db_read_mode::IRREVERSIBLE) { // chain head == lib == fork_db_root in irreversible auto forkdb_head = cc.fork_db_head(); - if (forkdb_head.block_num() - head_num >= sync_fetch_span) { + if (forkdb_head.block_num() > head_num && forkdb_head.block_num() - head_num >= sync_fetch_span) { auto calculated_lib = forkdb_head.irreversible_blocknum(); if (calculated_lib <= head_num) { fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp} for paused LIB ${l}, chain_lib ${cl}, forkdb size ${s}", From cb66a697ee52a1fa4aadc590499ca67cce653b97 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 16 Sep 2024 15:55:05 -0500 Subject: [PATCH 10/14] GH-773 Add assert --- plugins/net_plugin/net_plugin.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 4c76d08ae6..e096d05859 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2201,6 +2201,7 @@ namespace eosio { if (forkdb_head.block_num() > head_num && forkdb_head.block_num() - head_num >= sync_fetch_span) { auto calculated_lib = forkdb_head.irreversible_blocknum(); if (calculated_lib <= head_num) { + assert(calculated_lib == head_num); fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp} for paused LIB ${l}, chain_lib ${cl}, forkdb size ${s}", ("sp", sync_fetch_span)("l", calculated_lib)("cl", head_num)("s", cc.fork_db_size())); return true; From 2cd01887dd63a0ce8039ac67d923cd066f97bb96 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 16 Sep 2024 19:04:18 -0500 Subject: [PATCH 11/14] GH-773 Update assert to allow for block 2 --- plugins/net_plugin/net_plugin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index e096d05859..7f07debc72 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2201,7 +2201,7 @@ namespace eosio { if (forkdb_head.block_num() > head_num && forkdb_head.block_num() - head_num >= sync_fetch_span) { auto calculated_lib = forkdb_head.irreversible_blocknum(); if (calculated_lib <= head_num) { - assert(calculated_lib == head_num); + assert(calculated_lib == 0 || calculated_lib == head_num); fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp} for paused LIB ${l}, chain_lib ${cl}, forkdb size ${s}", ("sp", sync_fetch_span)("l", calculated_lib)("cl", head_num)("s", cc.fork_db_size())); return true; From 1d5d977a68b4ec58fbf789eecd3b47acce751c8d Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 18 Sep 2024 08:06:11 -0500 Subject: [PATCH 12/14] GH-773 Remove unneeded check --- plugins/net_plugin/net_plugin.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 7f07debc72..c21e1c55f8 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2198,14 +2198,12 @@ namespace eosio { if (cc.get_read_mode() == db_read_mode::IRREVERSIBLE) { // chain head == lib == fork_db_root in irreversible auto forkdb_head = cc.fork_db_head(); - if (forkdb_head.block_num() > head_num && forkdb_head.block_num() - head_num >= sync_fetch_span) { - auto calculated_lib = forkdb_head.irreversible_blocknum(); - if (calculated_lib <= head_num) { - assert(calculated_lib == 0 || calculated_lib == head_num); - fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp} for paused LIB ${l}, chain_lib ${cl}, forkdb size ${s}", - ("sp", sync_fetch_span)("l", calculated_lib)("cl", head_num)("s", cc.fork_db_size())); - return true; - } + auto calculated_lib = forkdb_head.irreversible_blocknum(); + if (calculated_lib <= head_num) { + assert(calculated_lib == 0 || calculated_lib == head_num); + fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp} for paused LIB ${l}, chain_lib ${cl}, forkdb size ${s}", + ("sp", sync_fetch_span)("l", calculated_lib)("cl", head_num)("s", cc.fork_db_size())); + return true; } fc_dlog(logger, "sync ahead not allowed. head ${h}, fhead ${fh}, fhead->lib ${fl}, sync-fetch-span ${s}", ("h", head_num)("fh", forkdb_head.block_num())("fl", forkdb_head.irreversible_blocknum()) From 655d0abdd2337576d995f913399bc88173b1ac91 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 18 Sep 2024 10:40:53 -0500 Subject: [PATCH 13/14] GH-773 Refactor to make conditions clearer --- plugins/net_plugin/net_plugin.cpp | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index c21e1c55f8..df70ccbeb1 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2190,29 +2190,35 @@ namespace eosio { // do not allow to get too far ahead (sync_fetch_span) of chain head // use chain head instead of fork head so we do not get too far ahead of applied blocks uint32_t head_num = my_impl->get_chain_head_num(); - if (blk_num < head_num + sync_fetch_span) + if (blk_num < head_num) { // avoid underflow on blk_num - head_num return true; + } + block_num_type num_blocks_not_applied = blk_num - head_num; + if (num_blocks_not_applied < sync_fetch_span) { + fc_dlog(logger, "sync ahead allowed past sync-fetch-span ${sp}, block ${bn} chain_lib ${cl}, forkdb size ${s}", + ("bn", blk_num)("sp", sync_fetch_span)("cl", head_num)("s", my_impl->chain_plug->chain().fork_db_size())); + return true; + } - // might be in irreversible mode controller& cc = my_impl->chain_plug->chain(); if (cc.get_read_mode() == db_read_mode::IRREVERSIBLE) { - // chain head == lib == fork_db_root in irreversible auto forkdb_head = cc.fork_db_head(); auto calculated_lib = forkdb_head.irreversible_blocknum(); - if (calculated_lib <= head_num) { - assert(calculated_lib == 0 || calculated_lib == head_num); - fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp} for paused LIB ${l}, chain_lib ${cl}, forkdb size ${s}", - ("sp", sync_fetch_span)("l", calculated_lib)("cl", head_num)("s", cc.fork_db_size())); + auto num_blocks_that_can_be_applied = calculated_lib > head_num ? calculated_lib - head_num : 0; + if (num_blocks_that_can_be_applied < sync_fetch_span) { + if (head_num ) + fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp}, block ${bn} for paused LIB ${l}, chain_lib ${cl}, forkdb size ${s}", + ("bn", blk_num)("sp", sync_fetch_span)("l", calculated_lib)("cl", head_num)("s", cc.fork_db_size())); return true; } - fc_dlog(logger, "sync ahead not allowed. head ${h}, fhead ${fh}, fhead->lib ${fl}, sync-fetch-span ${s}", - ("h", head_num)("fh", forkdb_head.block_num())("fl", forkdb_head.irreversible_blocknum()) - ("s", sync_fetch_span)); - return false; } + + fc_dlog(logger, "sync ahead not allowed. block ${bn}, head ${h}, fhead ${fh}, fhead->lib ${fl}, sync-fetch-span ${sp}, forkdb size ${s}", + ("bn", blk_num)("h", head_num)("fh", cc.fork_db_head().block_num())("fl", cc.fork_db_head().irreversible_blocknum()) + ("sp", sync_fetch_span)("s", cc.fork_db_size())); } - fc_dlog(logger, "sync ahead not allowed. block ${bn}, sync_last_requested_num ${lrn}, head ${h}, sync-fetch-span ${s}", + fc_dlog(logger, "sync ahead not allowed. block ${bn}, sync_last_requested_num ${lrn}, sync-fetch-span ${s}", ("bn", blk_num)("lrn", sync_last_requested_num)("s", sync_fetch_span)); return false; } From 8a2fcf3134ad3430044b91a68e237f29dcc5e25c Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 18 Sep 2024 16:46:22 -0500 Subject: [PATCH 14/14] GH-773 Simplify logic --- plugins/net_plugin/net_plugin.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 121e23e9ec..a77e19cc57 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2182,10 +2182,7 @@ namespace eosio { // do not allow to get too far ahead (sync_fetch_span) of chain head // use chain head instead of fork head so we do not get too far ahead of applied blocks uint32_t head_num = my_impl->get_chain_head_num(); - if (blk_num < head_num) { // avoid underflow on blk_num - head_num - return true; - } - block_num_type num_blocks_not_applied = blk_num - head_num; + block_num_type num_blocks_not_applied = blk_num > head_num ? blk_num - head_num : 0; if (num_blocks_not_applied < sync_fetch_span) { fc_dlog(logger, "sync ahead allowed past sync-fetch-span ${sp}, block ${bn} chain_lib ${cl}, forkdb size ${s}", ("bn", blk_num)("sp", sync_fetch_span)("cl", head_num)("s", my_impl->chain_plug->chain().fork_db_size()));