Tune and make configurable the PeerTransactionTracker
#8537
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ae010be to
ffced9f
Compare
PeerTransactionTracker
d38f003 to
0625525
Compare
macfarla
approved these changes
Apr 11, 2025
besu/src/main/java/org/hyperledger/besu/cli/options/TransactionPoolOptions.java
Outdated
Show resolved
Hide resolved
besu/src/main/java/org/hyperledger/besu/cli/options/TransactionPoolOptions.java
Outdated
Show resolved
Hide resolved
siladu
approved these changes
Apr 11, 2025
besu/src/main/java/org/hyperledger/besu/cli/options/TransactionPoolOptions.java
Show resolved
Hide resolved
| @CommandLine.Option( | ||
| names = {PEER_TRACKER_FORGET_EVICTED_TXS_FLAG}, | ||
| paramLabel = "<BOOLEAN>", | ||
| hidden = true, |
There was a problem hiding this comment.
Does this need a fallback value so --Xpeer-tracker-forget-evicted-txs will work without "=true"?
There was a problem hiding this comment.
Good point, the important thing is that fallback value does not act as a default, because in this case the default is chosen at runtime depending on the txpool implementation
There was a problem hiding this comment.
Fallback value added
| config -> assertThat(config.getUnstable().getPeerTrackerForgetEvictedTxs()).isTrue(), | ||
| "--tx-pool", | ||
| LAYERED.name().toLowerCase(Locale.ROOT), | ||
| "--Xpeer-tracker-forget-evicted-txs=true"); |
There was a problem hiding this comment.
Maybe a another test for --Xpeer-tracker-forget-evicted-txs using the fallBack value if we want to support that
…in the peer tracker Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
…nPoolOptions.java Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
51f9003 to
11bf0e9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR description
In PR #7755 the default behavior of the peer transaction tracker changed from always remember the txs exchanged with a peer (at least until they fits in the cache) to forget a txs has been already exchanged in case it is evicted from the pool for reason not related to the validity.
While the new default solves issues with re-broadcasting the same tx in small networks, it does not works well for public networks, where there are many thousands of pending txs and a lot of independent nodes, since it can cause the broadcast and the re-evaluation of already seen txs, with the waste of network and computation resources.
So this PR changes the current default based on the txpool implementation that is chosen:
Moreover it is possible to overwrite the default behavior using the new experimental option
Xpeer-tracker-forget-evicted-txs.Another option has been added to customized the max size of the cache that keeps memory of the tx exchanged with each peers, so it can be tuned to adapt to increases in the pending txs number. The new option is
Xmax-tracked-seen-txs-per-peerand by default is now200_000entries, double from the previous hard coded value.Fixed Issue(s)
Thanks for sending a pull request! Have you done the following?
doc-change-requiredlabel to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew spotlessApply./gradlew build./gradlew acceptanceTest./gradlew integrationTest./gradlew ethereum:referenceTests:referenceTests