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

Implement Least Recently Used eviction in RequestCache . #110

Open
happy-san opened this issue Jul 30, 2021 · 7 comments · May be fixed by #112
Open

Implement Least Recently Used eviction in RequestCache . #110

happy-san opened this issue Jul 30, 2021 · 7 comments · May be fixed by #112
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@happy-san
Copy link
Collaborator

Is your feature request related to a problem? Please describe.

_cachedResponses doesn't have a limit on the number of _Cache's it can store and the expired _Cache is conditionally deleted, so a memory leak occurs.

Describe the solution you'd like

Implement an LRU algorithm.

Additional context

related issue

@happy-san happy-san added enhancement New feature or request good first issue Good for newcomers labels Jul 30, 2021
@happy-san

This comment has been minimized.

@happy-san
Copy link
Collaborator Author

happy-san commented Aug 14, 2021

@harisarang Maybe read up about TLRU- Time aware LRU scheme. I think TLRU scheme would be more relevant because LRU doesn't take Time to utilize (TLU) of the cached data into consideration.

Otherwise, for now, using the dcache package in cache function is fine.

@happy-san happy-san linked a pull request Aug 19, 2021 that will close this issue
1 task
@harisarang harisarang linked a pull request Aug 22, 2021 that will close this issue
1 task
@CoryADavis
Copy link

Is this library considered safe for production use without this fix?

@happy-san
Copy link
Collaborator Author

@CoryADavis I wouldn't recommend setting cachedSearchResultsTTL in production code yet.

@CoryADavis
Copy link

Ahh, understood, this issue is only related to a feature that is off by default.

Thank you!

@o-artebiakin
Copy link

@happy-san can we trigger cache clear manually?

@happy-san
Copy link
Collaborator Author

@o-artebiakin No, there's no way to clear the cache manually at the moment. Are you planning to use this feature in your app?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants