[infinispan-dev] BdbjeCacheStore and JdbmCacheStore getExpiryTime todos

[Mircea Markus]

On Sep 16, 2009, at 12:03 PM, 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?
>
> For example: TransientCacheEntry.getExpiryTime() could return
> cachevalue.lastUsed + cachevalue.maxIdle. Obviously, this is not fixed
> and it's really a moving target based on when it's used, but  
> wouldn't it
> avoid code like the one above?)
>
> In the case where both max idle and lifespan are used, i.e.
> TransientMortalCacheEntry, getExpiryTime() would return whichever time
> is closer to expiration. So, if lifespan says it should expiry in  
> Xms +
> 1000ms but lastUsed + maxIdle says it should expiry in Xms + 2000ms,
> lifespan calculation would return, and viceversa.
+1. This will remove the code duplication between the two cache  
loaders as well (better design, keep operation where data is).
I think code wise it's only the  
TransientMortalCacheEntry.getExpiryTime that needs to change.
>
> Thoughts?
> -- 
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev