[
http://jira.magnolia-cms.com/browse/MAGNOLIA-3167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=28241#action_28241
]
Jan Haderka commented on MAGNOLIA-3167:
---------------------------------------
Implementing timeout as shown in patch attached to SUPPORT-538 certainly
ensures there is no single request blocking forever. However this doesn't
justify removal of the synchronized block.
The main purpose of removing synchronized block is to ensure the mutex is
always placed in the {{hasElement()}} call and not in the subsequent {{get()}}
call. Cache is accessible by other threads and items can be removed by them
directly ({{remove(Key)}}/{{clear()}}) or indirectly by eviction of expired
items or due to reaching max amount of allowed elements.
Due to the facts outlined above, we need either {{synchronized}} block to make
sure that only one thread is able to execute such two calls in a sequence or we
need to avoid using such construction altogether. (And yes, there is still the
possibility that item will be evicted after the {{hasElement()}} and before the
{{get()}} call, in which case {{get()}} will cause mutex to be placed and error
page returned to the user.)
Anyway, the synchronization would need to be in all places we manipulate the
cache ({{cache.clear()}}, {{cache.remove(Key)}} and also {{cache.put(Key,
Val)}} and if we ever expire entries by time to live, we would need to
synchronize there too).
Below, are outlined few scenarios which help to clarify the sort of problems
that occur due to lack of synchronization:
Scenario 1:
# Thread A: {{hasElement(KeyA) --> ehCache.get(KeyA) != null ==> false, [place
mutex]}}
# Thread A: {{store(KeyA) --> <takes time>}}
# Thread B: {{hasElement(KeyA) --> ehCache.get(KeyA) ==> LockTimeoutEx}}
# Thread B: {{<return error to user>}}
# Thread A: {{store(KeyA) --> <finishes now> --> ehCache.put(KeyA, ValA) ==>
[remove mutex]}}
# Thread A: {{<return page to user>}}
Scenario 2:
# Thread A: {{hasElement(KeyA) --> ehCache.get(KeyA) != null ==> true
# Thread B: {{evict/expire(KeyA)
# Thread A: {{get(KeyA) --> ehCache.get(KeyA) ==> null, [place mutex]
# Thread A: <return error to the user>
# Thread C: {{hasElement(KeyA) --> ehCache.get(KeyA) ==> LockTimeoutEx
# Thread C: <return error to the user>
The synchronization issue could be dealt with in multiple ways:
- add {{synchronized}} to all places as outlined above. cons: hard to maintain,
test and ensure that it is always the case. Synchronization of whole cache
serializes request processing, creating performance bottleneck
- synchronize everywhere, but use system of mutexes, one per key similar to
ehCache own system. pros: removing the bottleneck. cons: extra code to write
and maintain, significant increase in complexity
- abandon usage of problematic construction. There is no need to do the double
check.
- use
> cache: single blocking request can block all other requests to cached content
> ------------------------------------------------------------------------------
>
> Key: MAGNOLIA-3167
> URL: http://jira.magnolia-cms.com/browse/MAGNOLIA-3167
> Project: Magnolia
> Issue Type: Bug
> Components: cache
> Affects Versions: 3.6.8, 4.1.4, 4.2.3, 4.3.1
> Reporter: Philipp Bärfuss
> Assignee: Jan Haderka
> Fix For: 4.3.x
>
> Attachments: cacheBlock.png, cacheBlockOnflush.png
>
>
> the cache mechanism can block all requests:
> * cache.get() will block if an other request is caching the same key (this is
> a feature of the BlockingCache)
> ** mutex per key kept until cache.put() is called (either with an entry or
> null value)
> * this code is again in a synchronized block which synchronizes on the cache
> object itself
> ** this blocks all other request trying to enter the synchronization block
> The critical scenario which can prevent magnolia from responding any request
> (all threads blocked) is the following
> 1) first request to a resource which is slow or never returns (request to a
> service, poor database connection, ..)
> 2) second request to the same resource: --> thread is blocked at
> EhCacheWrapper.get(key):56, but also keeps the lock on the cache
> 3) all other caching requests are blocked (no matter which url) due to the
> synchronize block at Default.shouldCache(Cache, AggregationState,
> FlushPolicy):89
> Solution:
> * don't synchronize on the cache (why are we doing this???)
> * allow configuration of
> [BlockingCache.timeoutMillis|http://ehcache.org/apidocs/net/sf/ehcache/constructs/blocking/BlockingCache.html#timeoutMillis]
> ** throw an exception if a request waits for to long
> * uncomment finally block at doFilter(HttpServletRequest,
> HttpServletResponse, FilterChain), this is a safety net and should log a
> FATAL ERROR message
> ** only relevant if the result is a store request
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://jira.magnolia-cms.com/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
----------------------------------------------------------------
For list details see
http://www.magnolia-cms.com/home/community/mailing-lists.html
To unsubscribe, E-mail to: <[email protected]>
----------------------------------------------------------------
