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

Q: refresh immediatelly optionally possible? #688

Closed
ecki opened this issue Mar 24, 2022 · 5 comments
Closed

Q: refresh immediatelly optionally possible? #688

ecki opened this issue Mar 24, 2022 · 5 comments

Comments

@ecki
Copy link

ecki commented Mar 24, 2022

I am currently looking to migrate some functions from Guava to Caffeine due to its better "absent-refresh-result" behavior.

In Guava you are forced to implement background refreshing yourself, however you have the advantage that you can actually implement a done Future and that would be used as the value for the triggering get.

I am not sure if this can be replicated by caffeine: In case the asyncReload is used and it returns a completed future, it could return the value synchronously without dispatching a task (like it would with the load()).

The eager returning of refreshed values would be helpful in a scenario where the cache access is very wide spaces and the refreshAfter covers the tolerable staleness. In this case the refresh result might come very late (worst case double the tolerable time).

I am not sure, would it be an option to use an Executor which executes in the caller thread to implement something like that? (I know I can use expire instead, but I do like the more forgiving non-blocking refresh if I dont know beforehand what type of workload could happen)

In fact, the most flexible feature would be if you can specify a maximum wait time - if the reload Future is completed within this time (or already completed when returned by the asyncReload) then serve its value, otherwise fall back to the existing value and serve it from background (I am aware that this could invite users to implement a too slow asyncReload but then again, they might know what they want from it..).

@ben-manes
Copy link
Owner

ben-manes commented Mar 24, 2022

Currently we don’t inspect the newly created refresh future to see if it is done and can return it’s value. It’s fire-and-forget, returning the already looked up value. It’s likely that this could be changed without much trouble, but I haven’t looked into that. It’s messier in Caffeine since we have AsyncCache. It might also be confusing if a asMap().compute returned a different value than what you calculated due to a refresh overriding that…

The wait and fallback adds a lookup delay on every read during refresh. Since the feature is trying to hide latency by an optimistic reload prior to expiration, that seems a bit counter intuitive. We do expose in-flight refresh futures (see cache.policy().refreshes()) or you can maintain your own map, so that could be done in the calling code. I think being more explicit there would be better.

@ecki
Copy link
Author

ecki commented Mar 25, 2022

Thanks ben for the quick reply

It’s likely that this could be changed without much trouble, but I haven’t looked into that.

Yes I had a look at the source (around here) and it looks like the checking if the future is done would be simply. But it was not obvious to me how to inject the result (especially if transformation is pending)

It might also be confusing if a asMap().compute returned a different value than what you calculated due to a refresh overriding that

It confused me in the refresh case since i ported test code from Guava over. Are you saying the Map-view does the same with the compute() abstraction?

Since the feature is trying to hide latency by an optimistic reload prior to expiration, that seems a bit counter intuitive

Yes thats true, my usecase with relative seldom lookups and very heavy but still synchronous refreshing is quite odd, so I can understand if you dont want to shoehorn it in. (That was the nice part in Guava, the loader can make that decision to some extend). I will close the issue here since it was only a question if I understood the current logic correctly.

@ecki ecki closed this as completed Mar 25, 2022
@ben-manes
Copy link
Owner

ben-manes commented Mar 25, 2022

I think we could incorporate synchronous refreshing into the behavior. It would return the results after the whenComplete action below your snippet is done, which if synchronous is on the calling thread. That would mean that the new value is live for others (so linearizable reads). This would only stomp the return value if from a cache read, e.g. computeIfAbsent. My initial thought was weirdness of returning a different value to a caller than what their mappingFunction produced, but that wouldn't happen due to the cache hit. We'd simply need to be careful about when we stomped on the return value since we call afterRead in multiple scenarios.

ben-manes added a commit that referenced this issue Mar 27, 2022
In Guava if the CacheLoader returns a completed future on a reload then
the new value is returned to the caller and overwritten in the cache.
Otherwise the last read value is returned and the future overwrites
when it is done. This behavior is now supported instead.

A quirk in Guava is when using Cache.get(key, callable). In Guava this
is wrapped as the cache loader and is used if a refresh is triggered.
That causes the supplied CacheLoader's reload to not be called, which
could be more surprising. However, their asMap().computeIfAbsent does
not trigger a refresh on read so the value is not reloaded. As this
behavior is confusing and accidental, Caffeine will always use the
attached CacheLoader and its reload function for refreshing.
@ben-manes
Copy link
Owner

I implemented Guava's behavior in v3's development branch. This differs slightly because in Guava the manual computes may override the CacheLoader (get does, computeIfAbsent does not). That seems accidental and confusing, so in Caffeine the refresh always delegates to the attached CacheLoader's reload method.

ben-manes added a commit that referenced this issue Mar 27, 2022
In Guava if the CacheLoader returns a completed future on a reload then
the new value is returned to the caller and overwritten in the cache.
Otherwise the last read value is returned and the future overwrites
when it is done. This behavior is now supported instead.

A quirk in Guava is when using Cache.get(key, callable). In Guava this
is wrapped as the cache loader and is used if a refresh is triggered.
That causes the supplied CacheLoader's reload to not be called, which
could be more surprising. However, their asMap().computeIfAbsent does
not trigger a refresh on read so the value is not reloaded. As this
behavior is confusing and accidental, Caffeine will always use the
attached CacheLoader and its reload function for refreshing.
ben-manes added a commit that referenced this issue Mar 27, 2022
In Guava if the CacheLoader returns a completed future on a reload then
the new value is returned to the caller and overwritten in the cache.
Otherwise the last read value is returned and the future overwrites
when it is done. This behavior is now supported instead of the more
naive fire-and-forget.

A quirk in Guava is when using Cache.get(key, callable). In Guava this
is wrapped as the cache loader and is used if a refresh is triggered.
That causes the supplied CacheLoader's reload to not be called, which
could be more surprising. However, their asMap().computeIfAbsent does
not trigger a refresh on read so the value is not reloaded. As this
behavior is confusing and appears accidental, Caffeine will always use
the attached CacheLoader and its reload function for refreshing.
ben-manes added a commit that referenced this issue Mar 27, 2022
In Guava if the CacheLoader returns a completed future on a reload then
the new value is returned to the caller and overwritten in the cache.
Otherwise the last read value is returned and the future overwrites
when it is done. This behavior is now supported instead of the more
naive fire-and-forget.

A quirk in Guava is when using Cache.get(key, callable). In Guava this
is wrapped as the cache loader and is used if a refresh is triggered.
That causes the supplied CacheLoader's reload to not be called, which
could be more surprising. However, their asMap().computeIfAbsent does
not trigger a refresh on read so the value is not reloaded. As this
behavior is confusing and appears accidental, Caffeine will always use
the attached CacheLoader and its reload function for refreshing.
ben-manes added a commit that referenced this issue Mar 27, 2022
In Guava if the CacheLoader returns a completed future on a reload then
the new value is returned to the caller and overwritten in the cache.
Otherwise the last read value is returned and the future overwrites
when it is done. This behavior is now supported instead of the more
naive fire-and-forget.

A quirk in Guava is when using Cache.get(key, callable). In Guava this
is wrapped as the cache loader and is used if a refresh is triggered.
That causes the supplied CacheLoader's reload to not be called, which
could be more surprising. However, their asMap().computeIfAbsent does
not trigger a refresh on read so the value is not reloaded. As this
behavior is confusing and appears accidental, Caffeine will always use
the attached CacheLoader and its reload function for refreshing.
ben-manes added a commit that referenced this issue Mar 27, 2022
In Guava if the CacheLoader returns a completed future on a reload then
the new value is returned to the caller and overwritten in the cache.
Otherwise the last read value is returned and the future overwrites
when it is done. This behavior is now supported instead of the more
naive fire-and-forget.

A quirk in Guava is when using Cache.get(key, callable). In Guava this
is wrapped as the cache loader and is used if a refresh is triggered.
That causes the supplied CacheLoader's reload to not be called, which
could be more surprising. However, their asMap().computeIfAbsent does
not trigger a refresh on read so the value is not reloaded. As this
behavior is confusing and appears accidental, Caffeine will always use
the attached CacheLoader and its reload function for refreshing.
ben-manes added a commit that referenced this issue Mar 27, 2022
In Guava if the CacheLoader returns a completed future on a reload then
the new value is returned to the caller and overwritten in the cache.
Otherwise the last read value is returned and the future overwrites
when it is done. This behavior is now supported instead of the more
naive fire-and-forget.

A quirk in Guava is when using Cache.get(key, callable). In Guava this
is wrapped as the cache loader and is used if a refresh is triggered.
That causes the supplied CacheLoader's reload to not be called, which
could be more surprising. However, their asMap().computeIfAbsent does
not trigger a refresh on read so the value is not reloaded. As this
behavior is confusing and appears accidental, Caffeine will always use
the attached CacheLoader and its reload function for refreshing.
@ben-manes
Copy link
Owner

Released in 3.1.0

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

No branches or pull requests

2 participants