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

Fix transaction pool issue #4964

Merged
merged 5 commits into from
Jan 22, 2023

Conversation

matkt
Copy link
Contributor

@matkt matkt commented Jan 19, 2023

Signed-off-by: Karim TAAM [email protected]

PR description

There has been a regression on Besu regarding the transaction pool. Indeed it seems that the transaction pool validation part does not make a copy of the worldstate during the GetMutable and this means that we can have a version which can change and which is not safe. A copy mechanism has been set up which allows to use the snapshot and to protect against worldstate modifications during validation. it was used elsewhere except in transaction pool validation and block replay.

This caused us to invalidate too many transactions

Fixed Issue(s)

Documentation

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

Changelog

}
return mutableWorldstate.map(
Copy link
Contributor

@garyschulte garyschulte Jan 19, 2023

Choose a reason for hiding this comment

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

on second thought, this has the potential to leak snapshots and prevent compaction if the mutable worldstate is not explicitly closed.

Copy link
Contributor Author

@matkt matkt Jan 20, 2023

Choose a reason for hiding this comment

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

I will do as before. I cannot see easy way to manage that . I wanted to create an OptionalMutableWorldstate with only a ifPresent that close the ressource automatically but seems to be a big and complex modification

@matkt matkt force-pushed the feature/fix-transaction-pool branch from 53bc9fb to db0b785 Compare January 20, 2023 09:27
matkt and others added 4 commits January 21, 2023 02:35
Signed-off-by: Karim TAAM <[email protected]>
* make RocksDBSnapshotTransction.commit() no-op rather than throw
* in SnaphotUpdater:
 **  keep a single reference to snapshot transaction rather than storage and accessor
 ** commit all storage transactions instead of just trieLogTransaction (for rocksdb all other tx commits are no-op)

Signed-off-by: garyschulte <[email protected]>
Signed-off-by: garyschulte <[email protected]>
@garyschulte garyschulte marked this pull request as ready for review January 22, 2023 06:47
Copy link
Contributor

@garyschulte garyschulte left a comment

Choose a reason for hiding this comment

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

👍

@garyschulte garyschulte merged commit e36b1f6 into hyperledger:main Jan 22, 2023
jflo pushed a commit to jflo/besu that referenced this pull request Feb 5, 2023
* fix transaction pool issue
* add block replay
* support in-memory snapshots

Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: garyschulte <[email protected]>
jflo pushed a commit to jflo/besu that referenced this pull request Feb 5, 2023
* fix transaction pool issue
* add block replay
* support in-memory snapshots

Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: garyschulte <[email protected]>
Signed-off-by: Justin Florentine <[email protected]>
jflo added a commit that referenced this pull request Feb 8, 2023
* Fix transaction pool issue (#4964)
* Cache empty slots (#4874)
* clear after each block and copy during clone
* fix transaction pool issue
* add block replay
* support in-memory snapshots
* Keep Worldstate Storage open for Bonsai archive latest layer (#5039)
* bonsai layered worldstate subscription
* unsubscribe from worldstatestorage on close of BonsaiLayeredWorldState
* minor txpool logging improvements
* Avoid triggering a calculate root hash when empty slot cache is not empty.
* use the updater cache to get an account during block processing (#4897)
* Worldstate-only resync behavior (#4875)
* use debug rpc endpoint to resync worldstate
* Reset transaction pool state every time the initial sync is done
* init heal code



---------

Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: garyschulte <[email protected]>
Signed-off-by: Justin Florentine <[email protected]>
Signed-off-by: Ameziane H <[email protected]>
Signed-off-by: ahamlat <[email protected]>
Signed-off-by: Simon Dudley <[email protected]>
Co-authored-by: matkt <[email protected]>
Co-authored-by: garyschulte <[email protected]>
Co-authored-by: ahamlat <[email protected]>
Co-authored-by: Simon Dudley <[email protected]>
Co-authored-by: Fabio Di Fabio <[email protected]>
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
* fix transaction pool issue
* add block replay
* support in-memory snapshots

Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: garyschulte <[email protected]>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
* fix transaction pool issue
* add block replay
* support in-memory snapshots

Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: garyschulte <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants