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

Interactions between declarative API and CacheLoader&CacheWriter? #375

Open
jerrinot opened this issue Oct 24, 2016 · 5 comments
Open

Interactions between declarative API and CacheLoader&CacheWriter? #375

jerrinot opened this issue Oct 24, 2016 · 5 comments
Milestone

Comments

@jerrinot
Copy link

jerrinot commented Oct 24, 2016

When a declarative API (cache annotations) is used then a container generates a cache key.

When a cache is configured as read-through or write-through then the generated key is passed to a CacheLoader/CacheWriter.

However there is a mismatch: The key is generated by a container however the CacheLoader/CacheWriter are typically implemented by end-users -> the CacheLoader has no good way to access arguments used during invocation of the annotated method. All it has is just the generated key.

A possible workaround is to always always use an explicit CacheKeyGenerator and never use the default one -> a user has a full control of the generated cache key and can get back the arguments in the MapLoader code. Unfortunately this is tedious as CacheKeyGenerator is a very low-level API and it renders the default generator useless.

I'm not sure what's the best solution here:

  1. Perhaps the spec should mandate a concrete implementation of the default (composed?) key.
    The implementation could have an extra method to get back the arguments encapsulated by this key? However this approach goes against the idea of generated key relaxation.
  2. Alternatively the spec could provides a bunch of static method for introspection of the generated cache keys?
  3. Anything else?

Related to #313

@cruftex
Copy link
Member

cruftex commented Oct 24, 2016

the CacheLoader has no good way to access arguments used during invocation of the annotated method. All it has is just the generated key.

Why should it? That is actually the very concept of a cache key and the CacheLoader/CacheWriter. If something else then the cache key influences the behavior of the loader, your cache would be inconsistent. Sometimes people try it anyways: http://stackoverflow.com/questions/37944398/how-to-pass-two-pass-one-more-parameter-other-than-key-in-google-guava-cache

IMHO the JavaDoc of the CacheLoader should have much more words of warning about this.

Of course the loader might want to get an additional setup that is immutable during the cache live time. But "passing down" such a setup by method parameters would be very error prone.

Is there really a useful scenario for CacheLoader/CacheWriter being used together with annotations? I think its two different concepts to wire in the cache.

@jerrinot
Copy link
Author

@CacheResult
Person calculateArchenemy(Person person) {
  if (person.equals(Person.HOLMES)) {
    return Person.MORIARTY;
  } else {
    return expensiveCalculation(person.getId());
  }
}

Now imagine I have a database dump of archenemies of all people in the world. Then I could just plug the CacheLoader in and never ever need to invoke the expensiveCalculation(). Except I can't because the cache will pass me the GeneratedCacheKey and I have no way to access the person.getId()

@cruftex
Copy link
Member

cruftex commented Oct 24, 2016

Got you. The main issue is the GeneratedCacheKey. Since the way the key is generated is not part of the spec it could be anything. Don't we have another issue on that? I think that problem is actually not only related to CacheLoader/Writer. For example one might also additionally use the bulk API Cache.getAll() which annotations don't have.

Mixing Loader/Writer and annotation will get you also in other troubles as well: Annotations have (defined) exception handling, while loaders don't.

@jerrinot
Copy link
Author

jerrinot commented Oct 24, 2016

this is related: #374 <-- if this is going to be implemented then the key could be truly everything.

I also found this and this is partially related as well. The original idea behind this issue comes from ben-manes/caffeine#115

@cruftex cruftex added this to the 2.0 milestone Oct 7, 2017
@cruftex
Copy link
Member

cruftex commented Oct 7, 2017

Marking with 2.0. The key generation stuff can be improved but not in the 1.1 MR.

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

No branches or pull requests

2 participants