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

transactions in progress #8158

Merged
merged 2 commits into from
Nov 5, 2019
Merged

transactions in progress #8158

merged 2 commits into from
Nov 5, 2019

Conversation

heifner
Copy link
Contributor

@heifner heifner commented Nov 2, 2019

Change Description

  • net_plugin uses a callback on transaction processing to track size of transactions in progress. Therefore we need to always call transaction callback for proper accounting of transactions in progress.

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

Comment on lines 251 to 252
transaction_trace_ptr trace = std::make_shared<transaction_trace>();
trace->id = itr->trx_meta->id();
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't isn't creating the real transaction_trace. It's only initializing the fields needed for your specific use, which seems pretty fragile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I agree. I switched this to indicate duplicate transaction which is what it would have generated before this was integrated into the unapplied_transaction_queue.

@@ -221,8 +221,9 @@ class unapplied_transaction_queue {
iterator incoming_begin() { return queue.get<by_type>().lower_bound( trx_enum_type::incoming_persisted ); }
iterator incoming_end() { return queue.get<by_type>().end(); } // if changed to upper_bound, verify usage performance

/// callers responsibilty to call next() if applicable
Copy link
Contributor

Choose a reason for hiding this comment

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

caller's

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -242,10 +243,15 @@ class unapplied_transaction_queue {
}

template<typename Itr>
void removed( Itr itr ) {
void removed( Itr itr, bool call_next = true ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure that I like using a default argument that's only used at a single call site. Especially when the new default is different from the existing behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this and made it explicitly the callers responsibility.

@@ -221,8 +221,9 @@ class unapplied_transaction_queue {
iterator incoming_begin() { return queue.get<by_type>().lower_bound( trx_enum_type::incoming_persisted ); }
iterator incoming_end() { return queue.get<by_type>().end(); } // if changed to upper_bound, verify usage performance

/// callers responsibilty to call next() if applicable
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, at least one caller (

itr = _unapplied_transactions.erase( itr );
) does not call 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.

These should never have a next. However, to be extra safe I added in a check and call.

@heifner heifner requested a review from swatanabe-b1 November 5, 2019 20:10
@heifner heifner merged commit 806c7ee into develop Nov 5, 2019
@heifner heifner deleted the net-plugin-trx-calc branch November 5, 2019 22:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants