From 4bc6ef6edb20d1a8e80833abc99429ccb07c6b8f Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 14 Aug 2024 14:29:36 -0500 Subject: [PATCH 1/5] GH-528 Limit fetch-sync-span by max-reversible-blocks if needed --- libraries/chain/controller.cpp | 8 ++++ .../chain/include/eosio/chain/controller.hpp | 3 +- plugins/net_plugin/net_plugin.cpp | 47 +++++++++++++------ 3 files changed, 42 insertions(+), 16 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 90a5f73cb5..3c69d5beee 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1083,6 +1083,10 @@ struct controller_impl { return fork_db.apply([&](const auto& forkdb) { return !!forkdb.has_root(); }); } + size_t fork_db_size() const { + return fork_db.size(); + } + block_id_type fork_db_root_block_id() const { return fork_db.apply([&](const auto& forkdb) { return forkdb.root()->id(); }); } @@ -5305,6 +5309,10 @@ bool controller::fork_db_has_root() const { return my->fork_db_has_root(); } +size_t controller::fork_db_size() const { + return my->fork_db_size(); +} + uint32_t controller::last_irreversible_block_num() const { return my->fork_db_root_block_num(); } diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index fff00e4477..678545c0df 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -301,7 +301,8 @@ namespace eosio::chain { void set_savanna_lib_id(const block_id_type& id); - bool fork_db_has_root() const; + bool fork_db_has_root() const; + size_t fork_db_size() const; // thread-safe, applied LIB, fork db root uint32_t last_irreversible_block_num() const; diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index e4b2763dce..a44847ccd3 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -225,7 +225,7 @@ namespace eosio { uint32_t sync_next_expected_num GUARDED_BY(sync_mtx) {0}; // the next block number we need from peer connection_ptr sync_source GUARDED_BY(sync_mtx); // connection we are currently syncing from - const uint32_t sync_req_span {0}; + const uint32_t sync_fetch_span {0}; const uint32_t sync_peer_limit {0}; alignas(hardware_destructive_interference_sz) @@ -249,7 +249,7 @@ namespace eosio { connection_ptr find_next_sync_node(); // call with locked mutex void start_sync( const connection_ptr& c, uint32_t target ); // locks mutex bool verify_catchup( const connection_ptr& c, uint32_t num, const block_id_type& id ); // locks mutex - + uint32_t active_sync_fetch_span() const; public: enum class closing_mode { immediately, // closing connection immediately @@ -1999,13 +1999,25 @@ namespace eosio { ,sync_last_requested_num( 0 ) ,sync_next_expected_num( 1 ) ,sync_source() - ,sync_req_span( span ) + ,sync_fetch_span( span ) ,sync_peer_limit( sync_peer_limit ) ,sync_state(in_sync) ,min_blocks_distance(min_blocks_distance) { } + uint32_t sync_manager::active_sync_fetch_span() const { + const auto& cc = my_impl->chain_plug->chain_config(); + auto fork_db_size = my_impl->chain_plug->chain().fork_db_size(); + auto reversible_remaining = cc.max_reversible_blocks - fork_db_size; + if (reversible_remaining < sync_fetch_span) { + fc_wlog(logger, "sync-fetch-span ${sfs} restricted by max-reversible-blocks ${m}, fork_db_size ${fs}", + ("sfs", sync_fetch_span)("m", cc.max_reversible_blocks)("fs", fork_db_size)); + return reversible_remaining; + } + return sync_fetch_span; + } + constexpr auto sync_manager::stage_str(stages s) { switch (s) { case in_sync : return "in sync"; @@ -2113,8 +2125,8 @@ namespace eosio { void sync_manager::request_next_chunk( const connection_ptr& conn ) REQUIRES(sync_mtx) { auto chain_info = my_impl->get_chain_info(); - fc_dlog( logger, "sync_last_requested_num: ${r}, sync_next_expected_num: ${e}, sync_known_lib_num: ${k}, sync_req_span: ${s}, fhead: ${h}, lib: ${lib}", - ("r", sync_last_requested_num)("e", sync_next_expected_num)("k", sync_known_lib_num)("s", sync_req_span)("h", chain_info.fork_head_num)("lib", chain_info.lib_num) ); + fc_dlog( logger, "sync_last_requested_num: ${r}, sync_next_expected_num: ${e}, sync_known_lib_num: ${k}, sync-fetch-span: ${s}, fhead: ${h}, lib: ${lib}", + ("r", sync_last_requested_num)("e", sync_next_expected_num)("k", sync_known_lib_num)("s", sync_fetch_span)("h", chain_info.fork_head_num)("lib", chain_info.lib_num) ); if (conn) { // p2p_high_latency_test.py test depends on this exact log statement. @@ -2149,7 +2161,8 @@ namespace eosio { bool request_sent = false; if( sync_last_requested_num != sync_known_lib_num ) { uint32_t start = sync_next_expected_num; - uint32_t end = start + sync_req_span - 1; + auto fetch_span = active_sync_fetch_span(); + uint32_t end = start + fetch_span - 1; if( end > sync_known_lib_num ) end = sync_known_lib_num; if( end > 0 && end >= start ) { @@ -2529,20 +2542,24 @@ namespace eosio { } else { // 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(); - // do not allow to get too far ahead (one sync_req_span) of chain head - if (blk_num >= sync_last_requested_num && blk_num < head + sync_req_span) { - // block was not applied, possibly because we already have the block - fc_dlog(logger, "Requesting blocks ahead, 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 (blk_num >= sync_last_requested_num) { + auto fetch_span = active_sync_fetch_span(); + // do not allow to get too far ahead (one sync_fetch_span) of chain head + if (blk_num < head + fetch_span) { + // block was not applied, possibly because we already have the block + fc_dlog(logger, "Requesting ${fs} blocks ahead, head: ${h} fhead ${fh} blk_num: ${bn} sync_next_expected_num ${nen} " + "sync_last_requested_num: ${lrn}, sync_last_requested_block: ${lrb}", + ("fs", fetch_span)("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(); + } } } } 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()) From 78fa2c12c14a37d926b18272f7d0e98d404d22f2 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 15 Aug 2024 13:13:37 -0500 Subject: [PATCH 2/5] GH-528 chain_plug->chain_config() not available after plugin_initialize --- plugins/net_plugin/net_plugin.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index a44847ccd3..c854aa65dc 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -227,6 +227,7 @@ namespace eosio { const uint32_t sync_fetch_span {0}; const uint32_t sync_peer_limit {0}; + const size_t max_reversible_blocks {0}; alignas(hardware_destructive_interference_sz) std::atomic sync_state{in_sync}; @@ -255,7 +256,7 @@ namespace eosio { immediately, // closing connection immediately handshake // sending handshake message }; - explicit sync_manager( uint32_t span, uint32_t sync_peer_limit, uint32_t min_blocks_distance ); + explicit sync_manager( uint32_t span, uint32_t sync_peer_limit, size_t max_reversible_blocks, uint32_t min_blocks_distance ); static void send_handshakes(); bool syncing_from_peer() const { return sync_state == lib_catchup; } bool is_in_sync() const { return sync_state == in_sync; } @@ -1994,25 +1995,25 @@ namespace eosio { } //----------------------------------------------------------- - sync_manager::sync_manager( uint32_t span, uint32_t sync_peer_limit, uint32_t min_blocks_distance ) + sync_manager::sync_manager( uint32_t span, uint32_t sync_peer_limit, size_t max_reversible_blocks, uint32_t min_blocks_distance ) :sync_known_lib_num( 0 ) ,sync_last_requested_num( 0 ) ,sync_next_expected_num( 1 ) ,sync_source() ,sync_fetch_span( span ) ,sync_peer_limit( sync_peer_limit ) + ,max_reversible_blocks(max_reversible_blocks) ,sync_state(in_sync) ,min_blocks_distance(min_blocks_distance) { } uint32_t sync_manager::active_sync_fetch_span() const { - const auto& cc = my_impl->chain_plug->chain_config(); auto fork_db_size = my_impl->chain_plug->chain().fork_db_size(); - auto reversible_remaining = cc.max_reversible_blocks - fork_db_size; + auto reversible_remaining = max_reversible_blocks - fork_db_size; if (reversible_remaining < sync_fetch_span) { fc_wlog(logger, "sync-fetch-span ${sfs} restricted by max-reversible-blocks ${m}, fork_db_size ${fs}", - ("sfs", sync_fetch_span)("m", cc.max_reversible_blocks)("fs", fork_db_size)); + ("sfs", sync_fetch_span)("m", max_reversible_blocks)("fs", fork_db_size)); return reversible_remaining; } return sync_fetch_span; @@ -4245,6 +4246,9 @@ namespace eosio { try { fc_ilog( logger, "Initialize net plugin" ); + chain_plug = app().find_plugin(); + EOS_ASSERT( chain_plug, chain::missing_chain_plugin_exception, "" ); + peer_log_format = options.at( "peer-log-format" ).as(); txn_exp_period = def_txn_expire_wait; @@ -4266,6 +4270,7 @@ namespace eosio { sync_master = std::make_unique( options.at( "sync-fetch-span" ).as(), options.at( "sync-peer-limit" ).as(), + chain_plug->chain_config().max_reversible_blocks, min_blocks_distance); connections.init( std::chrono::milliseconds( options.at("p2p-keepalive-interval-ms").as() * 2 ), @@ -4362,8 +4367,6 @@ namespace eosio { } } - chain_plug = app().find_plugin(); - EOS_ASSERT( chain_plug, chain::missing_chain_plugin_exception, "" ); chain_id = chain_plug->get_chain_id(); fc::rand_pseudo_bytes( node_id.data(), node_id.data_size()); const controller& cc = chain_plug->chain(); From a6defa1a1156852c6ceaf82c9d04d6f97cb4a1f2 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 16 Aug 2024 05:55:40 -0500 Subject: [PATCH 3/5] GH-528 Avoid negative sync fetch --- plugins/net_plugin/net_plugin.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index c854aa65dc..4f36d2eed0 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2010,7 +2010,12 @@ namespace eosio { uint32_t sync_manager::active_sync_fetch_span() const { auto fork_db_size = my_impl->chain_plug->chain().fork_db_size(); - auto reversible_remaining = max_reversible_blocks - fork_db_size; + int32_t reversible_remaining = max_reversible_blocks - fork_db_size - 1; + if (reversible_remaining <= 0) { + fc_wlog(logger, "max-reversible-blocks ${m} exceeded, remaining ${r}, fork_db_size ${fs}", + ("m", max_reversible_blocks)("r", reversible_remaining)("fs", fork_db_size)); + reversible_remaining = 0; + } if (reversible_remaining < sync_fetch_span) { fc_wlog(logger, "sync-fetch-span ${sfs} restricted by max-reversible-blocks ${m}, fork_db_size ${fs}", ("sfs", sync_fetch_span)("m", max_reversible_blocks)("fs", fork_db_size)); @@ -2541,11 +2546,11 @@ namespace eosio { ("bn", blk_num)("kn", sync_known_lib_num)); send_handshakes_when_synced = true; } else { - // 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 >= sync_last_requested_num) { + // do not allow to get too far ahead (sync_fetch_span) of chain head auto fetch_span = active_sync_fetch_span(); - // do not allow to get too far ahead (one 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 + fetch_span) { // block was not applied, possibly because we already have the block fc_dlog(logger, "Requesting ${fs} blocks ahead, head: ${h} fhead ${fh} blk_num: ${bn} sync_next_expected_num ${nen} " From 57ffbccfb31c90e5f6096b8bf2f5db2cee8a1845 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 16 Aug 2024 07:08:25 -0500 Subject: [PATCH 4/5] GH-528 Use already retrieved value for log --- 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 4f36d2eed0..9bcdb77c4b 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2555,7 +2555,7 @@ namespace eosio { // block was not applied, possibly because we already have the block fc_dlog(logger, "Requesting ${fs} blocks ahead, head: ${h} fhead ${fh} blk_num: ${bn} sync_next_expected_num ${nen} " "sync_last_requested_num: ${lrn}, sync_last_requested_block: ${lrb}", - ("fs", fetch_span)("h", my_impl->get_chain_head_num())("fh", my_impl->get_fork_head_num()) + ("fs", fetch_span)("h", head)("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 3b153deefecef4f3c6d734733ea946c0ac210319 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 16 Aug 2024 07:58:28 -0500 Subject: [PATCH 5/5] GH-528 Add reversible_remaining to log statement --- 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 9bcdb77c4b..d64a1c5abe 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2017,8 +2017,8 @@ namespace eosio { reversible_remaining = 0; } if (reversible_remaining < sync_fetch_span) { - fc_wlog(logger, "sync-fetch-span ${sfs} restricted by max-reversible-blocks ${m}, fork_db_size ${fs}", - ("sfs", sync_fetch_span)("m", max_reversible_blocks)("fs", fork_db_size)); + fc_wlog(logger, "sync-fetch-span ${sfs} restricted to ${r} by max-reversible-blocks ${m}, fork_db_size ${fs}", + ("sfs", sync_fetch_span)("r", reversible_remaining)("m", max_reversible_blocks)("fs", fork_db_size)); return reversible_remaining; } return sync_fetch_span;