• New Feature
  • Status: Closed
  • 2 Major
  • Resolution: Fixed
  • hsingh
  • Reporter:
  • September 21, 2009
  • 0
  • Watchers: 0
  • January 17, 2013
  • April 05, 2010

Description

DESCRIPTION: I’m using the CacheManager#addCache(Ehcache) method to programatically add caches at runtime. The private CacheManager#addCacheNoCheck(Ehcache) throws an ObjectExistsException exception if the cache already exists. To avoid that exception, I need to synchronize my code which creates/adds a cache.

REQUEST: please consider creating an addCacheIfAbsent(Ehcache) method. Clients would not have to synchronize and would still avoid the ObjectExistsException. I was thinking it could work like ConcurrentMap#putIfAbsent() Sourceforge Ticket ID: 2818701 - Opened By: bradcupitlsu - 8 Jul 2009 19:41 UTC

Comments

Sourceforge Tracker 2009-09-21

Here’s some sample code that I use to ensure the ObjectExistsException isn’t thrown:

if (!cacheManager.cacheExists(cacheName)) { synchronized (lock) { if (!cacheManager.cacheExists(cacheName)) { Ehcache ehcache = …; cacheManager.addCache(ehcache); } } }

With the method I’ve proposed, my code would instead be:

if (!cacheManager.cacheExists(cacheName)) { Ehcache ehcache = …; cacheManager.addCacheIfAbsent(ehcache); }

This has the following advantages 1) shorter 2) no thread synchronization in client code 3) more scalable. The implementation of this proposed method could use ConcurrentHashMap#putIfAbsent(), which uses lock striping as opposed to my very granular lock in the first code example. Comment by: bradcupitlsu - 10 Jul 2009 16:22 UTC

Alexander Snaps 2009-12-07

I’ve been spending some time on this and I don’t think it is that easy. Here are my observations :

  • There are 2 maps to maintain in CacheManager, namely caches and ehcaches. Not entirely sure why we need the both, if we could only have one it would be much better… From CacheManager’s point of view I think I can do with one, but these 2 fields have protected visibility :(
  • The replaceCacheWithDecoratedCache doesn’t make it any easier, but again we could get to some locking mechanism for that case
  • At last but not least, with such methods on the CacheManager, I’d expect it to be thread-safe, which currently isn’t the case, so I wonder if that should be fixed on the same release…

We’ve decided to postpone this feature

Alexander Snaps 2010-03-26

CacheManager.replaceCacheWithDecoratedCache(Ehcache, Ehcache): V kind of suffers the same, but silent, issue. Right now, (even with the Maps refactoring) without external locking, you still can have one thread replacing the cache, then another one do the same (again) and have the first one use its replacement that will never be given out by the CacheManager to others. Not really sure how people use cache replacement, so that might be a non issue. Code is still safer now than it was before imho. But this might be something one would want to see enhanced as well… No clue.

Alexander Snaps 2010-04-05

Not quite the CHM.putIfAbsent() contract though due to CacheManager’s null handlings

Himadri Singh 2010-05-10

Following methods have been added

Ehcache net.sf.ehcache.CacheManager.addCacheIfAbsent(Ehcache cache)
Ehcache net.sf.ehcache.CacheManager.addCacheIfAbsent(String cacheName)