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

Use of Cache2k Cache Loaders with JSR 107 #103

Closed
frederikb opened this Issue Mar 7, 2019 · 7 comments

Comments

Projects
None yet
2 participants
@frederikb
Copy link

frederikb commented Mar 7, 2019

Hi, thanks for your well appreciated work on Cache2k.

We're evaluating Cache2k and have stumbled upon the following roadblock and are wondering if it's a bug or just an unsupported or undesired use case.

Scenario:

  • Fully define the entire cache configuration in cache2k.xml including a loader (via <loader><bean>...)
  • Access the cache via javax.cache.CacheManager#getCache

Expectation:

A call to javax.cache.Cache#get is forwarded to org.cache2k.Cache#get which in turn triggers uses the configured cache loader to perform a read through operation.

Observed behavior:

The cache returns null and the cache loader is not triggered.

This seems to be due to the JCacheAdapter#get call forwarding the get call to the peek operation of Cache2k due to the values loaderConfigured and readThrough being false. These values are determined by the JCacheBuilder based upon the JCache configuration which in our use case is always empty and therefore defaults to false.

See JCacheBuilder#buildAdapterCache

Question:

Is this working as designed? Could we instead make it possible for the Cache2k to be leading?

@cruftex

This comment has been minimized.

Copy link
Member

cruftex commented Mar 7, 2019

Interesting. Actually, its working mostly as designed. But the behavior isn't useful and unexpected.

JSR107 / JCache requires that readThrough is enabled in case the loader should be used to populate missing entries on Cache.get. Otherwise the loader is only used for explicit load requests via Cache.loadAll. That is somewhat unexpected. The behavior of cache2k is different. In case a loader is defined, the cache will operate in read through mode. Since cache2k also has the Cache.peek operation you can still skip the loading and just examine the cache contents. I think that is more flexible and the behavior is more according to expectations.

What is missing, is that readThrough can be enabled via the XML configuration. I have added enableStatistics and enableManagement for that reason, but missed your use case. So TODO: Add readThrough and writeThrough to the JCacheConfiguration.

loaderConfigured being false might be simply a bug because no one tried to do this before.

I think the problem can be resolved quite quickly.

@frederikb

This comment has been minimized.

Copy link
Author

frederikb commented Mar 7, 2019

Hi Jens, thank you for your quick reply. Some comments:

loaderConfigured is false due to JCacheBuilder checking against javax.cache.Cache#getCacheLoaderFactory which is of course null in our use case.

In addition it is also setting storeByValue based upon the JCache configuration, whereas we have set storeByReference for Cache2k in our cache2k.xml.

private void buildAdapterCache() {
    createdCache =
      new JCacheAdapter<K, V>(
        manager,
        new InternalCache2kBuilder<K,V>(cache2kConfiguration, manager.getCache2kManager()).buildAsIs(),
        keyType.getType(), valueType.getType(),
        config.isStoreByValue(), config.isReadThrough(), config.getCacheLoaderFactory() != null,
        eventHandling
      );
  }
@cruftex

This comment has been minimized.

Copy link
Member

cruftex commented Mar 7, 2019

In addition it is also setting storeByValue based upon the JCache configuration, whereas we have set storeByReference for Cache2k in our cache2k.xml.

storeByValue is ignored when the cache2k XML configuration is present, unless you enable it with copyAlwaysIfRequested

@cruftex

This comment has been minimized.

Copy link
Member

cruftex commented Mar 7, 2019

Just committed a a fix for this problem. I have currently some more stuff in the works at HEAD, if needed I can backport the change for the last release.

@frederikb

This comment has been minimized.

Copy link
Author

frederikb commented Mar 7, 2019

Incredible speed seems to be reflected in both the product and your reaction time. We would greatly appreciate if you could backport it and release a 1.2.1 version to Maven Central.

@cruftex cruftex added this to the v1.2.1 milestone Mar 8, 2019

@cruftex

This comment has been minimized.

Copy link
Member

cruftex commented Mar 8, 2019

Just released in 1.2.1, should show up on maven central in a bit.

@cruftex cruftex closed this Mar 8, 2019

@cruftex

This comment has been minimized.

Copy link
Member

cruftex commented Mar 8, 2019

Incredible speed seems to be reflected in both the product and your reaction time.

Thanks, you are welcome. Just open another issue, if you have any more issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.