Keep Worldstate Storage open for Bonsai archive latest layer #5039

garyschulte · 2023-02-02T01:56:43Z

PR description

Fixes an issue where the latest block in BonsaiWorldStateArchive had a closed worldstate. Persisting BonsaiInMemoryWorldState instances were creating entries in the archive with a snapshot worldstate that would subsequently be closed. The net result was that queries using the worldstate for the latest block would frequently return Optional.empty() when accessing the closed worldstate.

this PR:

uses a dedicated copy of the Bonsai in memory worldstate when creating a trielog
adds a close() override to Bonsai layered worldstate to ensure the worldstate resources are cleaned up
makes minor log change

Fixed Issue(s)

fixes #5038

Documentation

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

Changelog

I thought about the changelog and included a changelog update if required.

macfarla

makes sense to me. changelog entry?

ahamlat · 2023-02-02T10:13:29Z

I tested eth_getBalance several times with this PR and the result matches what I can see in Etherscan or by executing eth_getBalance on 22.10.3 (which does not have the regression).
I don't see any performance regression on block processing and FCU (from prometheus metric and CPU profiling).

matkt

just asked some questions to understand

matkt · 2023-02-02T10:51:10Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiInMemoryWorldState.java

+            localUpdater,
+            newWorldStateRootHash,
+            blockHeader,
+            (BonsaiPersistedWorldState) this.copy());


is this copy can impact the performance ? I see some synchronized method , call to DB . if @ahamlat can confirm that it is safe

Agreed, an alternate implementation is to directly subscribe and unsubscribe to the underlying worldstate storage. This is cleaner code-wise, but if there is a performance impact we can directly sub/unsub to the storage itself.

matkt · 2023-02-02T10:54:49Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiLayeredWorldState.java

@@ -75,9 +75,29 @@ public Optional<BonsaiWorldView> getNextWorldView() {
  }

  public void setNextWorldView(final Optional<BonsaiWorldView> nextWorldView) {
+    maybeUnSubscribe();


If we close the nextview, how will the layeredworldstate be able to ask it to retrieve the different values? Is it going to recreate it when needed?

this is to close/unsubscribe from the worldstate this layer was initially created with, so we do not leak any snapshots. since this is a mutator, this is the last opportunity we will have to unsubscribe, typically when the BonsaiWorldStateArchive is responding to a BlockAddedEvent, and changes the nextWorldView.

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiLayeredWorldState.java

Signed-off-by: garyschulte <[email protected]>

* unsubscribe from worldstatestorage on close of BonsaiLayeredWorldState * minor txpool logging improvements Signed-off-by: garyschulte <[email protected]>

Signed-off-by: garyschulte <[email protected]>

…dger#5039) * bonsai layered worldstate subscription * unsubscribe from worldstatestorage on close of BonsaiLayeredWorldState * minor txpool logging improvements Signed-off-by: garyschulte <[email protected]>

…dger#5039) * bonsai layered worldstate subscription * unsubscribe from worldstatestorage on close of BonsaiLayeredWorldState * minor txpool logging improvements Signed-off-by: garyschulte <[email protected]> Signed-off-by: Justin Florentine <[email protected]>

…dger#5039) * bonsai layered worldstate subscription * unsubscribe from worldstatestorage on close of BonsaiLayeredWorldState * minor txpool logging improvements Signed-off-by: garyschulte <[email protected]>

* 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]>

…yperledger#5039)" This reverts commit e715010. Signed-off-by: Simon Dudley <[email protected]>

…5039)" (#5073) This reverts commit e715010. Signed-off-by: Simon Dudley <[email protected]>

Signed-off-by: Sally MacFarlane <[email protected]>

…yperledger#5039)" (hyperledger#5073) This reverts commit e715010. Signed-off-by: Simon Dudley <[email protected]>

…yperledger#5039)" (hyperledger#5073) This reverts commit e715010. Signed-off-by: Simon Dudley <[email protected]> Signed-off-by: garyschulte <[email protected]>

* Prepare for version 23.1.1-SNAPSHOT (#5067) Signed-off-by: Simon Dudley <[email protected]> * Add getPayloadBodiesByRangeV1 and getPayloadBodiesByHash engine methods (#4980) * Add engine get payload body methods and test Signed-off-by: Zhenyang Shi <[email protected]> Signed-off-by: Gabriel Fukushima <[email protected]> * Add header Signed-off-by: Zhenyang Shi <[email protected]> * Update result struct & add test Signed-off-by: Zhenyang Shi <[email protected]> * Change constant to use upper case Signed-off-by: Gabriel Fukushima <[email protected]> * Add PayloadBody class and withdrawals to response of methods Signed-off-by: Gabriel Fukushima <[email protected]> * Add unit tests Signed-off-by: Gabriel Fukushima <[email protected]> * Add changelog Signed-off-by: Gabriel Fukushima <[email protected]> * spotless Signed-off-by: Gabriel Fukushima <[email protected]> * Add check to prevent returning trailing null results past the head Signed-off-by: Gabriel Fukushima <[email protected]> * Add test to check trailing null post head scenario Signed-off-by: Gabriel Fukushima <[email protected]> * Split tests into pre and post shanghai Signed-off-by: Gabriel Fukushima <[email protected]> * spotless Signed-off-by: Gabriel Fukushima <[email protected]> * Rename methods Signed-off-by: Gabriel Fukushima <[email protected]> * Use getName() to log method name Signed-off-by: Gabriel Fukushima <[email protected]> * spotless Signed-off-by: Gabriel Fukushima <[email protected]> * Rename variable Signed-off-by: Gabriel Fukushima <[email protected]> * Call constructor directly Signed-off-by: Gabriel Fukushima <[email protected]> * Fix ByHash json parsing Signed-off-by: Simon Dudley <[email protected]> * Fix json parsing again Signed-off-by: Simon Dudley <[email protected]> * Add check to prevent unnecessary queries Signed-off-by: Gabriel Fukushima <[email protected]> * Refactor method Signed-off-by: Gabriel Fukushima <[email protected]> * Add new error code for EngineGetPayloadBodies methods Signed-off-by: Gabriel Fukushima <[email protected]> * Add return error for request above the API limit Signed-off-by: Gabriel Fukushima <[email protected]> * Add constructor for empty response Signed-off-by: Gabriel Fukushima <[email protected]> * add check for number of blocks requested and for requests of post head Signed-off-by: Gabriel Fukushima <[email protected]> * Add test to check error code when request exceeds API limits Signed-off-by: Gabriel Fukushima <[email protected]> * add constant for max blocks allowed per request Signed-off-by: Gabriel Fukushima <[email protected]> * spotless Signed-off-by: Gabriel Fukushima <[email protected]> * Fix some nits Signed-off-by: Gabriel Fukushima <[email protected]> * Add invalid params check Signed-off-by: Gabriel Fukushima <[email protected]> * Add tests for invalid params check Signed-off-by: Gabriel Fukushima <[email protected]> * Fix test and spotless Signed-off-by: Gabriel Fukushima <[email protected]> * Revert "Fix json parsing again" This reverts commit 558d325. Signed-off-by: Gabriel Fukushima <[email protected]> * Revert "Fix ByHash json parsing Signed-off-by: Simon Dudley <[email protected]>" This reverts commit 663e11e Signed-off-by: Gabriel Fukushima <[email protected]> * Use UnsignedLongParameter to cast params of the request Signed-off-by: Gabriel Fukushima <[email protected]> * Add optional withdrawals to the NewPayload log (#5021) Signed-off-by: Simon Dudley <[email protected]> * kubernetes and errorprone - update versions (#5013) * update errorprone and kubernetes versions * fixed errorprone issues in prod cod * fixed errorprone issues in test code --------- Signed-off-by: Sally MacFarlane <[email protected]> * Rename JsonRpcService to EngineJsonRpcService (#5036) * rename JsonRpcService to EngineJsonRpcService Signed-off-by: Daniel Lehrner <[email protected]> * Params should be single item of array type, not outer array of strings (#5037) Signed-off-by: Simon Dudley <[email protected]> * Add EIP-2537 (BLS12-381 curve precompiles) to Cancun (#5017) Add the BLS curve precompiles into the registry for cancun. All of the curve precompiles have been here since berlin, so this is just wiring them in. Signed-off-by: Danno Ferrin <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> * Use UnsignedLongParameter to cast params of the request Signed-off-by: Gabriel Fukushima <[email protected]> * Add optional withdrawals to the NewPayload log (#5021) Signed-off-by: Simon Dudley <[email protected]> * kubernetes and errorprone - update versions (#5013) * update errorprone and kubernetes versions * fixed errorprone issues in prod cod * fixed errorprone issues in test code --------- Signed-off-by: Sally MacFarlane <[email protected]> * Rename JsonRpcService to EngineJsonRpcService (#5036) * rename JsonRpcService to EngineJsonRpcService Signed-off-by: Daniel Lehrner <[email protected]> * Params should be single item of array type, not outer array of strings (#5037) Signed-off-by: Simon Dudley <[email protected]> * Add EIP-2537 (BLS12-381 curve precompiles) to Cancun (#5017) Add the BLS curve precompiles into the registry for cancun. All of the curve precompiles have been here since berlin, so this is just wiring them in. Signed-off-by: Danno Ferrin <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> * Convert start and count from hex to match JSON-RPC Spec standard Signed-off-by: Gabriel Fukushima <[email protected]> --------- Signed-off-by: Zhenyang Shi <[email protected]> Signed-off-by: Gabriel Fukushima <[email protected]> Signed-off-by: Simon Dudley <[email protected]> Signed-off-by: Sally MacFarlane <[email protected]> Signed-off-by: Daniel Lehrner <[email protected]> Signed-off-by: Danno Ferrin <[email protected]> Co-authored-by: Zhenyang Shi <[email protected]> Co-authored-by: Simon Dudley <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Co-authored-by: Daniel Lehrner <[email protected]> Co-authored-by: Danno Ferrin <[email protected]> * Revert "Keep Worldstate Storage open for Bonsai archive latest layer (#5039)" (#5073) This reverts commit e715010. Signed-off-by: Simon Dudley <[email protected]> * Support post merge forks at genesis for hive tests (#5019) Signed-off-by: Jason Frame <[email protected]> * Fix manifest docker not skipping interim builds for RCs (#5068) Signed-off-by: Jason Frame <[email protected]> * Fix PoS checkpoint validation (#5081) * change validation to lessThan Signed-off-by: Gabriel Fukushima <[email protected]> * Change exception message to PoS instead of Near head Signed-off-by: Gabriel Fukushima <[email protected]> * Add unit test and fix message of existing unit test Signed-off-by: Gabriel Fukushima <[email protected]> --------- Signed-off-by: Gabriel Fukushima <[email protected]> * moves check for init code length before balance check (#5077) Signed-off-by: Justin Florentine <[email protected]> * bump revision for 23.1.x release branch Signed-off-by: garyschulte <[email protected]> * Burn in build of 23.1.0 (#5093) * rebase off of burn-in release, remove unreleased 23.1.0-RC2 from CHANGELOG Signed-off-by: garyschulte <[email protected]> Signed-off-by: Simon Dudley <[email protected]> Signed-off-by: Ameziane H <[email protected]> Signed-off-by: Karim TAAM <[email protected]> * re-default global max rpc batch size to 1k (#5104) (#5105) * default global max rpc batch size to 1000 for now Signed-off-by: garyschulte <[email protected]> * Fix Layered World State issue (#5076) * Use the copy during prepareTrieLog instead of saveTrieLog * add final flag on BonsaiWorldStateUpdater * Use a copy of BonsaiInMemoryWorldState inside prepareTrieLog * add link to persisted worldstate storage * fix tests * Make a copy of the worldstate after committing changes to the trielog * spotless + remove maybeUnSubscribe in setNextWorldView * subscribe storage on layered worldstate * fix null issue * not close layered worldstate during getAccount * clean code * Add changelog entry --------- Signed-off-by: ahamlat <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Co-authored-by: Karim TAAM <[email protected]> * Add shanghaiTime to sepolia (#5088) Update and fix forkId tests Move timestamp forks from getForkBlockNumbers to getForkBlockTimestamps in JsonGenesisConfigOptions - this ultimately gets used to popoulate the ForkIdManager which handles lists of blocks and timestamps the same way so this hasn't changed any actual behaviour, but rather supports the test fixes. Implement TransitionProtocolSchedule.streamMilestoneBlocks as a concatenation of blockNumbers++blockTimestamps. This may have been a latent bug since it's used to update the node record when a fork transition occurs. Signed-off-by: Simon Dudley <[email protected]> * If a PoS block creation repetition takes less than a configurable dur… (#5048) * If a PoS block creation repetition takes less than a configurable duration, then waits before next repetition Signed-off-by: Fabio Di Fabio <[email protected]> * Update CHANGELOG Signed-off-by: Fabio Di Fabio <[email protected]> * Add unit test Signed-off-by: Fabio Di Fabio <[email protected]> * Update besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> * Update besu/src/main/java/org/hyperledger/besu/cli/options/unstable/MiningOptions.java Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> --------- Signed-off-by: Fabio Di Fabio <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> * Allow dashes in ethstats password (#5090) Signed-off-by: Simon Dudley <[email protected]> * reintroduce checking of block height for certain tasks when we are not PoS (Revert PR#3911) (#5083) * reintroduce checking of block height for certain tasks when we are not PoS (Revert PR#3911) Signed-off-by: Stefan Pingel <[email protected]> * Allow other users to read the /opt/besu dir when using docker (#5092) Signed-off-by: Rafael Matias <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> * Only use MAINNET version of KZG (#5095) Signed-off-by: Fabio Di Fabio <[email protected]> * add more context to exception messages and debug logging (#5066) Signed-off-by: Sally MacFarlane <[email protected]> * Fix block value calculation (#5100) * Add gasUsed calculation Signed-off-by: Gabriel Fukushima <[email protected]> * Fix unit test Signed-off-by: Gabriel Fukushima <[email protected]> * Fix typo Signed-off-by: Gabriel Fukushima <[email protected]> --------- Signed-off-by: Gabriel Fukushima <[email protected]> * Add 23.1.1-RC1 changelog Signed-off-by: Jason Frame <[email protected]> * Change version to 23.1.1-RC1 Signed-off-by: Jason Frame <[email protected]> --------- Signed-off-by: Simon Dudley <[email protected]> Signed-off-by: Zhenyang Shi <[email protected]> Signed-off-by: Gabriel Fukushima <[email protected]> Signed-off-by: Sally MacFarlane <[email protected]> Signed-off-by: Daniel Lehrner <[email protected]> Signed-off-by: Danno Ferrin <[email protected]> Signed-off-by: Jason Frame <[email protected]> Signed-off-by: Justin Florentine <[email protected]> Signed-off-by: garyschulte <[email protected]> Signed-off-by: Ameziane H <[email protected]> Signed-off-by: Karim TAAM <[email protected]> Signed-off-by: ahamlat <[email protected]> Signed-off-by: Fabio Di Fabio <[email protected]> Signed-off-by: Stefan Pingel <[email protected]> Signed-off-by: Rafael Matias <[email protected]> Co-authored-by: Simon Dudley <[email protected]> Co-authored-by: Gabriel Fukushima <[email protected]> Co-authored-by: Zhenyang Shi <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Co-authored-by: Daniel Lehrner <[email protected]> Co-authored-by: Danno Ferrin <[email protected]> Co-authored-by: Justin Florentine <[email protected]> Co-authored-by: garyschulte <[email protected]> Co-authored-by: ahamlat <[email protected]> Co-authored-by: Karim TAAM <[email protected]> Co-authored-by: Fabio Di Fabio <[email protected]> Co-authored-by: Stefan Pingel <[email protected]> Co-authored-by: Rafael Matias <[email protected]>

…dger#5039) * bonsai layered worldstate subscription * unsubscribe from worldstatestorage on close of BonsaiLayeredWorldState * minor txpool logging improvements Signed-off-by: garyschulte <[email protected]>

…yperledger#5039)" (hyperledger#5073) This reverts commit e715010. Signed-off-by: Simon Dudley <[email protected]>

…dger#5039) * bonsai layered worldstate subscription * unsubscribe from worldstatestorage on close of BonsaiLayeredWorldState * minor txpool logging improvements Signed-off-by: garyschulte <[email protected]>

…yperledger#5039)" (hyperledger#5073) This reverts commit e715010. Signed-off-by: Simon Dudley <[email protected]>

…dger#5039) * bonsai layered worldstate subscription * unsubscribe from worldstatestorage on close of BonsaiLayeredWorldState * minor txpool logging improvements Signed-off-by: garyschulte <[email protected]>

…yperledger#5039)" (hyperledger#5073) This reverts commit e715010. Signed-off-by: Simon Dudley <[email protected]>

garyschulte requested a review from matkt February 2, 2023 01:56

macfarla approved these changes Feb 2, 2023

View reviewed changes

matkt reviewed Feb 2, 2023

View reviewed changes

garyschulte force-pushed the bugfix/latest-layered-worldstate branch from c4274fb to 2f1faa2 Compare February 3, 2023 00:59

garyschulte added 5 commits February 3, 2023 13:48

bonsai layered worldstate subscription

faf0180

Signed-off-by: garyschulte <[email protected]>

* remove unnecessary worldstate copy in txpool

f6a8900

* unsubscribe from worldstatestorage on close of BonsaiLayeredWorldState * minor txpool logging improvements Signed-off-by: garyschulte <[email protected]>

refactor to use copy and close rather than explicit storage sub/unsub

2b265e9

Signed-off-by: garyschulte <[email protected]>

re-add worldstate copy to txpool

4780b25

Signed-off-by: garyschulte <[email protected]>

add changelog

3e92f02

Signed-off-by: garyschulte <[email protected]>

garyschulte force-pushed the bugfix/latest-layered-worldstate branch from eaba4b9 to 3e92f02 Compare February 3, 2023 21:48

garyschulte enabled auto-merge (squash) February 3, 2023 21:51

Merge branch 'main' into bugfix/latest-layered-worldstate

410e034

garyschulte merged commit e715010 into hyperledger:main Feb 4, 2023

siladu added a commit to siladu/besu that referenced this pull request Feb 9, 2023

Revert "Keep Worldstate Storage open for Bonsai archive latest layer (h…

bf39b7b

…yperledger#5039)" This reverts commit e715010. Signed-off-by: Simon Dudley <[email protected]>

siladu added a commit that referenced this pull request Feb 9, 2023

Revert "Keep Worldstate Storage open for Bonsai archive latest layer (#…

d7afa41

…5039)" (#5073) This reverts commit e715010. Signed-off-by: Simon Dudley <[email protected]>

macfarla added a commit to macfarla/besu that referenced this pull request Feb 9, 2023

redo of hyperledger#5039 without the saveTrieLog change

94e8742

Signed-off-by: Sally MacFarlane <[email protected]>

macfarla mentioned this pull request Feb 9, 2023

redo of #5039 without the saveTrieLog change #5074

Closed

3 tasks

siladu mentioned this pull request Feb 10, 2023

eth_getBalance returning 0x0 for latest block #5038

Closed

ensi321 pushed a commit to ensi321/besu that referenced this pull request Feb 19, 2023

Revert "Keep Worldstate Storage open for Bonsai archive latest layer (h…

568441e

…yperledger#5039)" (hyperledger#5073) This reverts commit e715010. Signed-off-by: Simon Dudley <[email protected]>

eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023

Revert "Keep Worldstate Storage open for Bonsai archive latest layer (h…

8f46a69

…yperledger#5039)" (hyperledger#5073) This reverts commit e715010. Signed-off-by: Simon Dudley <[email protected]>

garyschulte deleted the bugfix/latest-layered-worldstate branch January 24, 2024 21:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keep Worldstate Storage open for Bonsai archive latest layer #5039

Keep Worldstate Storage open for Bonsai archive latest layer #5039

garyschulte commented Feb 2, 2023 •

edited

Loading

macfarla left a comment

ahamlat commented Feb 2, 2023

matkt left a comment

matkt Feb 2, 2023

garyschulte Feb 3, 2023

matkt Feb 2, 2023

garyschulte Feb 3, 2023

Keep Worldstate Storage open for Bonsai archive latest layer #5039

Keep Worldstate Storage open for Bonsai archive latest layer #5039

Conversation

garyschulte commented Feb 2, 2023 • edited Loading

PR description

Fixed Issue(s)

Documentation

Changelog

macfarla left a comment

Choose a reason for hiding this comment

ahamlat commented Feb 2, 2023

matkt left a comment

Choose a reason for hiding this comment

matkt Feb 2, 2023

Choose a reason for hiding this comment

garyschulte Feb 3, 2023

Choose a reason for hiding this comment

matkt Feb 2, 2023

Choose a reason for hiding this comment

garyschulte Feb 3, 2023

Choose a reason for hiding this comment

garyschulte commented Feb 2, 2023 •

edited

Loading