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

fatxpool: rotator cache size now depends on pool's limits #7102

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

michalkucharczyk
Copy link
Contributor

@michalkucharczyk michalkucharczyk commented Jan 9, 2025

Description

This PR modifies the hard-coded size of extrinsics cache within PoolRotator to be inline with pool limits.

The problem was, that due to small size (comparing to number of txs in single block) of hard coded size:


excessive number of unnecessary verification were performed in prune_tags:
let reverified_transactions =
self.verify(at, pruned_transactions, CheckBannedBeforeVerify::Yes).await;

This was resulting in quite long durations of prune_tags execution time (which was ok for 6s, but becomes noticable for 2s blocks):

Pruning at HashAndNumber { number: 83, ... }. Resubmitting transactions: 6142, reverification took: 237.818955ms    
Pruning at HashAndNumber { number: 84, ... }. Resubmitting transactions: 5985, reverification took: 222.118218ms    
Pruning at HashAndNumber { number: 85, ... }. Resubmitting transactions: 5981, reverification took: 215.546847ms

The fix reduces the overhead:

Pruning at HashAndNumber { number: 92, ... }. Resubmitting transactions: 6325, reverification took: 14.728354ms    
Pruning at HashAndNumber { number: 93, ... }. Resubmitting transactions: 7030, reverification took: 23.973607ms    
Pruning at HashAndNumber { number: 94, ... }. Resubmitting transactions: 4465, reverification took: 9.532472ms    

Review Notes

I decided to leave the hardocded EXPECTED_SIZE for the legacy transaction pool. Removing verification of transactions during re-submission may negatively impact the behavior of the legacy (single-state) pool. As in long-term we probably want to deprecate old pool, I did not invest time to assess the impact of rotator change in behavior of the legacy pool.

@michalkucharczyk michalkucharczyk added the T0-node This PR/Issue is related to the topic “node”. label Jan 9, 2025
@michalkucharczyk michalkucharczyk changed the title Mku rotator size fix fatxpool: rotator limits updated Jan 9, 2025
@michalkucharczyk michalkucharczyk changed the title fatxpool: rotator limits updated fatxpool: rotator cache size now depends on pool's limits Jan 9, 2025
@michalkucharczyk
Copy link
Contributor Author

/cmd prdoc --bump minor --audience node_dev

@michalkucharczyk michalkucharczyk added the R0-silent Changes should not be mentioned in any release notes label Jan 9, 2025
prdoc/pr_7102.prdoc Outdated Show resolved Hide resolved
prdoc/pr_7102.prdoc Outdated Show resolved Hide resolved
prdoc/pr_7102.prdoc Outdated Show resolved Hide resolved
substrate/client/transaction-pool/src/graph/rotator.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants