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

reset cache after heal #5266

Merged

Conversation

matkt
Copy link
Contributor

@matkt matkt commented Mar 24, 2023

PR description

We found that sometimes after the heal we still have old snapshot in the cache because after the clearTrieLog some transaction pool thread are still running
The fix is to clear the cache after the snapsync and also add a snapshot of the new head

Fixed Issue(s)

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
@github-actions
Copy link

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • I have considered running ./gradlew acceptanceTestNonMainnet locally if my PR affects non-mainnet modules.
  • I thought about the changelog and included a changelog update if required.

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.

LGTM, naming and documenting intent is probably wise considering the effort went into making bonsai more dev friendly

@@ -338,6 +338,9 @@ public TrieLogManager getTrieLogManager() {
@Override
public void setArchiveStateUnSafe(final BlockHeader blockHeader) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this method name should change to reflect its use. IMO it isn't setting the archive unsafe for use like the name implies, but rather is resetting the archive cache and adding the new pivot as the only entry. Perhaps resetArchiveStateTo(BlockHeader)

Similarly, BonsaiWorldState.setArchiveUnsafe should be something more along the lines of resetWorldStateTo(BlockHeader)

These methods could use javadoc describing the intent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated the code

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
@matkt matkt requested a review from garyschulte March 24, 2023 20:34
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 a064a4a into hyperledger:main Mar 30, 2023
@matkt matkt deleted the feature/fix-reset-cache-after-heal branch July 5, 2023 07:23
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
* reset cache after heal

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
* reset cache after heal

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
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