From 50470bb136fb2576352246a99f51d199fd101cb2 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 15 Nov 2024 13:47:08 -0600 Subject: [PATCH 01/10] GH-1030 Allow speculative trx execution on node running in irreversible mode --- libraries/chain/controller.cpp | 2 -- plugins/chain_plugin/chain_plugin.cpp | 8 -------- plugins/net_plugin/net_plugin.cpp | 7 ------- 3 files changed, 17 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 9cf654d167..7fc8cb387d 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -5325,7 +5325,6 @@ transaction_trace_ptr controller::push_transaction( const transaction_metadata_p uint32_t billed_cpu_time_us, bool explicit_billed_cpu_time, int64_t subjective_cpu_bill_us ) { validate_db_available_size(); - EOS_ASSERT( get_read_mode() != db_read_mode::IRREVERSIBLE, transaction_type_exception, "push transaction not allowed in irreversible mode" ); EOS_ASSERT( trx && !trx->implicit() && !trx->scheduled(), transaction_type_exception, "Implicit/Scheduled transaction not allowed" ); return my->push_transaction(trx, block_deadline, max_transaction_time, billed_cpu_time_us, explicit_billed_cpu_time, subjective_cpu_bill_us ); } @@ -5334,7 +5333,6 @@ transaction_trace_ptr controller::push_scheduled_transaction( const transaction_ fc::time_point block_deadline, fc::microseconds max_transaction_time, uint32_t billed_cpu_time_us, bool explicit_billed_cpu_time ) { - EOS_ASSERT( get_read_mode() != db_read_mode::IRREVERSIBLE, transaction_type_exception, "push scheduled transaction not allowed in irreversible mode" ); validate_db_available_size(); return my->push_scheduled_transaction( trxid, block_deadline, max_transaction_time, billed_cpu_time_us, explicit_billed_cpu_time ); } diff --git a/plugins/chain_plugin/chain_plugin.cpp b/plugins/chain_plugin/chain_plugin.cpp index 4cf8613e76..866401f617 100644 --- a/plugins/chain_plugin/chain_plugin.cpp +++ b/plugins/chain_plugin/chain_plugin.cpp @@ -942,12 +942,6 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) { } api_accept_transactions = options.at( "api-accept-transactions" ).as(); - if( chain_config->read_mode == db_read_mode::IRREVERSIBLE ) { - if( api_accept_transactions ) { - api_accept_transactions = false; - wlog( "api-accept-transactions set to false due to read-mode: irreversible" ); - } - } if( api_accept_transactions ) { enable_accept_transactions(); } @@ -1138,8 +1132,6 @@ void chain_plugin::plugin_initialize(const variables_map& options) { void chain_plugin_impl::plugin_startup() { try { - EOS_ASSERT( chain_config->read_mode != db_read_mode::IRREVERSIBLE || !accept_transactions, plugin_config_exception, - "read-mode = irreversible. transactions should not be enabled by enable_accept_transactions" ); try { auto shutdown = [](){ return app().quit(); }; auto check_shutdown = [](){ return app().is_quiting(); }; diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 1d3a80e486..a8d43909e8 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -4275,14 +4275,7 @@ namespace eosio { chain_id = chain_plug->get_chain_id(); fc::rand_pseudo_bytes( node_id.data(), node_id.data_size()); - const controller& cc = chain_plug->chain(); - if( cc.get_read_mode() == db_read_mode::IRREVERSIBLE ) { - if( p2p_accept_transactions ) { - p2p_accept_transactions = false; - fc_wlog( logger, "p2p-accept-transactions set to false due to read-mode: irreversible" ); - } - } if( p2p_accept_transactions ) { chain_plug->enable_accept_transactions(); } From 39cce9e3cfddba45b3cd4f001001fdab828dd8f4 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 15 Nov 2024 13:47:52 -0600 Subject: [PATCH 02/10] GH-1030 Add irreversible and speculative nodes to head nodes in distributed transaction test --- tests/distributed-transactions-test.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/distributed-transactions-test.py b/tests/distributed-transactions-test.py index 9cda9456f8..9bca542358 100755 --- a/tests/distributed-transactions-test.py +++ b/tests/distributed-transactions-test.py @@ -12,7 +12,7 @@ # Performs currency transfers between N accounts sent to http endpoints of # N nodes and verifies, after a steady state is reached, that the accounts # balances are correct -# if called with --nodes-file it will will load a json description of nodes +# if called with --nodes-file it will load a json description of nodes # that are already running and run distributed test against them (not # currently testing this feature) # @@ -22,20 +22,19 @@ errorExit=Utils.errorExit appArgs = AppArgs() -extraArgs = appArgs.add_bool(flag="--speculative", help="Run nodes in read-mode=speculative") -args=TestHelper.parse_args({"-p","-n","-d","-s","--nodes-file","--seed", "--speculative", "--activate-if" +args=TestHelper.parse_args({"-p","-n","-d","-s","--nodes-file","--seed", "--activate-if" ,"--dump-error-details","-v","--leave-running","--keep-logs","--unshared"}, applicationSpecificArgs=appArgs) pnodes=args.p topo=args.s delay=args.d total_nodes = pnodes if args.n < pnodes else args.n +total_nodes = total_nodes if total_nodes > pnodes + 4 else pnodes + 4 debug=args.v nodesFile=args.nodes_file dontLaunch=nodesFile is not None seed=args.seed dumpErrorDetails=args.dump_error_details -speculative=args.speculative activateIF=args.activate_if Utils.Debug=debug @@ -64,11 +63,13 @@ (pnodes, total_nodes-pnodes, topo, delay)) Print("Stand up cluster") - extraNodeosArgs = "" - if speculative: - extraNodeosArgs = " --read-mode speculative " + specificExtraNodeosArgs = {} + specificExtraNodeosArgs[total_nodes-1] = f' --read-mode irreversible ' + specificExtraNodeosArgs[total_nodes-2] = f' --read-mode irreversible ' + specificExtraNodeosArgs[total_nodes-3] = f' --read-mode speculative ' + specificExtraNodeosArgs[total_nodes-4] = f' --read-mode speculative ' - if cluster.launch(pnodes=pnodes, totalNodes=total_nodes, topo=topo, delay=delay, extraNodeosArgs=extraNodeosArgs, activateIF=activateIF) is False: + if cluster.launch(pnodes=pnodes, totalNodes=total_nodes, topo=topo, delay=delay, specificExtraNodeosArgs=specificExtraNodeosArgs, activateIF=activateIF) is False: errorExit("Failed to stand up eos cluster.") Print ("Wait for Cluster stabilization") From 3881c890498b10117bae8c74e55659131b8bec9e Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 15 Nov 2024 13:49:33 -0600 Subject: [PATCH 03/10] GH-1030 Remove separate speculative test of distributed transactions. The distributed transaction test now always tests speculative, irreversible, and head. --- tests/CMakeLists.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8c77c638de..d9fe61e955 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -228,8 +228,6 @@ set_property(TEST get_account_test PROPERTY LABELS nonparallelizable_tests) add_test(NAME distributed-transactions-test COMMAND tests/distributed-transactions-test.py -d 2 -p 4 -n 6 -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST distributed-transactions-test PROPERTY LABELS nonparallelizable_tests) -add_test(NAME distributed-transactions-speculative-test COMMAND tests/distributed-transactions-test.py -d 2 -p 4 -n 6 --speculative -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) -set_property(TEST distributed-transactions-speculative-test PROPERTY LABELS nonparallelizable_tests) add_test(NAME distributed-transactions-if-test COMMAND tests/distributed-transactions-test.py -d 2 -p 4 -n 6 --activate-if -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST distributed-transactions-if-test PROPERTY LABELS nonparallelizable_tests) add_test(NAME restart-scenarios-test-resync COMMAND tests/restart-scenarios-test.py -c resync -p4 -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) From 15b32e0851ba433a9212c0a8684e96a0e8ec7a05 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 18 Nov 2024 16:52:26 -0600 Subject: [PATCH 04/10] GH-1030 Reset pending_savanna_lib_id when resetting root --- libraries/chain/fork_database.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index dbcadb8f92..706398f7d0 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -192,10 +192,11 @@ namespace eosio::chain { template void fork_database_impl::reset_root_impl( const bsp_t& root_bsp ) { - index.clear(); assert(root_bsp); root = root_bsp; root->set_valid(true); + pending_savanna_lib_id = block_id_type{}; + index.clear(); } template From 6d43d6417c4f6cd73c80d1296b69985aade56e6b Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 18 Nov 2024 19:24:17 -0600 Subject: [PATCH 05/10] GH-1030 Don't interrupt start_block in irreversible mode for received_block >= pending_block since blocks are not processed until they become irreversible --- plugins/producer_plugin/producer_plugin.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 4ceb1463a3..9199652a79 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -687,6 +687,7 @@ class producer_plugin_impl : public std::enable_shared_from_this _signature_providers; chain::bls_pub_priv_key_map_t _finalizer_keys; // public, private std::set _producers; + chain::db_read_mode _db_read_mode = db_read_mode::HEAD; boost::asio::deadline_timer _timer; block_timing_util::producer_watermarks _producer_watermarks; pending_block_mode _pending_block_mode = pending_block_mode::speculating; @@ -1528,10 +1529,12 @@ void producer_plugin_impl::plugin_startup() { dlog("producer plugin: plugin_startup() begin"); chain::controller& chain = chain_plug->chain(); - EOS_ASSERT(_producers.empty() || chain.get_read_mode() != chain::db_read_mode::IRREVERSIBLE, plugin_config_exception, + _db_read_mode = chain.get_read_mode(); + + EOS_ASSERT(_producers.empty() || _db_read_mode != chain::db_read_mode::IRREVERSIBLE, plugin_config_exception, "node cannot have any producer-name configured because block production is impossible when read_mode is \"irreversible\""); - EOS_ASSERT(_finalizer_keys.empty() || chain.get_read_mode() != chain::db_read_mode::IRREVERSIBLE, plugin_config_exception, + EOS_ASSERT(_finalizer_keys.empty() || _db_read_mode != chain::db_read_mode::IRREVERSIBLE, plugin_config_exception, "node cannot have any finalizers configured because finalization is impossible when read_mode is \"irreversible\""); EOS_ASSERT(_producers.empty() || chain.get_validation_mode() == chain::validation_mode::FULL, plugin_config_exception, @@ -1972,8 +1975,11 @@ bool producer_plugin_impl::should_interrupt_start_block(const fc::time_point& de if (in_producing_mode()) { return deadline <= fc::time_point::now(); } - // if we can produce then honor deadline so production starts on time - return (!_producers.empty() && deadline <= fc::time_point::now()) || (_received_block >= pending_block_num); + // if we can produce then honor deadline so production starts on time. + // if in irreversible mode then a received block should not interrupt since the incoming block is not processed until + // it becomes irreversible. We could check if LIB changed, but doesn't seem like the extra complexity is worth it. + return (!_producers.empty() && deadline <= fc::time_point::now()) + || (_db_read_mode != db_read_mode::IRREVERSIBLE && _received_block >= pending_block_num); } producer_plugin_impl::start_block_result producer_plugin_impl::start_block() { From 72f673a775472bde42798d9bf460482019ee8743 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 18 Nov 2024 19:44:39 -0600 Subject: [PATCH 06/10] GH-1030 Just have one of each type of read-mode --- tests/distributed-transactions-test.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/distributed-transactions-test.py b/tests/distributed-transactions-test.py index 9bca542358..f95fd422cd 100755 --- a/tests/distributed-transactions-test.py +++ b/tests/distributed-transactions-test.py @@ -29,7 +29,7 @@ topo=args.s delay=args.d total_nodes = pnodes if args.n < pnodes else args.n -total_nodes = total_nodes if total_nodes > pnodes + 4 else pnodes + 4 +total_nodes = total_nodes if total_nodes > pnodes + 3 else pnodes + 3 debug=args.v nodesFile=args.nodes_file dontLaunch=nodesFile is not None @@ -64,10 +64,9 @@ Print("Stand up cluster") specificExtraNodeosArgs = {} - specificExtraNodeosArgs[total_nodes-1] = f' --read-mode irreversible ' + specificExtraNodeosArgs[total_nodes-1] = f' --read-mode head ' specificExtraNodeosArgs[total_nodes-2] = f' --read-mode irreversible ' specificExtraNodeosArgs[total_nodes-3] = f' --read-mode speculative ' - specificExtraNodeosArgs[total_nodes-4] = f' --read-mode speculative ' if cluster.launch(pnodes=pnodes, totalNodes=total_nodes, topo=topo, delay=delay, specificExtraNodeosArgs=specificExtraNodeosArgs, activateIF=activateIF) is False: errorExit("Failed to stand up eos cluster.") From 6195d6b7b96ba2d00f71454829eef5f263fe577e Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 19 Nov 2024 09:14:53 -0600 Subject: [PATCH 07/10] GH-1030 Do not test irreversible mode in legacy as the trxs take way too long --- tests/distributed-transactions-test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/distributed-transactions-test.py b/tests/distributed-transactions-test.py index f95fd422cd..b383f51035 100755 --- a/tests/distributed-transactions-test.py +++ b/tests/distributed-transactions-test.py @@ -65,7 +65,8 @@ Print("Stand up cluster") specificExtraNodeosArgs = {} specificExtraNodeosArgs[total_nodes-1] = f' --read-mode head ' - specificExtraNodeosArgs[total_nodes-2] = f' --read-mode irreversible ' + if activateIF: # irreversible mode speculative trx execution not recommended in legacy mode + specificExtraNodeosArgs[total_nodes-2] = f' --read-mode irreversible ' specificExtraNodeosArgs[total_nodes-3] = f' --read-mode speculative ' if cluster.launch(pnodes=pnodes, totalNodes=total_nodes, topo=topo, delay=delay, specificExtraNodeosArgs=specificExtraNodeosArgs, activateIF=activateIF) is False: From 2b9cd9527ee1514d0ca7549539289dd9ecdbe716 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 19 Nov 2024 09:16:29 -0600 Subject: [PATCH 08/10] GH-1030 Add an irreversible_mode() method. Add warning if accepting trxs in irreversible mode when not in savanna --- plugins/producer_plugin/producer_plugin.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index af0df03f15..5b8f7e57d0 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -834,6 +834,10 @@ class producer_plugin_impl : public std::enable_shared_from_thischain(); auto before = _unapplied_transactions.size(); @@ -1535,10 +1539,10 @@ void producer_plugin_impl::plugin_startup() { chain::controller& chain = chain_plug->chain(); _db_read_mode = chain.get_read_mode(); - EOS_ASSERT(!is_configured_producer() || _db_read_mode != chain::db_read_mode::IRREVERSIBLE, plugin_config_exception, + EOS_ASSERT(!is_configured_producer() || !irreversible_mode(), plugin_config_exception, "node cannot have any producer-name configured because block production is impossible when read_mode is \"irreversible\""); - EOS_ASSERT(_finalizer_keys.empty() || _db_read_mode != chain::db_read_mode::IRREVERSIBLE, plugin_config_exception, + EOS_ASSERT(_finalizer_keys.empty() || !irreversible_mode(), plugin_config_exception, "node cannot have any finalizers configured because finalization is impossible when read_mode is \"irreversible\""); EOS_ASSERT(!is_configured_producer() || chain.get_validation_mode() == chain::validation_mode::FULL, plugin_config_exception, @@ -1590,6 +1594,10 @@ void producer_plugin_impl::plugin_startup() { _irreversible_block_time = fc::time_point::maximum(); } + if (!_is_savanna_active && irreversible_mode() && chain_plug->accept_transactions()) { + wlog("Accepting speculative transaction execution not recommended in read-mode=irreversible"); + } + if (is_configured_producer()) { ilog("Launching block production for ${n} producers at ${time}.", ("n", _producers.size())("time", fc::time_point::now())); @@ -1985,7 +1993,7 @@ bool producer_plugin_impl::should_interrupt_start_block(const fc::time_point& de // if in irreversible mode then a received block should not interrupt since the incoming block is not processed until // it becomes irreversible. We could check if LIB changed, but doesn't seem like the extra complexity is worth it. return (is_configured_producer() && deadline <= fc::time_point::now()) - || (_db_read_mode != db_read_mode::IRREVERSIBLE && _received_block >= pending_block_num); + || (!irreversible_mode() && _received_block >= pending_block_num); } producer_plugin_impl::start_block_result producer_plugin_impl::start_block() { From f04371166908157b6403e476151ea7e92d9a13a0 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 19 Nov 2024 09:17:10 -0600 Subject: [PATCH 09/10] GH-1030 Use 5 minutes instead of 5 seconds to match heuristic for syncing in controller --- plugins/producer_plugin/producer_plugin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 5b8f7e57d0..21bd815ee0 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -2095,7 +2095,7 @@ producer_plugin_impl::start_block_result producer_plugin_impl::start_block() { // Determine if we are syncing: if we have recently started an old block then assume we are syncing if (last_start_block_time < now + fc::microseconds(config::block_interval_us)) { auto head_block_age = now - chain.head().block_time(); - if (head_block_age > fc::seconds(5)) + if (head_block_age > fc::minutes(5)) return start_block_result::waiting_for_block; // if syncing no need to create a block just to immediately abort it } last_start_block_time = now; From 792f4778182d9c80ac7cfe2995b05e6d060b8776 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 19 Nov 2024 14:37:51 -0600 Subject: [PATCH 10/10] GH-1030 Add indication that node is in legacy consensus mode. --- plugins/producer_plugin/producer_plugin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 21bd815ee0..c271654dee 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -1595,7 +1595,7 @@ void producer_plugin_impl::plugin_startup() { } if (!_is_savanna_active && irreversible_mode() && chain_plug->accept_transactions()) { - wlog("Accepting speculative transaction execution not recommended in read-mode=irreversible"); + wlog("Legacy consensus active. Accepting speculative transaction execution not recommended in read-mode=irreversible"); } if (is_configured_producer()) {