search

jsr107 / jsr107spec

JSR107 Cache Specification
Apache License 2.0
413 stars 164 forks source link

CacheResult annotation should allow blocking behavior #373

Open jerrinot opened 7 years ago

jerrinot commented 7 years ago

When a cache key is invalidated or expires then multiple concurrent invocations of a method annotated with @CacheResult may get a cache miss. This can trigger multiple potentially expensive computations running in parallel - MISS storm.

The CacheResult annotation should be configurable to allow only a single thread to invoke the annotated method. All other threads should be blocked until the 1st thread is done and then they should recheck the cache.

This is the idea:

public @interface CacheResult {
[...]
  boolean blocking() default false;
[...]

alternative name: boolean atomic() default false;

cruftex commented 7 years ago

Yes, see Bens comment here: https://rmannibucau.wordpress.com/2015/08/21/cacheresult-jcache-cdi-to-the-rescue-of-microservices/

The problem is that JCache does not cover the fact of blocking at all. It is not mandatory in the imperative API and no established concept. "atomicity" maybe, however, whether atomicity leads to blocking, would be implementation dependent.

However, nothing I think we can do in a maintenance release.

jerrinot commented 7 years ago

yes, this is definitely JCache 2.0 material. Is there a better place to collect ideas for JCache 2.0 ?

cruftex commented 7 years ago

The issue is very relevant for using annotations in high throughput scenarios.

I know the problem, but I didn't dare for bringing it up here, since there is other fish to fry. I have a couple of things like this on my private list, but I didn't want to start discussion on this, since the work needs to be focused and time is limited. However, it is good to see that others come to the same conclusion, and it is not only me raising new issues :)

Procedure is to label with "JCache next" and close the ticket, to keep the overview what we need to address for the next MR. This is something the Spec leads do not need to bother with currently, so I would do so, too. Although it is closed, the topic is not lost. Okay?

jerrinot commented 7 years ago

sounds good to me.

cruftex commented 7 years ago

Thanks, done!

jerrinot commented 7 years ago

I'm confused what it was re-opened.

gregrluck commented 7 years ago

I am happy to collect JCache 2.0 stuff here. I added a milestone. I will then transfer these over to JCache 2.0 once it starts.

gregrluck commented 7 years ago

Reopened it because I am collecting JCache 2.0 feature requests and issues now.

ben-manes commented 7 years ago

Note that users have tried to work around this using a CacheLoader, but it gets messy due to CacheKeyGenerator. It also means that the method is merely a stub, which defeats the value imho. I think this was what @gregrluck was trying to convey as his desired solution when we discussed the feature in the RI. Unfortunately while a good idea, the result isn't very usable.

Spring uses sync = true, defaulted to false, in its Cacheable annotation.