-
Notifications
You must be signed in to change notification settings - Fork 809
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
fatxpool
: do not use individual transaction listeners
#7316
Conversation
substrate/client/transaction-pool/src/fork_aware_txpool/multi_view_listener.rs
Outdated
Show resolved
Hide resolved
substrate/client/transaction-pool/src/fork_aware_txpool/multi_view_listener.rs
Outdated
Show resolved
Hide resolved
substrate/client/transaction-pool/src/fork_aware_txpool/view_store.rs
Outdated
Show resolved
Hide resolved
substrate/client/transaction-pool/src/fork_aware_txpool/multi_view_listener.rs
Show resolved
Hide resolved
substrate/client/transaction-pool/src/fork_aware_txpool/multi_view_listener.rs
Outdated
Show resolved
Hide resolved
…view_listener.rs Co-authored-by: Sebastian Kunert <skunert49@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some stylistic comments and some questions/opinions. LGTM but I will approve after clarifying the comments (if we don't need other changes than the ones I suggested).
substrate/client/transaction-pool/src/fork_aware_txpool/multi_view_listener.rs
Outdated
Show resolved
Hide resolved
substrate/client/transaction-pool/src/fork_aware_txpool/multi_view_listener.rs
Outdated
Show resolved
Hide resolved
substrate/client/transaction-pool/src/fork_aware_txpool/multi_view_listener.rs
Show resolved
Hide resolved
substrate/client/transaction-pool/src/fork_aware_txpool/multi_view_listener.rs
Show resolved
Hide resolved
substrate/client/transaction-pool/src/fork_aware_txpool/multi_view_listener.rs
Outdated
Show resolved
Hide resolved
All GitHub workflows were cancelled due to failure one of the required jobs. |
Description
During 2s block investigation it turned out that ForkAwareTxPool::register_listeners call takes significant amount of time.
This PR implements the idea outlined in #7071. Instead of having a separate listener for every transaction in each view, we now use a single stream of aggregated events per view, with each stream providing events for all transactions in that view. Each event is represented as a tuple: (transaction-hash, transaction-status). This significantly reduce the time required for
maintain
.Review Notes
(transaction-hash, transaction-status)
,MultiViewListener
now has a task. This task is responsible for:controller_receiver
which provides side-channel commands (likeAddView
orFinalizeTransaction
) sent from the transaction pool.Closes #7071