Add cache to Eth1 RPC functions #5347
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nisdas
reviewed
Apr 8, 2020
| @@ -72,8 +80,9 @@ func (s *Service) BlockTimeByHeight(ctx context.Context, height *big.Int) (uint6 | |||
| // This is a naive implementation that will use O(ETH1_FOLLOW_DISTANCE) calls to cache | |||
| // or ETH1. This is called for multiple times but only changes every | |||
| // SlotsPerEth1VotingPeriod (1024 slots) so the whole method should be cached. | |||
| // How should caching the whole method be done? | |||
There was a problem hiding this comment.
Just for personal tracking as I try to figure out whats needed, will remove before the PR is ready for review.
0xKiwi
commented
Apr 8, 2020
| @@ -29,6 +29,8 @@ import ( | |||
|
|
|||
| // eth1DataNotification is a latch to stop flooding logs with the same warning. | |||
| var eth1DataNotification bool | |||
| var cachedEth1VotingStartTime uint64 | |||
| var cachedEth1Data *ethpb.Eth1Data | |||
There was a problem hiding this comment.
This caching is very simple, should we cache it on the beacon node side or is on the validator end fine?
There was a problem hiding this comment.
It should be fine if it lives in the beacon node side, that way many validator clients connected to one beacon node can benefit
Codecov Report
@@ Coverage Diff @@
## master #5347 +/- ##
===========================================
+ Coverage 12.56% 41.51% +28.94%
===========================================
Files 112 233 +121
Lines 8740 19385 +10645
===========================================
+ Hits 1098 8047 +6949
- Misses 7454 9949 +2495
- Partials 188 1389 +1201 |
rauljordan
approved these changes
Apr 9, 2020
michaelhly
pushed a commit
to michaelhly/prysm
that referenced
this pull request
May 17, 2020
…ticlabs#5876) * Revert "Add cache to Eth1 RPC functions (prysmaticlabs#5347)" This reverts commit 159ef3d.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #5340
This PR improves the RPC functions by adding a cache to both.