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

refactor: handle cache mutex in common #28

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mkykadir
Copy link

Description

Mutex lock for cache can be handled from a common place instead of the on the method usage.

@mkykadir mkykadir requested a review from a team as a code owner July 19, 2024 12:40
@mkykadir mkykadir requested a review from mslipper July 19, 2024 12:40
@jelias2
Copy link
Contributor

jelias2 commented Jul 26, 2024

Thanks for the contribution!

StaticRPCHandler currently handles an cache interface, which can either be in-memory, redis cache, or something else in the future.

Its a good idea however my concern is by moving the rw.Mutex from the StaticRPCHandler to the cache.go, the StaticRPCHandler will now depend on private implementations to avoid locking errors if the cache implementation is extended to something else in the future.

@mkykadir
Copy link
Author

mkykadir commented Aug 5, 2024

yeah exactly, that was my point actually for this change. I think the caching mechanism should be responsible for concurrent access and lock mechanism. What if that new caching method may require some other way to avoid locking errors, or another caching library is to be integrated that already capable to handle concurrent access. And cache.go can be utilized somewhere else in proxyd and locks should already be handled.

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.

2 participants