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

Committees cache rework (remove double lock) #13811

Open
wants to merge 20 commits into
base: develop
Choose a base branch
from

Conversation

v4lproik
Copy link
Contributor

@v4lproik v4lproik commented Mar 27, 2024

What type of PR is this?

  • Enhancement

What does this PR do? Why is it needed?

  • This PR reworks the beacon-chain caches implementation by leveraging the latest lru/v2 from Hashicorp.
  • It removes the "double lock" case where the BN locks the mutex in the caller (here, committee cache) and in the cache library.
  • It provides better readability, fewer type assertions, and avoids using the pattern interface{} as a function argument.
  • Better API: the cache key (the "seed") cannot be passed as a string in some methods or a [32]byte in others. Thus, the generation of the cache key stays in the caller.
  • It is more resilient as it prevents the case where a caller tries to insert a key associated with a nil value into the cache. This will ultimately result in less bugs / checks between the callers and the cache.
  • It enforces the use of "miss" and "hit" metrics across for the caches implementing the private interface LRUCache for better fine performance tuning. Also, it avoids the caller to manually handle the hit or miss increment itself making all the caches implementing the private interface to have the same behaviour.

Which issues(s) does this PR fix?
#13723

Other notes for review
I will keep opening PRs for the other caches (hashicorp lru) once the implementation is reviewed and accepted by the team.
Referencing this PR #13794

@v4lproik v4lproik marked this pull request as ready for review March 29, 2024 04:31
@v4lproik v4lproik requested a review from a team as a code owner March 29, 2024 04:31
@v4lproik v4lproik changed the title Committee cache rework Committee cache rework (remove double lock) Mar 29, 2024
@v4lproik v4lproik changed the title Committee cache rework (remove double lock) Committees cache rework (remove double lock) Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant