Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support speculative trx execution in irreversible mode #1036

Merged
merged 11 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions libraries/chain/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}
Expand All @@ -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 );
}
Expand Down
3 changes: 2 additions & 1 deletion libraries/chain/fork_database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,11 @@ namespace eosio::chain {

template<class BSP>
void fork_database_impl<BSP>::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();
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reset of pending_savanna_lib_id is needed as starting in irreversible mode can reset the forkdb root to head. If the node is started again then it would try and advance LIB and segfault because the fork db didn't have anything in the index which means forkdb.head() returns nullptr. I attempted to reproduce this in 1.0.x, but couldn't find a way to trigger it. Not sure if we should backport it to 1.0.x or not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not backport for now as it is a rare case and 1.1.x is not far away.

template<class BSP>
Expand Down
8 changes: 0 additions & 8 deletions plugins/chain_plugin/chain_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -942,12 +942,6 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) {
}
api_accept_transactions = options.at( "api-accept-transactions" ).as<bool>();

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();
}
Expand Down Expand Up @@ -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(); };
Expand Down
7 changes: 0 additions & 7 deletions plugins/net_plugin/net_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4277,14 +4277,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();
}
Expand Down
24 changes: 19 additions & 5 deletions plugins/producer_plugin/producer_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,7 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
std::map<chain::public_key_type, signature_provider_type> _signature_providers;
chain::bls_pub_priv_key_map_t _finalizer_keys; // public, private
std::set<chain::account_name> _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;
Expand Down Expand Up @@ -833,6 +834,10 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
return !_producers.empty();
}

bool irreversible_mode() const {
return _db_read_mode == db_read_mode::IRREVERSIBLE;
}

void on_accepted_block(const signed_block_ptr& block, const block_id_type& id) {
auto& chain = chain_plug->chain();
auto before = _unapplied_transactions.size();
Expand Down Expand Up @@ -1532,10 +1537,12 @@ void producer_plugin_impl::plugin_startup() {
dlog("producer plugin: plugin_startup() begin");

chain::controller& chain = chain_plug->chain();
EOS_ASSERT(!is_configured_producer() || chain.get_read_mode() != chain::db_read_mode::IRREVERSIBLE, plugin_config_exception,
_db_read_mode = chain.get_read_mode();

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() || chain.get_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,
Expand Down Expand Up @@ -1587,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()));

Expand Down Expand Up @@ -1978,8 +1989,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 (is_configured_producer() && 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 (is_configured_producer() && deadline <= fc::time_point::now())
|| (!irreversible_mode() && _received_block >= pending_block_num);
}

producer_plugin_impl::start_block_result producer_plugin_impl::start_block() {
Expand Down Expand Up @@ -2081,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;
Expand Down
2 changes: 0 additions & 2 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down
17 changes: 9 additions & 8 deletions tests/distributed-transactions-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
#
Expand All @@ -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 + 3 else pnodes + 3
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
Expand Down Expand Up @@ -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 head '
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, 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")
Expand Down