2

I'm trying to use caffeine cache and have a problem:

Let's say cache is empty and I query for a value, it uses loader and loads a new value into the cache, after 2 days have passed I query for the same value and I get OLD value first then refresh is initiated on a separate thread and the new value is loaded if loading is possible.

Caffeine.newBuilder()
        .refreshAfterWrite(5, TimeUnit.MINUTES)
        .expireAfterWrite(3, TimeUnit.DAYS)
        .build(loader);

What I want to archive is - try to refresh first and return the new value first if possible, if something goes wrong only then return the old one. How can I archive this? How to implement simple, future proof, neat implementation, without workarounds? That would be awesome!

Edit: Would this solution work properly?

boolean needsSyncRefresh = cache.policy().expireAfterWrite()
                .flatMap(stringBigDecimalExpiration -> stringBigDecimalExpiration.ageOf(key))
                .filter(age -> age.toMinutes() < 0 || age.toMinutes() > REFRESH_AFTER_MINUTES)
                .isPresent();

V value = cache.get(key);

if (needsSyncRefresh) {
    return cache.asMap().computeIfPresent(key, (k, oldValue) -> {
        if (oldValue.equals(value)) {
            try {
                return loader(key);
            } catch (Exception e) {
                //handle error
            }
        }
        return oldValue;
    });
}
return value;

1 Answers1

4

I don't think using CacheWriter is necessary here. Instead inspect the metadata to determine if a refresh is needed.

The simplest might be to run a scheduled task that triggers a refresh early. For example,

var policy = cache.policy().expireAfterWrite().get();
for (var key : cache.asMap().keySet()) {
  boolean refresh = policy.ageOf(key)
      .filter(age -> age.toDays() >= 2)
      .isPresent();
  if (refresh) {
    cache.refresh(key);
  }
}

A slightly more complicated version does this at retrieval by checking if the retrieved value is stale and performing a new computation, e.g.

V value = cache.get(key, this::loadValue);

var policy = cache.policy().expireAfterWrite().get();
boolean refresh = policy.ageOf(key)
    .filter(age -> age.toDays() >= 2)
    .isAbsent();
if (!refresh) {
  return value;
}
return cache.asMap().compute((key, oldValue) -> {
  if ((oldValue != null) && (oldValue != value)) {
    return oldValue; // reloaded by another thread
  }
  try {
    return loadValue(key);
  } catch (Exception e) {
    logger.error("Failed to load {}", key, e);
    return oldValue;
  }
});
Ben Manes
  • 9,178
  • 3
  • 35
  • 39
  • The first solution will not work if I will not query the cache anything for two days. In my case, it would be the weekend. I would get the old value if I would ask old value first after two days of pause because refresh is still async. – Vytautas Šerėnas Mar 31 '20 at 11:22
  • The first probably would work to keep it refreshed, but then disable the expiration. So unfortunately its not a great idea on my part. I think the second should work though. – Ben Manes Mar 31 '20 at 18:58
  • I edited my question with a possible solution, I interpreted your suggestion and added cache.put(k,v) to it instead of double-take when the thread in parallel also does another call into the loader for the same key. For some reason, I get a negative value of age in my case, but it works anyway, I have tested it. I would be thankful if you could review my code of any flaws I have left and I will accept your answer. – Vytautas Šerėnas Mar 31 '20 at 20:17
  • Your solution suffers from a cache stampede where multiple threads try to reload at once. This is why I used a computation, which performs the work atomically. If you fix that then it seems fine. – Ben Manes Mar 31 '20 at 22:20
  • I have updated it, now not much different from yours suggestion. A bit afraid if smth can go wrong with threads, because it's tricky to test it, but it seems like it works and should be good to go. – Vytautas Šerėnas Apr 01 '20 at 14:25
  • @VytautasŠerėnas I used compute because there is a race where the entry is evicted between calls. You only recognize these edge cases by being burned far too many times at little things like this... – Ben Manes Apr 01 '20 at 16:03