[infinispan-dev] BdbjeCacheStore and JdbmCacheStore getExpiryTime todos

[Galder Zamarreno]

On 09/23/2009 07:45 PM, Manik Surtani wrote:
>
> On 16 Sep 2009, at 10:03, Galder Zamarreno wrote:
>
>> Hi,
>>
>> I'm looking at the TODOs in the Infinispan code base and I've seen
>> BdbjeCacheStore and JdbmCacheStore doing the following:
>>
>>          long expiry = entry.getExpiryTime();
>>          if (entry.getMaxIdle()>  0) {
>>              // TODO do we need both?
>>              expiry = entry.getMaxIdle() + System.currentTimeMillis();
>>          }
>>          Long at = new Long(expiry);
>>          Object key = entry.getKey();
>>          expiryMap.put(at, key);
>>
>> They use this expiry time as key in a expiryMap that they purge when
>> they need to expire entries from the cache store:
>>
>>      protected void purgeInternal() throws CacheLoaderException {
>>          try {
>>              Map<Long, Object>  expired =
>> expiryMap.tailMap(System.currentTimeMillis(), true);
>>              for (Map.Entry<Long, Object>  entry : expired.entrySet()) {
>>                  expiryMap.remove(entry.getKey());
>>                  cacheMap.remove(entry.getValue());
>>              }
>>          } catch (RuntimeException caught) {
>>              throw convertToCacheLoaderException("error purging expired
>> entries", caught);
>>          }
>>      }
>>
>> InternalCacheEntry.getExpiryTime() says it has no meaning for entries
>> with max idle but without lifespan. Couldn't this be extended to
>> support
>> max idle too?
>
> Well that would be tough since as you say it would be a moving
> target.  And as a moving target, it would require care in how the
> value is used.  E.g.,
>
> long expiry = e.getExpiryTime();
>
> while (expiry<  System.currentTimeMillis()) {
>     // do some stuff
> }
>
> // remove entry
>
> Would lead to incorrect behaviour if some other thread updates the
> entry.

Hmmm, yeah that sounds rather risky.

I'll leave the cache store code as it is making a note on the reasons 
for both calls.

>
> In the case of a cache loader this makes sense since an entry would
> only be loaded if it isn't in memory (and hence no one else is using
> it).

-- 
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache