Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CacheResult annotation should allow blocking behavior #373

Open
jerrinot opened this issue Oct 17, 2016 · 9 comments
Open

CacheResult annotation should allow blocking behavior #373

jerrinot opened this issue Oct 17, 2016 · 9 comments
Assignees
Milestone

Comments

@jerrinot
Copy link

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
Copy link
Member

cruftex commented Oct 17, 2016

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
Copy link
Author

jerrinot commented Oct 17, 2016

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

@cruftex
Copy link
Member

cruftex commented Oct 17, 2016

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
Copy link
Author

sounds good to me.

@cruftex cruftex self-assigned this Oct 17, 2016
@cruftex
Copy link
Member

cruftex commented Oct 17, 2016

Thanks, done!

@jerrinot
Copy link
Author

I'm confused what it was re-opened.

@gregrluck
Copy link
Member

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
Copy link
Member

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

@ben-manes
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants