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

GCache2: Add a "Refresh" method to the Cache interface #20

Open
erwanor opened this issue Apr 12, 2018 · 1 comment
Open

GCache2: Add a "Refresh" method to the Cache interface #20

erwanor opened this issue Apr 12, 2018 · 1 comment

Comments

@erwanor
Copy link
Owner

erwanor commented Apr 12, 2018

GCache2 eviction policies are "event-driven". This means that we trigger an eviction when a user operation happen, be it a Get, Set, or getting the Length of the cache.

This is desirable most of the time but creates some peculiar situation some of the time. The reason for this is that GCache2 offers "expirable" entries according to wall-clock time.

There has been suggestions to deal with that. One of the most popular one seem to be the following: have GCache2 maintain a pool of workers tasked with cleaning the cache and trigger evictions when need be.

I don't think that this is a good idea to have such feature built-in. First the added complexity is not worth it. It's an overkill to engineer a solution like this just to improve the performance of a single feature.

However, I think we should make it easier for users to extend the library to support this without having them dive into the internals. The solution I have in mind is simple: offer a cache.Refresh method in the Cache interface. Such method would scan the cache and trigger evictions+clean-ups if need be. It would make it very easy for a user to create a worker routine tasked with refreshing the cache every N seconds or so.

Another benefit of this approach is that it fits well with #13, that is to embrace the principle of least-astonishment and stop having cache.Len have such huge potential side-effects.

type Cache interface {
// ... 
    Refresh() error
}
@erwanor erwanor added enhancement New feature or request v2 experimental labels Apr 12, 2018
@erwanor erwanor changed the title GCache2: Add an "Refresh" method to the Cache interface GCache2: Add a "Refresh" method to the Cache interface Apr 12, 2018
@erwanor erwanor added in progress and removed to do labels May 1, 2018
@ben-manes
Copy link

In the Java world (Guava and Caffeine), we called this cleanUp() and allow estimateSize() to include entries pending eviction. For us, refresh is to trigger reloading an entry, e.g. refreshAfterWrite can be combined with expireAfterWrite to reload the entry if in use rather than wait until it has been evicted and incur the penalty of the blocking call. I think that naming might fit here, too.

Actually, reviewing your library and I think you'd enjoy taking a deep dive into Caffeine. In particular see this article and slide deck. I suspect there is a lot of similarity to your current train of thought of how features might work.

@erwanor erwanor added to do and removed in progress labels May 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants