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.

Sign up for GitHub

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

Jump to bottom

Das replace cache #2179

Merged
merged 3 commits into from
Mar 11, 2024
Merged

Das replace cache #2179

merged 3 commits into from
Mar 11, 2024

Conversation

Tristan-Wilson
Copy link
Member

Remove the dependency in the DAS on bigcache and use geth's lru.Cache instead which has better understood behavior.

@cla-bot cla-bot bot added the s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA. label Mar 11, 2024
amsanghi
amsanghi approved these changes Mar 11, 2024
View reviewed changes
Copy link
Contributor

@amsanghi amsanghi left a comment

Choose a reason for hiding this comment

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

LGTM

Curious though what was the rationale for going with bigCache earlier? I think I only suggested using bigCache back then but not able to remember why...

@Tristan-Wilson
Copy link
Member Author

Curious though what was the rationale for going with bigCache earlier? I think I only suggested using bigCache back then but not able to remember why...

I don't remember either 😆

@Tristan-Wilson Tristan-Wilson merged commit 3ed2d0d into master Mar 11, 2024
8 checks passed
@Tristan-Wilson Tristan-Wilson deleted the das-replace-cache branch March 11, 2024 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amsanghi amsanghi amsanghi approved these changes

@anodar anodar Awaiting requested review from anodar

Assignees
No one assigned
Labels
s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Tristan-Wilson @amsanghi