EHC ❯ Storing timestamps at one second granularity causes problems for caches with a large fraction of Elements of a very similar age (the same to within a second).
-
Bug
-
Status: Closed
-
2 Major
-
Resolution: Fixed
-
ehcache-core
-
-
hsingh
-
Reporter: cdennis
-
February 04, 2010
-
0
-
Watchers: 1
-
January 17, 2013
-
February 09, 2010
Description
Currently Element stores timestamps as integer seconds in order to save memory. This causes problems for caches with a large fraction of their population all the same age (to within a second). It make the statistical evictor appear to make mistakes, because its samples can end up all having the same timestamps, and so it cannot order them correctly, and effectively becomes a random evictor.
Comments
Chris Dennis 2010-02-04
gluck 2010-02-04
The test that reproduces it is:
LRUMemoryStoreTest. testProbabilisticEvictionPolicy
When the precision was changed from ms to seconds this test broke. The test was then “fixed” by adding a sleep, which is clearly contrived. I have changed the test back to its original state as I wrote it and added an @Ignore. When we fix the code lets unignore the test.
/** * Test the LRU policy */ @Ignore @Test public void testProbabilisticEvictionPolicy() throws Exception { createMemoryOnlyStore(MemoryStoreEvictionPolicy.LRU, 500);
//Make sure that the store is empty to start with
assertEquals(0, cache.getSize());
// Populate the store till the max limit
Element element = new Element("key1", "value1");
for (int i = 0; i < 500; i++) {
cache.put(new Element("" + i, "value1"));
}
//Let the last used timestamps be effective for the evictor
Thread.sleep(3010);
//Now read to update the put count
for (int i = 0; i < 500; i++) {
cache.get("" + i);
}
//Let the last used timestamps be effective for the evictor
//Note: This should not be necessary. It became necessary in 1.7 because the timestamp precision was changed to 1 second // Thread.sleep(3010);
//Add some fresher content
for (int i = 501; i < 750; i++) {
cache.put(new Element("" + i, "value1"));
}
//The fresh ones should be kept.
int lastPutCount = 0;
for (int i = 501; i < 750; i++) {
if (cache.get("" + i) != null) {
lastPutCount++;
}
}
LOG.info("Last Put count: " + lastPutCount);
assertTrue("Ineffective eviction algorithm. Less than 230 of the last 249 put Elements remain: " + lastPutCount, lastPutCount >= 245);
}
gluck 2010-02-04
Suggestion for possible fix:
Can we have it so that Element as used by Terracotta CDM is different to that used by standalone. The Terracotta one is to the second as an eviction is to the L2 and therefore not harmful and the standalone is to the ms so that the evictor is more accurate.
Chris Dennis 2010-02-09
I’ve moved the ElementEvictionData interface to use long milliseconds, and modified the standalone Ehcache implementation to use millisecond precision.
Himadri Singh 2010-02-24
Verified in rev 1933 w/ LruMemoryStoreTest testProbabilisticEvictionPolicy
T E S T S ——————————————————- Running net.sf.ehcache.store.LruMemoryStoreTest Feb 24, 2010 3:44:29 AM net.sf.ehcache.store.LruMemoryStoreTest testProbabilisticEvictionPolicy INFO: Last Put count: 249
Attaching original user reported issue.