EHC ❯ eternal attribute is confusing
-
Bug
-
Status: Closed
-
1 Critical
-
Resolution: Fixed
-
Documentation
-
-
-
Reporter: asingh
-
June 30, 2010
-
0
-
Watchers: 3
-
August 13, 2012
-
March 07, 2012
Description
“eternal” attribute in cache element is quite confusing wrt tti/ttl. Basic confusion is if user sets both tti/ttl and eternal with conflicting values, its not clear which one will override which value. e.g. tti/ttl = 10 eternal = true Will the cache elements have tti/ttl of or elements will never expire as eternal = true?
Actual behavior right now depends on which attribute comes first in the actual config xml. Setting eternal = true resets values of tti/ttl to 0.
This cache will have ttl of 15 even if eternal = true as ttl comes later in the config.
For:
As the eternal attribute comes later here, it will reset the value of ttl to 0 and the cache will not have any ttl and elements will never expire.
Basically we either need to remove/deprecate the eternal attribute (as tti/ttl values of 0 means eternal=true) Or define when both are specified, one of them always overrides other.
Comments
Nihit Purwar 2010-06-30
Abhishek Singh 2010-06-30
Exactly. Returning Long.MAX_VALUE gives the impression eternal = true and elements will never expire. Also eviction happens, meaning the ttl is actually effective, regardless of eternal=true.
If ttl is declared before eternal=true in the config, the ttl will be reset to 0 and eviction won’t really happen even if ttl=30.
Saravanan Subbiah 2010-06-30
I think we should flag an error if both eternal is set to true and also tti ttl is set to non-zero.
Fiona OShea 2010-06-30
Is this something that should be done right away for Taraval?
Saravanan Subbiah 2010-06-30
Comment from email thread.
I think we should make sure order doesn’t matter in the config and if both eternal is set to true and tti/ttl are > 0 then we flag it as an error instead of silently ignoring tti/ttl.
cheers, Saravanan
On 06/30/2010 01:55 PM, Steve Harris wrote:
Having order matter seems a bit counter intuitive
On Jun 30, 2010, at 1:52 PM, Abhishek Sanoujam wrote:
I don’t quite understand how its mutually exclusive as discussed in jira. The jira talks about what it means when you have both of them like e.g.: eternal=true and tti/ttl=10 (any positive integer) Setting eternal=true resets the tti/ttl, and the actual behavior right now depends on which one is declared first in the config (as we use a SAX based parser)
On 6/30/10 11:14 PM, Geert Bevin wrote:
One of the reasons this was introduced was to save one field in the elements when they’re stored in the cluster. Instead of using a dedicated eternal attribute, the TTI and TTL flags are used to store this. I don’t think it’s confusing actually since they’re mutually exclusive properties.
On 30 Jun 2010, at 18:25, Steve Harris wrote:
I think so, what do you guys think? We could save it for ehcache 2.2.1 if we don’t know exactly what to do yet?
Should we fix this ? Its confusing now.
Cheers, Saravanan
gluck 2010-07-27
Agree with Saro. If set it should override. Also throw an error on startup if there is a conflict. eternal is true should not be deprecated.
Fiona OShea 2010-07-27
Fix in branch 3.3 and trunk if tc core
gluck 2010-08-01
Gary Keim raised a good point. There will be people out there with different configs. Why break them unnecessarily?
Indeed a whole bunch of unit tests broke when this change was made. That should have reminded us, as our own unit tests are good examples of config that has been developed over time.
Let’s change this to:
- eternal if set overrides TTI and TTL
- If eternal is set, and TTI/TTL is set to non-zero values, then emit a warning as follows: Cache name_of_cache is set to eternal but also has TTI/TTL set. The TTI/TTL setting will be ignored and the cache will be set to eternal.”. Do not throw an exception.
The upshot of this is that the configs will be gradually cleaned up over time.
Saravanan Subbiah 2010-08-02
Allowing both eternal and tti/ttl setting is still confusing. Users have to read the doc to know which one precedes which. And ppl don’t look at logs unless there is a problem so silently ignoring some setting might make the cache work in unintended ways. Personally I feel failing fast is always a better approach. But if everyone is having a strong opinion about breaking compatibility with exiting config, then I am ok with the change.
Abhishek Singh 2010-08-02
Existing behavior currently is that if eternal=true and tti/ttl is also set, and in the config the tti/ttl comes later in the config, the cache honors the tti/ttl. Making eternal override TTI/TTL will also break backwards incompatibility (in more subtle manner than the exception actually until user finds and reads doc that eternal now overrides tti/ttl).
For now, printing a warn log (at console? instead of log files) and not changing any behavior might be better. Throwing exception/config overhaul (schema validation etc) may come in next major release.
Will revert code to not throw exception on conflicting config values for now.
Abhishek Singh 2010-08-09
Cannot get this fixed in 3.3.1 Will have to wait till next major release to fail-fast with exception. Logging warn level msgs for now.
Fiona OShea 2011-01-21
Adding this to the doc required list. Not sure what needs to be docc’d but better safe and add it
ilevy 2011-02-17
Removed ‘doc required’ flag. Source 3.5.0-RC product docs now state that eternal=”true” in a cache overrides any TTI/TTL settings.
Fiona OShea 2011-04-19
From Manoj. The fix is
When both TTI/TTL and eternal attribute are set, eternal attribute takes the higher precedence. Also, we get a new warning message like below. The fix is to have this higher precedence and the warning message. We decided not to throw error message as many other places might break.
LOG.warn("Cache '" + getName() + "' is set to eternal but also has TTI/TTL set. "
+ " To avoid this warning, clean up the config " + "removing conflicting values of eternal,"
+ " TTI and TTL. Effective configuration for Cache '" + getName() + "' will be eternal='" + newEternalValue
+ "', timeToIdleSeconds='0', timeToLiveSeconds='0'.");
Gary Keim 2011-10-19
If we can’t remove eternal, can we at least not make it required?
Saravanan Subbiah 2011-10-19
Never thought it was required. It should be optional.
Gary Keim 2011-10-19
<xs:attribute name="eternal" type="xs:boolean" use="required"/>
Abhishek Singh 2011-10-20
eternal attribute is now optional and has default false value.
Assigning to Igal for doc.
ilevy 2012-03-07
this was dealt with a while back in the ehcache faq and in http://ehcache.org/documentation/configuration/data-life
element.getExpirationTime() seems to be broken as well.
In the test ServerMapTTLExpressTest, if you print element.getExpirationTime it will give u Long.MAX_VALUE But inline eviction happens if u sleep and then try to access this element.