• Icon: Suggestion Suggestion
    • Resolution: Won't Fix
    • None
    • None
    • Datacenter
    • We collect Confluence feedback from various sources, and we evaluate what we've collected when planning our product roadmap. To understand how this piece of feedback will be reviewed, see our Implementation of New Features Policy.

      NOTE: This suggestion is for Confluence Server. Using Confluence Cloud? See the corresponding suggestion.

      Works fine for non-cluster environment but the cache fails to expire properly when in a cluster environment.

      Place holder for the moment, we will attach more information to reproduce soon.

            [CONFSERVER-35512] Atlassian cache entry not expiring in cluster environment

            Bob Swift added a comment -

            BTW, I think you can close this and we can re-open if necessary later.

            Bob Swift added a comment - BTW, I think you can close this and we can re-open if necessary later.

            Bob Swift added a comment -

            Thanks David. So, no big deal having empty entries out there from your perspective. Refresh period does get updated occasionally, but, you are right, not that often. Still not sure when a cluster system is considered restarted.
            In any case, there are 2 limitations we are trying to work around

            1. tracking the key so that an old entry can be found/cleared before creating the new entry (with different refresh parameters) - Sunita has a solution although it has some drawbacks
            2. no access/control over the stale entry (in some cases it is valuable) - I don't see away around this without managing it ourselves

            Bob Swift added a comment - Thanks David. So, no big deal having empty entries out there from your perspective. Refresh period does get updated occasionally, but, you are right, not that often. Still not sure when a cluster system is considered restarted. In any case, there are 2 limitations we are trying to work around tracking the key so that an old entry can be found/cleared before creating the new entry (with different refresh parameters) - Sunita has a solution although it has some drawbacks no access/control over the stale entry (in some cases it is valuable) - I don't see away around this without managing it ourselves

            David Rizzuto added a comment - - edited

            bob.swift@charter.net, you're correct, the cache objects live in memory until the next restart.

            I see the problem here you guys are having - no runtime updating of cache settings is making atlassian-cache a difficult fit for the cache macro.

            The solution you propose would work, although there might not be need for it. Cache objects themselves are not so heavyweight. If they are holding large data object in them, then that's a problem. But you can mitigate this my nulling out any values left over.

            For example:

            1) User saves a page with a cache macro
            2) If there already exists a cache for this particular instance of the macro, copy the values to a new cache, and null out the values in the old one.

            This will leave cache objects lying around until the next restart, which is not ideal I guess. But how often to people adjust the cache macro?

            We might consider adding a destroy() method to caches in the future, and I believe it's a shortcoming we need to address at some point, but right now I'm not aware of any plans to implement this.

            David Rizzuto added a comment - - edited bob.swift@charter.net , you're correct, the cache objects live in memory until the next restart. I see the problem here you guys are having - no runtime updating of cache settings is making atlassian-cache a difficult fit for the cache macro. The solution you propose would work, although there might not be need for it. Cache objects themselves are not so heavyweight. If they are holding large data object in them, then that's a problem. But you can mitigate this my nulling out any values left over. For example: 1) User saves a page with a cache macro 2) If there already exists a cache for this particular instance of the macro, copy the values to a new cache, and null out the values in the old one. This will leave cache objects lying around until the next restart, which is not ideal I guess. But how often to people adjust the cache macro? We might consider adding a destroy() method to caches in the future, and I believe it's a shortcoming we need to address at some point, but right now I'm not aware of any plans to implement this.

            Bob Swift added a comment -

            So, we are considering alternatives to work around some of the limitations. We would appreciate any thoughts or concerns you have for this alternative:

            Suppose we wrapped Atlassian cache with our own management layer:

            1. Add a Atlassian cache entry with unlimited expiry
            2. We would track the created/updated time in the entry
            3. On a request, we would determine staleness of the entry and either
              • update the entry with the new value (normal case)
              • use the stale entry data in the case where the new value could not be obtained
            4. Provide a scheduled job on some long interval (every day) that would look for our entries in the cache that are really stale entries that would indicate abandoned entries or too old to consider being used even as stale entries using some heuristics on the user requested retention time
              • Perhaps track space usage (for our entries) against a user controlled value for additional cleanup

            Bob Swift added a comment - So, we are considering alternatives to work around some of the limitations. We would appreciate any thoughts or concerns you have for this alternative: Suppose we wrapped Atlassian cache with our own management layer: Add a Atlassian cache entry with unlimited expiry We would track the created/updated time in the entry On a request, we would determine staleness of the entry and either update the entry with the new value (normal case) use the stale entry data in the case where the new value could not be obtained Provide a scheduled job on some long interval (every day) that would look for our entries in the cache that are really stale entries that would indicate abandoned entries or too old to consider being used even as stale entries using some heuristics on the user requested retention time Perhaps track space usage (for our entries) against a user controlled value for additional cleanup

            Bob Swift added a comment -

            I would assume that everything gets removed on a system restart. Correct?
            On a clustered environment, what are the conditions that would be equivalent to a system restart? And it would likely be a rare event unless the shared storage needed to be taken offline.

            Bob Swift added a comment - I would assume that everything gets removed on a system restart. Correct? On a clustered environment, what are the conditions that would be equivalent to a system restart? And it would likely be a rare event unless the shared storage needed to be taken offline.

            Thanks again David!

            I am curious to know, if there is any way to clear the existing unused and hanging around Cache objects ? WIll it be taken care by CacheManager? If so, when will they be cleared?

            Because in Cache macro of Cache Plugin, we are in the process of implementing the Caching using atlassian-cache API . And in Cache macro, a user can change the expiry period any number of times. This would lead to a number of unused Cache objects hanging around at any point of time.

            Sunita Patro {Appfire} added a comment - Thanks again David! I am curious to know, if there is any way to clear the existing unused and hanging around Cache objects ? WIll it be taken care by CacheManager? If so, when will they be cleared? Because in Cache macro of Cache Plugin, we are in the process of implementing the Caching using atlassian-cache API . And in Cache macro, a user can change the expiry period any number of times. This would lead to a number of unused Cache objects hanging around at any point of time.

            David Rizzuto added a comment - - edited

            Hi spatro and bob.swift@charter.net,

            That's correct. The Cache object itself will hang around forever once first requested, with whatever settings you asked for. The expiry (and other related settings) are referring to the cached values held for each key.

            So settings expireAfterAccess = 15 means that if it has been 15 seconds since a cache.get("key") or cache.put("key", <some value>), then the value held for the key "key" will be evicted, and from then on cache.get("key") will be null (until you put something else in, with cache.put("key", <some value>))

            David Rizzuto added a comment - - edited Hi spatro and bob.swift@charter.net , That's correct. The Cache object itself will hang around forever once first requested, with whatever settings you asked for. The expiry (and other related settings) are referring to the cached values held for each key. So settings expireAfterAccess = 15 means that if it has been 15 seconds since a cache.get("key") or cache.put("key", <some value>), then the value held for the key "key" will be evicted, and from then on cache.get("key") will be null (until you put something else in, with cache.put("key", <some value>))

            Bob Swift added a comment -

            Thanks for the discussion on this. It occurred to me from this discussion that it appears that once the expiry period is reached the expired value can not be retrieved. True? This was a nice feature that OSCache was capable of.

            Bob Swift added a comment - Thanks for the discussion on this. It occurred to me from this discussion that it appears that once the expiry period is reached the expired value can not be retrieved. True? This was a nice feature that OSCache was capable of.

            Sunita Patro {Appfire} added a comment - - edited

            Hi David,

            Thanks for rechecking!

            Since the CacheSetting is at Cache object level (not at the level of entries in it) we assumed that after the expiry period, Cache will be null. But it seems like, once expiry period is reached, cache object will still be valid and only "Entries in the Cache" are cleared.

            As you rightly pointed, because of if block in following snippet, Cache was not expiring.

            if (this.atlCacheManager.getManagedCache(cachepluginKey) != null) {
                           this.cache = this.atlCacheManager.getCache(cachepluginKey);
            }
            else{
                          CacheSettings required = new CacheSettingsBuilder().flushable().remote().replicateViaCopy().expireAfterAccess(refreshPeriod, TimeUnit.SECONDS).build();
                         this.cache = this.atlCacheManager.getCache(cachepluginKey, null, required); //store fresh entry
            }

            Sunita Patro {Appfire} added a comment - - edited Hi David, Thanks for rechecking! Since the CacheSetting is at Cache object level (not at the level of entries in it) we assumed that after the expiry period, Cache will be null. But it seems like, once expiry period is reached, cache object will still be valid and only "Entries in the Cache" are cleared. As you rightly pointed, because of if block in following snippet, Cache was not expiring. if ( this .atlCacheManager.getManagedCache(cachepluginKey) != null ) { this .cache = this .atlCacheManager.getCache(cachepluginKey); } else { CacheSettings required = new CacheSettingsBuilder().flushable().remote().replicateViaCopy().expireAfterAccess(refreshPeriod, TimeUnit.SECONDS).build(); this .cache = this .atlCacheManager.getCache(cachepluginKey, null , required); //store fresh entry }

            Hi spatro,

            I tried to reproduce your issue, but no matter what I tried I couldn't. I tried a simple plugin that just did:

                    Cache<String, String> cache = cacheManager.getCache(getName(), null, new CacheSettingsBuilder()
                            .flushable()
                            .remote()
                            .replicateViaCopy()
                            .expireAfterAccess(20000, TimeUnit.MILLISECONDS)
                            .build()
                    );
                    cache.put("mykey", "value");
            
                    Thread.sleep(25000L);
            
                    String fromCache = cache.get("mykey");
            
                    if (fromCache != null)
                    {
                        throw new IllegalArgumentException("cache didn't expire");
                    }
            

            And no matter what values I picked for my cacheName or expiry settings, it never threw an exception.

            I had a look at your sources jar that you attached to the issue and I suspect the problem is that you're calling getCache(String) at the top of your method. This will cause the cache to be created with default settings:

            this.cache = this.atlCacheManager.getCache(cacheKey);
                        
                        if (this.cache != null) {
                            if(this.cache.get(cacheKey) != null) {
                            this.cache.remove(cacheKey);
                            System.out.println("Removing existing cache");
                        }
                            this.atlCacheManager.getManagedCache(cacheKey).clear();
                            System.out.println("currentExpireAfterAccessMillis before put:"+this.atlCacheManager.getManagedCache(cacheKey).currentExpireAfterAccessMillis());
                            this.atlCacheManager.getManagedCache(cacheKey).updateExpireAfterAccess(refreshPeriod, TimeUnit.SECONDS);
                        }
                        else{
                            CacheSettings required = new CacheSettingsBuilder().flushable().remote().replicateViaCopy().expireAfterAccess(refreshPeriod, TimeUnit.SECONDS).build();
                            this.cache = this.atlCacheManager.getCache(cacheKey, null, required); //store fresh entry
                        }
            

            The first line in this snippet is the problem: it will cause the cache to be created with default settings. Calling getCache(String, Loader, Settings) later on will not change that.

            Let me know if this helps.

            David Rizzuto added a comment - Hi spatro , I tried to reproduce your issue, but no matter what I tried I couldn't. I tried a simple plugin that just did: Cache< String , String > cache = cacheManager.getCache(getName(), null , new CacheSettingsBuilder() .flushable() .remote() .replicateViaCopy() .expireAfterAccess(20000, TimeUnit.MILLISECONDS) .build() ); cache.put( "mykey" , "value" ); Thread .sleep(25000L); String fromCache = cache.get( "mykey" ); if (fromCache != null ) { throw new IllegalArgumentException( "cache didn't expire" ); } And no matter what values I picked for my cacheName or expiry settings, it never threw an exception. I had a look at your sources jar that you attached to the issue and I suspect the problem is that you're calling getCache(String) at the top of your method. This will cause the cache to be created with default settings: this .cache = this .atlCacheManager.getCache(cacheKey); if ( this .cache != null ) { if ( this .cache.get(cacheKey) != null ) { this .cache.remove(cacheKey); System .out.println( "Removing existing cache" ); } this .atlCacheManager.getManagedCache(cacheKey).clear(); System .out.println( "currentExpireAfterAccessMillis before put:" + this .atlCacheManager.getManagedCache(cacheKey).currentExpireAfterAccessMillis()); this .atlCacheManager.getManagedCache(cacheKey).updateExpireAfterAccess(refreshPeriod, TimeUnit.SECONDS); } else { CacheSettings required = new CacheSettingsBuilder().flushable().remote().replicateViaCopy().expireAfterAccess(refreshPeriod, TimeUnit.SECONDS).build(); this .cache = this .atlCacheManager.getCache(cacheKey, null , required); //store fresh entry } The first line in this snippet is the problem: it will cause the cache to be created with default settings. Calling getCache(String, Loader, Settings) later on will not change that. Let me know if this helps.

              Unassigned Unassigned
              bob.swift@charter.net Bob Swift
              Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

                Created:
                Updated:
                Resolved: