-
Notifications
You must be signed in to change notification settings - Fork 943
Delete attester cache #8469
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
base: unstable
Are you sure you want to change the base?
Delete attester cache #8469
Conversation
b95a664 to
224fbff
Compare
Co-authored-by: Michael Sproul <[email protected]>
224fbff to
ad4cf8f
Compare
| %request_slot, | ||
| "Attester cache miss" | ||
| ); | ||
| let (advanced_state_root, mut state) = self |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add metrics for the number of times we hit this code path? Would be nice to have a histogram here or instrumentation of the time spent computing the hot state
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea - I've instrumented the produce_unaggregated_attestation code path, and we already have a span for get_advanced_hot_state, so this should give us good visibility.
|
Nice simplification! |
eserilev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome! LGTM
Issue Addressed
Fixes attester cache write lock contention. Alternative to #8463.
Description
Deletes the attester cache which was causing lock contention across multiple hot code paths:
We now use the state cache directly via
get_advanced_hot_state, which builds all caches without lock contention.