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

Inefficiency of txpool during block production #4333

Closed
crystalin opened this issue Apr 30, 2024 · 5 comments
Closed

Inefficiency of txpool during block production #4333

crystalin opened this issue Apr 30, 2024 · 5 comments
Labels
I10-unconfirmed Issue might be valid, but it's not yet known.

Comments

@crystalin
Copy link

crystalin commented Apr 30, 2024

When we have a lot of activity on Moonbeam, the txpool gets filled (as expected) over time, but that brings an issue for the block production. It happens often that the block production is going to wait on the txpool lock for a long time (130ms is the max IIRC), timeout and so produce empty blocks.

basic-authorship: [🌗] Timeout fired waiting for transaction pool at block #7073337. Proceeding with production.
...
sc_basic_authorship::basic_authorship: [🌗] 🎁 Prepared block for proposing at 7073338 (134 ms) [hash: 0xb9d9a1d5584ec06cd0045ac7e3466e6e2f7ce931192dbff31a90fb777024dd4d; parent_hash: 0xf90f…96ea; extrinsics (5): [0x217f…ac74, 0x5ae0…cac5, 0xef12…0427, 0x3a5c…6b7f, 0xad9a…621a]    

Would it be possible to pause the txpool ingest/revalidate prior to the block production ?

stagenet-collator-uko-txpool-debug.log
(I included logs of one of our test environment, search for block #7073338)

@github-actions github-actions bot added the I10-unconfirmed Issue might be valid, but it's not yet known. label Apr 30, 2024
@eskimor
Copy link
Member

eskimor commented Apr 30, 2024

Worth mentioning async backing is around the corner and should fix with this as well.

@crystalin
Copy link
Author

crystalin commented Apr 30, 2024

@eskimor This is using async backing already (@librelois to confirm)
(The relay is using Westend local and you can see the blocks are produced every 6s)

@librelois
Copy link
Contributor

Yes I confirm that theses load tests where ran against a network with async backing and 6s blocktime.

it seem's that the txpool is not spam resiliant, especially here: https://github.com/paritytech/polkadot-sdk/blob/master/substrate/client/transaction-pool/src/revalidation.rs#L164

We revalidate 25% of the pending transactions at each block, that can be problematic where there is a lot of transactions, we should probably add an upper bound here.

@bkchr
Copy link
Member

bkchr commented Apr 30, 2024

uko-moon-moondev-1|stagenet-ct-collator-1 2024-04-29 16:28:02 [Parachain] 💤 Idle (2 peers), best: #600757 (0x3212…54e2), finalized #600751 (0x3bef…4adf), ⬇ 4.5kiB/s ⬆ 0.1kiB/s    
uko-moon-moondev-1|stagenet-collator-1 2024-04-29 16:28:01.240  INFO tokio-runtime-worker substrate: [Relaychain] 💤 Idle (33 peers), best: #14295752 (0xd209…7af4), finalized #14295749 (0xd8a4…ec4e), ⬇ 26.8kiB/s ⬆ 94.7kiB/s    
uko-moon-moondev-1|stagenet-collator-1 2024-04-29 16:28:01.267  INFO tokio-runtime-worker substrate: [🌗] 💤 Idle (15 peers), best: #7073334 (0x88e9…4c44), finalized #7073331 (0xfeaa…ce5c), ⬇ 50.0kiB/s ⬆ 226.6kiB/s   

Your logs are partly confusing :D Do you run there 3 different nodes? :D

Worth mentioning async backing is around the corner and should fix with this as well.

This is not really related to the issue here. Not should it fix this.

We revalidate 25% of the pending transactions at each block, that can be problematic where there is a lot of transactions, we should probably add an upper bound here.

Transaction validation generally should be quite lightweight. Maybe some upper bound makes sense, but the tx pool is also not unbounded.

If you change this:
https://github.com/moonbeam-foundation/moonbeam/blob/master/node/service/src/lib.rs#L527-L530

To

ParachainBlockImport::new

The transaction pool should be ready when you build a block.

The overall tx pool behavior should improve with: #1202

@bkchr
Copy link
Member

bkchr commented Jun 3, 2024

Closing as I think this will be solved with #1202.

@bkchr bkchr closed this as completed Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I10-unconfirmed Issue might be valid, but it's not yet known.
Projects
None yet
Development

No branches or pull requests

4 participants