reverted validatorIndexCache #8354
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
Reverted the changes to validatorIndexCache, we'll have to take a look again for full electra support. Although the intent was to only index finalized validators, the result was to constantly be doing StateTransitions somehow to get indices, and the outcome was a serious slow down in the important state transitions that happen constantly, so as a starting point, this will put us back to releasable code that will be ok for Deneb. Before reverting, it seemed to process 1 state transition every 20 seconds or so, and even just rebuilding a handful of states was taking 10 minutes or longer, which is not really going to work in production. We'll also need to build out testing for this once we re-implement. Archive storage is already slow due to epoch processing, we can't afford to take this kind of increased performance hit. I'm guessing the tracker may still be part of an eventual solution, haven't removed at this point but i've cleaned it out functionally... i probably need to clean that before any merge... `BlockProcessorElectra` was changed to not use the cache to find validator index, as it's a simple operation, and it was causing problems. Signed-off-by: Paul Harris <[email protected]>
Signed-off-by: Paul Harris <[email protected]>
Signed-off-by: Paul Harris <[email protected]>
gfukushima
approved these changes
Jun 6, 2024
...src/main/java/tech/pegasys/teku/spec/logic/versions/electra/block/BlockProcessorElectra.java
Show resolved
Hide resolved
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.
Reverted the changes to validatorIndexCache, we'll have to take a look again for full electra support.
Although the intent was to only index finalized validators, the result was to constantly be doing StateTransitions somehow to get indices, and the outcome was a serious slow down in the important state transitions that happen constantly, so as a starting point, this will put us back to releasable code that will be ok for Deneb.
Before reverting, it seemed to process 1 state transition every 20 seconds or so, and even just rebuilding a handful of states was taking 10 minutes or longer, which is not really going to work in production.
We'll also need to build out testing for this once we re-implement. Archive storage is already slow due to epoch processing, we can't afford to take this kind of increased performance hit.
I'm guessing the tracker may still be part of an eventual solution, haven't removed at this point but i've cleaned it out functionally... i probably need to clean that before any merge...
BlockProcessorElectrawas changed to not use the cache to find validator index, as it's a simple operation, and it was causing problems.Documentation
doc-change-requiredlabel to this PR if updates are required.Changelog