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

Enable head sync only during period of non-finality #7784

Merged
merged 8 commits into from
Nov 12, 2020

Conversation

farazdagi
Copy link
Contributor

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

  • The --head-sync helps us not to re-fetch blocks which have already been fetched and saved in DB. However, if node's finalization checkpoint is close enough to last saved head, then it is better to just use the finalized epoch on restarts.
  • This PR makes sure that --head-sync amends head only when non-finality period is long enough (configurable, currently set to 128 epochs).

Which issues(s) does this PR fix?

Fixes #7695

Other notes for review

@farazdagi farazdagi requested a review from a team as a code owner November 11, 2020 18:49
@farazdagi farazdagi self-assigned this Nov 11, 2020
@farazdagi farazdagi added the Ready For Review A pull request ready for code review label Nov 11, 2020
@codecov
Copy link

codecov bot commented Nov 11, 2020

Codecov Report

Merging #7784 (fb28365) into master (ec2e677) will increase coverage by 0.05%.
The diff coverage is 69.23%.

@@            Coverage Diff             @@
##           master    #7784      +/-   ##
==========================================
+ Coverage   62.19%   62.25%   +0.05%     
==========================================
  Files         429      429              
  Lines       30349    30358       +9     
==========================================
+ Hits        18877    18898      +21     
+ Misses       8534     8518      -16     
- Partials     2938     2942       +4     

Copy link
Member

@terencechain terencechain left a comment

Choose a reason for hiding this comment

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

one feedback on failing instead of proceeding silently

beacon-chain/blockchain/service.go Show resolved Hide resolved
@prylabs-bulldozer prylabs-bulldozer bot merged commit 79d19ea into master Nov 12, 2020
@delete-merged-branch delete-merged-branch bot deleted the disable-head-sync-before-finalized-epoch branch November 12, 2020 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--head-sync before sync has reached last finalized block will cause sync to fail
2 participants