ensuring a failed async mapping is cleared before retrying

[benjaminp]

A pattern I keep running into with AsyncCache and AsyncLoadingCache is that tests that create a failed mapping and then want to immediately check that the operation is retriable. This makes the tests racy because failed mapping removal is asynchronous to completion of the CompletableFuture returned by AsyncCache.get.

To be concrete, a program like this:

import com.github.benmanes.caffeine.cache.Caffeine;

void main() throws Exception {
  var c = Caffeine.newBuilder().<Integer, Integer>buildAsync();
  for (var i = 0; i < 10_000; i++) {
    c.asMap().remove(42);
    try {
      c.get(
              42,
              k -> {
                throw new RuntimeException("failed");
              })
          .get();
    } catch (ExecutionException ee) {
      assert ee.getCause().getMessage().equals("failed");
    }
    c.get(42, k -> 19).get();
  }
  IO.println("success");
}

will probably never complete success and instead fail with an exception from the c.get(42, k -> 19).get() line in one of the loop iterations.

Are there any recommended patterns for this?

[ben-manes]

CompletableFuture uses a stack of dependent actions so your own action is run prior to Caffeine receiving a callback. We have to return the computed future, not the dependent action's, for the asMap() view (reference identity, e.g. replace) and since methods like cancel() do not communicate. That means we really can do anything in the implementation.

You could perform an explicit remove in your own callback, e.g. cache.asMap().remove(key, future), prior to a retry. Or you could return a future that already handles the retry internally (e.g. failsafe example). I usually use .executor(Runnable::run) in tests to disable asynchronous processing unless I am testing for concurrent executions.

[benjaminp]

CompletableFuture uses a stack of dependent actions so your own action is run prior to Caffeine receiving a callback. We have to return the computed future, not the dependent action's, for the asMap() view (reference identity, e.g. replace) and since methods like cancel() do not communicate. That means we really can do anything in the implementation.

Was "can" supposed to be "can't"?

You could perform an explicit remove in your own callback, e.g. cache.asMap().remove(key, future), prior to a retry.

seems reasonable, thanks

Or you could return a future that already handles the retry internally (e.g. failsafe example). I usually use .executor(Runnable::run) in tests to disable asynchronous processing unless I am testing for concurrent executions.

Is .executor(Runnable::run) sufficient? handleCompletion uses CompletableFuture.whenComplete, so in theory the callback could be posted to the the common FJP. I suppose everything could be work out if the asyncLoad callback religiously uses its executor parameter. (Sometimes I want the cache internally executor to be different than the one used by mapping functions, though.)

[ben-manes]

whenComplete runs on the completing thread or caller if done. If a direct executor is set then the future created by CacheLoader uses that executor. The calls become immediate which is nice for code not testing asynchronous behavior.
(Yes, earlier typo)