Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Return failure traces when running with amqp_trx_plugin #9872

Merged
merged 3 commits into from
Jan 7, 2021
Merged

Return failure traces when running with amqp_trx_plugin #9872

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
73c2721
Return failure traces when running with amqp_trx_plugin
heifner Jan 4, 2021
9d20bc9
Update trx logging to report correct log message when returning trans…
heifner Jan 4, 2021
a7a9fe4
Report exception failure in logging and response
heifner Jan 4, 2021
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
29 changes: 21 additions & 8 deletions plugins/producer_plugin/producer_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,21 +458,28 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin

return [this, &trx, &chain, &next](const fc::static_variant<fc::exception_ptr, transaction_trace_ptr>& response) {
next(response);

fc::exception_ptr except_ptr;
if (response.contains<fc::exception_ptr>()) {
_transaction_ack_channel.publish(priority::low, std::pair<fc::exception_ptr, transaction_metadata_ptr>(response.get<fc::exception_ptr>(), trx));
except_ptr = response.get<fc::exception_ptr>();
} else if (response.get<transaction_trace_ptr>()->except) {
except_ptr = response.get<transaction_trace_ptr>()->except->dynamic_copy_exception();
}
_transaction_ack_channel.publish(priority::low, std::pair<fc::exception_ptr, transaction_metadata_ptr>(except_ptr, trx));

if (except_ptr) {
if (_pending_block_mode == pending_block_mode::producing) {
fc_dlog(_trx_failed_trace_log, "[TRX_TRACE] Block ${block_num} for producer ${prod} is REJECTING tx: ${txid} : ${why} ",
("block_num", chain.head_block_num() + 1)
("prod", get_pending_block_producer())
("txid", trx->id())
("why",response.get<fc::exception_ptr>()->what()));
("why", except_ptr->what()));
} else {
fc_dlog(_trx_failed_trace_log, "[TRX_TRACE] Speculative execution is REJECTING tx: ${txid} : ${why} ",
("txid", trx->id())
("why",response.get<fc::exception_ptr>()->what()));
("why", except_ptr->what()));
}
} else {
_transaction_ack_channel.publish(priority::low, std::pair<fc::exception_ptr, transaction_metadata_ptr>(nullptr, trx));
if (_pending_block_mode == pending_block_mode::producing) {
fc_dlog(_trx_successful_trace_log, "[TRX_TRACE] Block ${block_num} for producer ${prod} is ACCEPTING tx: ${txid}",
("block_num", chain.head_block_num() + 1)
Expand All @@ -491,7 +498,8 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
bool process_incoming_transaction_async(const transaction_metadata_ptr& trx,
bool persist_until_expired,
next_function<transaction_trace_ptr> next,
RetryLaterFunc retry_later)
RetryLaterFunc retry_later,
bool return_failure_trace = false)
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems pretty fragile to me. I think you'd be better of making sure that all downstream consumers can handle a transaction trace with an exception appropriately, and never extract the exception here. Either that or somehow use types that enforce consistency between return_failure_trace and next.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now this will only be true for amqp_trx_plugin. However, a new send_transaction2 in the future should also work this way (at least optionally). The community has requested this type of behavior.

{
bool exhausted = false;
chain::controller& chain = chain_plug->chain();
Expand Down Expand Up @@ -546,8 +554,12 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
if( !exhausted )
exhausted = block_is_exhausted();
} else {
auto e_ptr = trace->except->dynamic_copy_exception();
send_response( e_ptr );
if( return_failure_trace ) {
send_response( trace );
swatanabe-b1 marked this conversation as resolved.
Show resolved Hide resolved
} else {
auto e_ptr = trace->except->dynamic_copy_exception();
send_response( e_ptr );
}
}
} else {
if( persist_until_expired ) {
Expand Down Expand Up @@ -2094,7 +2106,8 @@ bool producer_plugin::execute_incoming_transaction(const chain::transaction_meta
};

const bool persist_until_expired = false;
bool exhausted = !my->process_incoming_transaction_async( trx, persist_until_expired, std::move(next), retry_later_func );
const bool return_failure_trace = true;
bool exhausted = !my->process_incoming_transaction_async( trx, persist_until_expired, std::move(next), retry_later_func, return_failure_trace );
if( exhausted ) {
if( my->_pending_block_mode == pending_block_mode::producing ) {
my->schedule_maybe_produce_block( true );
Expand Down