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

Increase regen historical state cache size #5613

Merged
merged 4 commits into from
Apr 25, 2020
Merged

Conversation

terencechain
Copy link
Member

This PR fixes #5611. It has no impact to beacon node run time, it only impacts the duration when beacon node is generating and saving historical states. It's 8 mins given current chain length

Change list:

  • Increased cache size from 64 to 80 with rationale
  • Garbage collect at the end of regenHistoricalStates

@terencechain terencechain requested a review from a team as a code owner April 24, 2020 21:04
@terencechain terencechain self-assigned this Apr 24, 2020
@terencechain terencechain added the Ready For Review A pull request ready for code review label Apr 24, 2020
// Using max possible size to avoid using DB to save and retrieve pre state (slow)
// The size is 80 because block at slot 43772 built on top of block at slot 43693.
// That is the worst case.
cacheState, err := lru.New(80)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a global constant? Just so its not hardcoded into a parameter.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep! Done

@codecov
Copy link

codecov bot commented Apr 24, 2020

Codecov Report

Merging #5613 into master will increase coverage by 33.97%.
The diff coverage is 20.00%.

@@             Coverage Diff             @@
##           master    #5613       +/-   ##
===========================================
+ Coverage   13.04%   47.01%   +33.97%     
===========================================
  Files         116      239      +123     
  Lines        9285    20738    +11453     
===========================================
+ Hits         1211     9750     +8539     
- Misses       7865     9235     +1370     
- Partials      209     1753     +1544     

@terencechain terencechain merged commit 5fbf38c into master Apr 25, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix-nil-state branch April 25, 2020 14:59
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.

Could not generate historical state, pre state can't be nil
2 participants