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

Make transaction pool limits for sender based on pool size #4417

Conversation

garyschulte
Copy link
Contributor

PR description

Limit the portion of the transaction pool that a single sender can occupy to a percentage

  • defaults to 0.1%, or default of 5 per sender with default max txpool size
  • moves this config out of BesuCommand and into TransactionPoolOptions

This PR will see the default txpool configuration restrict the number of transactions from a single sender in the pool from 64 to 5. This should decrease the cost of transaction eviction during transaction processing, and yield a transaction pool with a greater percentage of executable transactions for building a block.

Fixed Issue(s)

relates to #4401

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if
    updates are required.

Changelog

Copy link
Contributor

@fab-10 fab-10 left a comment

Choose a reason for hiding this comment

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

Looks good, we just need to found a way to avoid a breaking change

Comment on lines -1221 to -1230

@Option(
names = {"--tx-pool-future-max-by-account"},
paramLabel = MANDATORY_INTEGER_FORMAT_HELP,
converter = PercentageConverter.class,
description =
"Maximum per account of currently unexecutable future transactions that can occupy the txpool (default: ${DEFAULT-VALUE})",
arity = "1")
private final Integer maxFutureTransactionsByAccount =
TransactionPoolConfiguration.MAX_FUTURE_TRANSACTION_BY_ACCOUNT;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to keep this for the moment, otherwise it is a breaking change, what we can do is to give an error if this and the new one are used together, or to say that this has no effect if the new one is used

@garyschulte garyschulte force-pushed the feature/tx-pool-by-sender-limit-percentage branch 3 times, most recently from a0f38dd to d3003a8 Compare September 21, 2022 20:47
@fab-10
Copy link
Contributor

fab-10 commented Sep 22, 2022

Please just update the CHANGELOG

@garyschulte garyschulte force-pushed the feature/tx-pool-by-sender-limit-percentage branch from 5dbad31 to 95b24a4 Compare September 23, 2022 00:49
@garyschulte garyschulte enabled auto-merge (squash) September 23, 2022 00:56
@garyschulte
Copy link
Contributor Author

burned in on mainnet along with 4423, 4425

…y sender is 5

move config into TransactionPoolOptions class

Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte garyschulte force-pushed the feature/tx-pool-by-sender-limit-percentage branch from 95b24a4 to 3544300 Compare September 23, 2022 04:11
@garyschulte garyschulte enabled auto-merge (squash) September 23, 2022 04:11
@garyschulte garyschulte merged commit 42086bf into hyperledger:main Sep 23, 2022
@garyschulte garyschulte deleted the feature/tx-pool-by-sender-limit-percentage branch September 23, 2022 08:11
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
…er#4417)

* make transaction pool limits for sender based on pool size.  default by sender is 5
move config into TransactionPoolOptions class

Signed-off-by: garyschulte <garyschulte@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants