Jens Wilke opened SPR-16503 and commented

The cache abstraction currently operates the cache in a cache aside pattern. Operating the cache via a cache loader would make it possible to use advanced caching features, such as refresh ahead or resilience.

Right now, it is possible to "hack up" a CacheResolver that creates a loading cache, in case there is only a single method annotated with Cachable and it only has a single argument.

IMHO, the current cache abstraction would provide enough to be able to create a loading cache, however, the current Spring internals don't make it possible.

Needed:

  • A concept of a StaticCacheResolver that is only run once at startup and gets all needed information
  • A reverse mapping for KeyGenerator to be able to call the loading method on behalf of the cache key

Thoughts?

No further details from SPR-16503

Comment From: spring-projects-issues

Stéphane Nicoll commented

I am not keen to add any complexity to the abstraction because any feature we introduce at that level forces every implementation to fulfill the contract. While most cache implementations support read through/look ahead, I don't see (yet) the benefit of adding that in the abstraction.

I lost you with the StathcCacheResolver and the reverse mapping. Perhaps you could share an example that demonstrates what you're after?

Comment From: spring-projects-issues

Jens Wilke commented

Benefits for a loading cache:

Usually the loading cache provides a blocking mechanism that ensures that the data source is only called once in case the value is missing. A Spring cache implementation uses not the typical cache aside pattern with Cache.get and Cache.put but a cache operation that works atomically e.g. Cache.putIfAbsent, to provide a blocking mechanism as well.

For cache2k the code looks like:

public <T> T get(final Object key, final Callable<T> valueLoader) {
     if (loaderPresent) {
          return (T) cache.get(key);
     } else {
          try {
               return (T) cache.computeIfAbsent(key, (Callable<Object>) valueLoader);
          } catch (CacheLoaderException ex) {
               throw new ValueRetrievalException(key, valueLoader, ex.getCause());
          } catch (RuntimeException ex) {
               throw new ValueRetrievalException(key, valueLoader, ex);
          }
     }
}

The usage of the cache.computeIfAbsent via the Callable has the disadvantage that there needs to be one additional object created for each cache access, because it is not known in advance whether it is a cache hit or miss.

Blocking is considered essential. In the Spring documentation this can be found (Javadoc of Cache.get):

If possible, implementations should ensure that the loading operation is synchronized so that the specified valueLoader is only called once in case of concurrent access on the same key.

Let's consider a high traffic web site that serves 5000 requests per second and a data source that requires 5 seconds to load the data. In case the case is not blocking, then 25000 requests would hit the data source. With the blocking cache only one request hits the data source.

But, to make the high traffic web site work, the blocking cache is not "good enough". The problem is, that there is still a 5 second service disruption while the value is (re)loaded. To sustain the "hickup" the system would need 25000 threads available, that all block until the value is loaded. Depending on configuration a bunch of other system resources will be occupied by the stuck requests as well. The cache feature refresh ahead makes sure that data is always available, since the refresh is done in background when the value expires. Refresh ahead requires that the cache is in read through configuration.

Operating in read through configuration opens the possibilities for other useful features as well. One feature that I would like to see is an automatic cache warm up during application start.

Comment From: spring-projects-issues

Jens Wilke commented

I lost you with the StaticCacheResolver and the reverse mapping. Perhaps you could share an example that demonstrates what you're after?

I have put together a spring boot example, you can find here: https://github.com/cruftex/spring-boot-cache2k-resolver-experiment

Here is a "hack" implementation that constructs a loading cache via the CacheResolver interface: https://github.com/cruftex/spring-boot-cache2k-resolver-experiment/blob/master/src/main/java/sample/cache/ExperimentalLoadingCache2kCacheResolver.java

However, if there are different cache annotations used and the first interaction with the cache is not via a @cachable annotated method, the cache resolver does not know about the method that could populate the cache. The idea of the StaticCacheResolver is that the cache is only resolved once per application run and the resolver gets information about all methods that are relevant.

Alternatively the loader could be wired later as soon as a @cachable method is called. I don't like that, since all information is available.

Personally, I would prefer that all cache instances are created at startup, so any configuration mistake will show up instantly. This makes also a feature like automatic cache warm up possible.

Reverse Mapping

Spring calls the cache with (simplified): Cache.get(keyGenerator.generate(methodParameters)). The key generator produces a single object from multiple method parameters. Inside the cache loader the method parameters need to be extracted from the cache key again to invoke the method. Example:

Object load(Object key) { 
  return method.invoke(target, keyGenerator.extract(key));
}

Comment From: spring-projects-issues

Juergen Hoeller commented

So you'd like to see a mechanism that assembles all the cache keys from the @Cacheable declarations and passes them to the cache implementation for specific warming-up on startup? That would indeed a require a mechanism we don't quite have yet.

As for blocking, we're only calling Cache.get(key, valueloader) in case of @Cacheable(sync=true) which users need to explicitly opt into. Otherwise we do use the typical cache interaction pattern of separate Cache.get and Cache.put calls.

Comment From: spring-projects-issues

Jens Wilke commented

So you'd like to see a mechanism that assembles all the cache keys from the @Cacheable declarations and passes them to the cache implementation for specific warming-up on startup?

The warm up idea was just a side note. Probably it needs more explanation. If the cache is only in-process it looses all information by a JVM restart. This is no problem, since everything can be regenerated. However, the application will be sluggish until the cache(s) have all the relevant content again. If the cache is created at startup and wired together with the method that provides the cached values (technically that is the cache loader delegating to the @cacheable method), it can start to populate itself with relevant content. The relevant content could be determined by a set of cache keys that were saved from the previous application run.

However, I see the main "selling point" for a loading cache to be able to use refresh ahead. The refresh ahead feature will reduce resource spikes in the system and it will smooth out response rates.

As for blocking, we're only calling Cache.get(key, valueloader) in case of @Cacheable(sync=true) which users need to explicitly opt into. Otherwise we do use the typical cache interaction pattern of separate Cache.get and Cache.put calls.

Thanks for the clarification. I am not a fan of the extra parameter sync=true, since I don't know of a single usage where you do not want the blocking behavior. In terms of semantic, sync=true does not give any "hard" guarantees, so its actually a tuning option which I think should not be at this fine level in the code. Probably the default is false for historic/compatibility reasons. What about a global switch at @EnableCaching? Should I open another ticket for this?

What would be the reasons for not using sync=true? Only avoid the additional overhead of the Callable?

Comment From: snicoll

There has been a number of requests in that direction and we don't think this is the right fit for the cache abstraction. There is a recent discussion in https://github.com/spring-projects/spring-framework/pull/30124 that elaborates on our thinking. Thanks anyway for the suggestion.