Skip to content
This repository has been archived by the owner on Oct 3, 2022. It is now read-only.

JCacheConfiguration ignores eternal flag if configured using ehcache.xml #40

Closed
tarioch opened this issue Jan 27, 2015 · 13 comments
Closed
Assignees
Labels
Milestone

Comments

@tarioch
Copy link

tarioch commented Jan 27, 2015

As far as I'm able to tell, if I configure the cache with an ehcache.xml and have set the cache to be eternal, this is getting ignored as the constructor of JCacheConfiguration always sets up an expiry policy.

@tarioch
Copy link
Author

tarioch commented Jan 27, 2015

Looks like this was introduced when fixing #26

@alexsnaps alexsnaps self-assigned this Jan 27, 2015
@alexsnaps
Copy link
Member

@tarioch You configure the Cache in ehcache.xml, but still create a JCacheConfiguration for it? I must be confused, can you provide a small (even pseudo code, doesn't need to make javac happy) example of how to problem is experienced?

@tarioch
Copy link
Author

tarioch commented Jan 27, 2015

Sure

I have an ehcache.xml and then do

Caching.getCachingProvider().getCacheManager().getCache("foo");

to retrieve the cache.

This triggers JCacheManager.refreshAllCaches() which does

allCaches.put(s, new JCache(this, new JCacheConfiguration(cache.getCacheConfiguration()), cache));

which then ends up in the failing code block.

@alexsnaps
Copy link
Member

Oh ok... so the issue is what the JCacheConfiguration "pretends", but effectively the Cache does honor it, right? I misunderstood that. That being said it still needs fixing! I'll make sure that happens... Thanks!

@alexsnaps alexsnaps added this to the 1.0.2 milestone Jan 27, 2015
@tarioch
Copy link
Author

tarioch commented Jan 27, 2015

The issue I noticed is that, JCache.getValue() starts triggering a remove as it is actually reading this generated JCache config.

@alexsnaps
Copy link
Member

@tarioch I'll try to put a test case together to reproduce this... I seem to see we'll expose the TTI & TTL values, but these may well be set in your config (along with the eternal flag), so that these get picked up, rather than them being 0?

@opoo
Copy link

opoo commented Apr 27, 2015

Line 843-846 in net.sf.ehcache.config.CacheConfiguration (ehcache 2.8.3):

    public final void setEternal(boolean eternal) {
        checkDynamicChange();
        isEternalValueConflictingWithTTIOrTTL(eternal, getTimeToLiveSeconds(), getTimeToIdleSeconds());
        this.eternal = eternal;
        if (eternal) {
            setTimeToIdleSeconds(0);
            setTimeToLiveSeconds(0);
        }
    }

If eternal is true, set timeToIdleSeconds and timeToLiveSeconds to 0.

Line 93-108 in org.ehcache.jcache.JCacheConfiguration (ehcache-jcache 1.0.1):

                expiryPolicy = new ExpiryPolicy() {
                    @Override
                    public Duration getExpiryForCreation() {
                        return new Duration(TimeUnit.SECONDS, cacheConfiguration.getTimeToLiveSeconds());
                    }

                    @Override
                    public Duration getExpiryForAccess() {
                        return new Duration(TimeUnit.SECONDS, cacheConfiguration.getTimeToLiveSeconds());
                    }

                    @Override
                    public Duration getExpiryForUpdate() {
                        return getExpiryForCreation();
                    }
                };

If eternal is true, the value of exipryForCreation and expiryForAccess will be 0 seconds duration.

Line 138-141 in org.ehcache.jcache.JCache (ehcache-jcache 1.0.1):

    private Element getElement(final K key) {
        final Element element = ehcache.get(key);
        if (element == null)
            return null;
        final Duration expiryForUpdate = cfg.getExpiryPolicy().getExpiryForAccess();
        if(expiryForUpdate != null && expiryForUpdate.isZero()) {
            ehcache.removeElement(element);
        }
        return element;
    }

If the expireForAccess is zero, the cached element will be removed when call JCache#get(K key). So, if you configure eternal="true" in ehcache.xml, the cache will be expired immediately when you call get(K key).

Add these codes in org.ehcache.jcache.JCacheConfiguration may be fix it:

        if(cacheConfiguration.isEternal()){
            expiryPolicyFactory = EternalExpiryPolicy.factoryOf();
            expiryPolicy = expiryPolicyFactory.create();
        }else{
            expiryPolicyFactory = null;
            expiryPolicy = new ExpiryPolicy(){
                //...
            }
        }

@jdillon
Copy link
Contributor

jdillon commented Jul 26, 2015

Any reason this has not been fixed? The change is pretty simple, and presently its not possible to configure an eternal cache via ehcache.xml because of this. Unsure about the expiryPolicyFactory = null; in example above, but this seems to resolve the problem for me:

                if (cacheConfiguration.isEternal()) {
                    expiryPolicy = EternalExpiryPolicy.factoryOf().create();
                }
                else {
                    expiryPolicy = new ExpiryPolicy()
                    {
                        @Override
                        public Duration getExpiryForCreation() {
                            return new Duration(TimeUnit.SECONDS, cacheConfiguration.getTimeToLiveSeconds());
                        }

                        @Override
                        public Duration getExpiryForAccess() {
                            return new Duration(TimeUnit.SECONDS, cacheConfiguration.getTimeToLiveSeconds());
                        }

                        @Override
                        public Duration getExpiryForUpdate() {
                            return getExpiryForCreation();
                        }
                    };
                }
                expiryPolicyFactory = new FactoryBuilder.SingletonFactory<ExpiryPolicy>(expiryPolicy);

jdillon added a commit to sonatype/nexus-oss that referenced this issue Jul 26, 2015
jdillon added a commit to sonatype/nexus-oss that referenced this issue Jul 26, 2015
commit 30aa0f1
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 21:38:01 2015 -0700

    Remove shiro-ehcache, its no longer used

commit 355d5b1
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 19:46:52 2015 -0700

    Tidy api

commit e60f740
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 19:44:27 2015 -0700

    Add some more tests, but have to disable due to ehcache/ehcache-jcache#40

commit ce44f7d
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 19:41:13 2015 -0700

    Add commented shiro-activeSessionCache config, with notes why we can not enable this atm due to bug in ehcache-jcache impl

commit baf920b
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 19:36:58 2015 -0700

    Simplify shiro jcache adapter, add some logging

commit 9c7ab27
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 18:53:39 2015 -0700

    Fix exclude

commit de22ee4
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 15:49:17 2015 -0700

    Replace ehcache CacheManager.shutdown() with jcache CacheManager.close()

commit b92b3f3
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 15:18:50 2015 -0700

    notes

commit 85668db
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 14:34:17 2015 -0700

    note to use Externalizable

commit c7f903c
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 14:21:11 2015 -0700

    Move to internal package nothing should be referencing this directly, a bit more tidy.

commit 70e0000
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 14:20:29 2015 -0700

    Make JCache key and values Serializable, likely will want to make these Externalize instead though to optimize

commit 0ccb87b
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 13:57:44 2015 -0700

    tidy

commit ff4aa29
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 13:51:43 2015 -0700

    Minor tidy and text update

commit 27130d3
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 13:39:26 2015 -0700

    tidy sync, no needed atm
    update license headers

commit 263ca70
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 13:30:24 2015 -0700

    failsafe -> default to avoid confusing with ehcache's own failsafe config

commit 044c78e
Author: Jason Dillon <jason@planet57.com>
Date:   Sat Jul 25 13:28:08 2015 -0700

    Bring back ehcache-failsafe.xml configuration
@jdillon
Copy link
Contributor

jdillon commented Jul 27, 2015

Sorry for noise above, sometimes github linking is a bit of PITA

@alexsnaps
Copy link
Member

Sorry, my bad. Remained assigned to me, but I was off on vacation. Back now though.
I'll get back to you. Did you sign the contributor agreement already?
See here: https://github.com/ehcache/ehcache3/wiki#contributor-agreement

@jdillon
Copy link
Contributor

jdillon commented Jul 30, 2015

Ah the project is alive.. yay. Will sent over signed legal mumbo jumbo shortly.

@alexsnaps
Copy link
Member

Cool! Thanks... Now, that being said, the project is slow moving though. I have to be honest with you: all our efforts are going into Ehcache3, this wrapper was a temporary solution for 2.x users. But all (including features that were only "proprietary" ones in the 2.x line, e.g. Offheap) go into the new dev line. Here: https://github.com/ehcache/ehcache3

@jdillon
Copy link
Contributor

jdillon commented Jul 30, 2015

@alexsnaps understood, and we'll consume ehcache3 once it stabilizes, but until then we are moving to use javax.cache api to allow us to switch the backend, so having the adapter for ehcache2 is helping us move forward while ehcache3 becomes more realistic for usage.

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

Successfully merging a pull request may close this issue.

4 participants