You're welcome. I've been told by few other people that it was too dry and condensed. I'll try to throw in few jokes next time I'm writing something like this :D
On Fri, 2009-10-30 at 12:24 +0000, Luke Biddell wrote: > Thanks for the reply Jan. Took me several reads, think I understand > now. I'll check the docco on ehcache. > > 2009/10/16 Jan Haderka <[email protected]> > > It's been a while, so bear with me. > I guess the comment should have read BlockingCache or even > better the > net.st.ehcache.construct.blockingBlockingCache > > As you have noticed Magnolia uses EhCache underneath. The > EhCache > provides 2 implementations of EhCache interface, one of which > is > blocking (in a sense that after first call to cache.get() it > will block > all consecutive calls until there is something to return from > the cache. > Since there might be multiple threads calling different > instances of the > Default class we need to prevent such thing from happening to > avoid a > dead lock. What happens is that the first thread enters the > method, > calls the hasElement() (which results in call to cache.get()), > if the > key is not cached (the potential collision situation), the > hasElement() > returns false and new CachePolicyResult (with request to store > the key) > is created. If this thread finishes storing the key before > next request > comes, everything is fine. > Let's assume the second request for same key comes _before_ > the key is > stored, it will enter the synchronized block (because the > first thread > already left it) and it will call the hasElement() (and > consecutive call > to cache.get() will block the thread there in the synchronized > block > until the cache key is stored and retrievable). So this second > thread is > effectively stuck in the sync block, preventing anyone else > from > entering it until the first thread is done with the retrieval > of the > content for the key and caching the entry. > The point here is that we know exactly how many other threads > might have > called the cache.get() method and can recover them by placing > the value > later in the executor.Store.processCacheRequest() method (in > the finally > block, notice we store the the cache key anyway, even if we > can't > generate the entry to release the second thread that might be > stuck in > the synchronized block in the Default cache policy). This > works for as > long as there is max one thread stuck in the hasElement() (or > more > precisely in cache.get()) call. If there will be more requests > for the > key, they would be all released at a same time and they will > all try to > generate the entry at a same time. This in itself should not > be the > problem, but you need to keep in mind that entry creation is > normally > pretty fast. If this situation occurred in the first place, it > was > because either server is very busy, or producing response for > this given > request is very heavy and slow. In either case you don't want > to perform > it multiple times in parallel as it will not be faster then > waiting for > the one processing thread to finish it and provide it as > cached entry to > the others. > > Hope this was clear enough. > > More pointers - look at the svn history of the Default cache > policy. > Look at the issues related to cache since Magnolia 3.6 (incl. > the > milestone and RC issues). > Look at the EhCacheWrapper, Store executor and CacheFilter. > Look at the > EhCache documentation and explanation of BlockingCache. > > > Cheers, > Jan > > > On Thu, 2009-10-08 at 15:55 +0100, Luke Biddell wrote: > > > > Hi folks, > > > > I've got some questions about the cache and hoped you > wouldn't mind me > > harassing you about it. I've some queries around the cache > > implementation within Magnolia and was hoping you could set > me > > straight. > > > > > > I've recently implemented a custom cache policy and as part > of that > > work stumbled upon this piece of code within the > > info.magnolia.module.cache.cachepolicy.Default class. > > > > > > // we need to synchronize on the cache instance, as multiple > threads > > might be accessing this > > // concurrently, and we don't want to block the system if > we're using > > a blocking cache. > > // (since hasElement() might place a mutex on the cache key) > > synchronized (cache) { > > if (cache.hasElement(key)) { > > final Object cachedEntry = cache.get(key); > > return new > CachePolicyResult(CachePolicyResult.useCache, key, > > cachedEntry); > > } else { > > return new > CachePolicyResult(CachePolicyResult.store, key, > > null); > > } > > } > > > > > > > > > > > > > > 1) My first query is around the two calls to retrieve any > given > > element. eg you first call hasElement and then get. Could we > not just > > call get and check null as below? > > > > > > > > > > > > > > final Object cachedEntry = cache.get(key); > > if (cachedEntry != null) { > > return new > CachePolicyResult(CachePolicyResult.useCache, key, > > cachedEntry); > > } else { > > return new CachePolicyResult(CachePolicyResult.store, > key, > > null); > > } > > > > > > > > 2) The second query I have is about the use of the > synchronised block > > around the cache retrieval. I've searched the entire > codebase and all > > of the calls which add to the cache have no such > synchronized block, > > only the retrieval does (I could easily have missed them > though as I > > don't have the full source). > > > > The comment above the block doesn't make sense as by > synchronising on > > the cache object you're only marshalling access to that > section of > > code; subsequent calls to hasElement and get will block > globally > > regardless of this? > > > > Under the covers, it appears to be using ehCache which is > fully thread > > safe and no doubt uses appropriate read/write locking. > > > > I would also expect locking code to be encapsulated within > the cache > > class implementation, the code which uses the cache can then > be > > properly agnostic regarding the locking strategy. > > > > Thanks for any pointers you can give. > > > > > > > > Luke > > > > ---------------------------------------------------------------- > For list details see > http://www.magnolia-cms.com/home/community/mailing-lists.html > To unsubscribe, E-mail to: > <[email protected]> > ---------------------------------------------------------------- > > ---------------------------------------------------------------- For list details see http://www.magnolia-cms.com/home/community/mailing-lists.html To unsubscribe, E-mail to: <[email protected]> ----------------------------------------------------------------
