Possible difference in behaviour between caffeine and guava for LoadingCache.refresh calls with non-present keys

[slovdahl]

I think that caffeine and guava differs in behaviour when LoadingCache.refresh is invoked for a key that doesn't yet have any cached entry.

Consider the following scenario: an initial invocation of refresh triggers the CacheLoader to start computing the value. While the value is being computed, multiple refreshs are invoked for the same key. Guava notices that there's already a refresh in progress, while caffeine triggers new loads for every refresh invocation. Feel free to correct me if I misunderstood the code. 🙂

We hit this issue in production after migrating from guava. At this point it's a bit unclear to me why we use refresh here instead of get. Might be a misuse of refresh. Mitigating the issue would of course be easy by limiting the executor to one thread as well.

HeavyObject obj = loadingCache.getIfPresent(key);
if (obj != null) {
  return obj;
}
else {
  executor.submit(() -> loadingCache.refresh(key));
  // return object representing computation in progress
}

[ben-manes]

Yes, it's a flaw that I've meant to fix.

Guava has an advantage by forking the hash table and swaps out the underlying entry during the computation. This allows it to serve the current value during the refresh, and the callback restores it to a lighter-weight key/value entry. It is a cute trick that we cannot emulate easily.

The proper solution may be to have a lazily initialized Map<K, CompletableFuture<V>> in the LoadingCache of the in-flight refreshes. This way if one is on-going, it can no-op. However, it is not clear what linearizability expectations users might have with refresh, since a put will stampede it. That could mean that refresh => put => refresh should cancel the first one, update, and finally contain the last refresh's result. We'd need to emulate that in our map by capturing the current value to decide if a new async operation should be triggered or not.

Which all means it got complicated enough that I didn't get to it. Since it is a rarely used feature (vs. refreshAfterWrite) no one seemed to notice until now. 😀

[slovdahl]

Thanks for the informative answer!

Thinking of it further, it doesn't matter if there is a value or not, does it? Doing multiple refresh invocations while a value is being computed triggers equally many loads nevertheless. Wouldn't of course happen in our case, because we don't trigger any refresh if a (stale) value exists.

[ben-manes]

yep, absence and present values are treated similarly by their respective implementations.

[ben-manes]

For now please use get in your executor. If you are returning futures anyway, you might want to switch to AsyncLoadingCache and simplify the logic to its get(key) call. If I'm lucky then I may get to this over the weekend, but may not.

[ben-manes]

Released in 3.0