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

Add worldstate heal mechanism #5059

Merged
merged 4 commits into from
Feb 8, 2023
Merged

Add worldstate heal mechanism #5059

merged 4 commits into from
Feb 8, 2023

Conversation

matkt
Copy link
Contributor

@matkt matkt commented Feb 6, 2023

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

PR description

This PR add a heal mechanism fo the worldtstate in case of inconsistency (unable to trie node). To fix this, we start a quick heal of the worldstate automatically and once the fix is done we restart the block import.

After the detection of an invalid path

  • we delete this path to force the healing of this part.
  • then we delete the trielogs.
  • we select a pivot block (before of after the heal)
  • we move the blokchain to this pivot block (rewind or download the missing blocks)
  • then we launch a worldstate heal.

This feature can also heal a node that has been inconsistent for a long time, but it will take longer because there will be more nodes to heal. With this PR the healing will be done as soon as the problem is detected so there will not a lot to heal and it will be fast

Performed tests

  • Trigger multiple inconsistencies to fix (passed)
  • Fixed a node that has been inconsistent for a long time (passed)
  • Run a snapsync from scratch on goerli (passed)
  • Run a checkpoint sync from scratch on goelri (passed)
  • Run a checkpoint sync from scratch on main (in progress)
  • Run a validator teku+besu on goerli (passed)
  • Profile performance on this node to avoid perf regression (passed)

Fixed Issue(s)

#4379
#4785
#4768

Documentation

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

Changelog

Signed-off-by: Karim TAAM <[email protected]>
@matkt matkt mentioned this pull request Feb 6, 2023
2 tasks
@matkt matkt marked this pull request as ready for review February 6, 2023 07:25
@matkt
Copy link
Contributor Author

matkt commented Feb 6, 2023

Initial PR is here #4972

Comment on lines +47 to +49
return firstHeader ->
blockchain.contains(firstHeader.getParentHash())
&& blockchain.getChainHeadBlockNumber() + 1 >= firstHeader.getNumber();
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

looks fine though, I will assume it's an extra safety check.

Copy link
Contributor

@siladu siladu Feb 7, 2023

Choose a reason for hiding this comment

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

...ah just caught up with the new comments from the old PR :)
#4972 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's fine . It's a fix

Copy link
Contributor

@siladu siladu left a comment

Choose a reason for hiding this comment

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

Looks like be an accurate recreation of #4972 except for the minor change I mentioned in the comment

Copy link
Contributor

@siladu siladu left a comment

Choose a reason for hiding this comment

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

LGTM as someone with not much in-depth bonsai knowledge. Ideally someone else with more experience in this area will approve :)

Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

LGTM (but I am also no bonsai expert)
Needs a changelog entry

Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

LGTM

@macfarla macfarla enabled auto-merge (squash) February 8, 2023 02:21
@macfarla macfarla merged commit 77b55ed into hyperledger:main Feb 8, 2023
ensi321 pushed a commit to ensi321/besu that referenced this pull request Feb 19, 2023
Signed-off-by: Karim TAAM <[email protected]>
Co-authored-by: Simon Dudley <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
Signed-off-by: Karim TAAM <[email protected]>
Co-authored-by: Simon Dudley <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
@matkt matkt deleted the heal-bonsai branch September 22, 2023 09:37
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
Signed-off-by: Karim TAAM <[email protected]>
Co-authored-by: Simon Dudley <[email protected]>
Co-authored-by: Sally MacFarlane <[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.

3 participants