Support of ehcache's BlockingCache [SPR-11540]

[spring-projects-issues]

Alexander Zagumennikov opened SPR-11540 and commented

Hi
I tried to use spring cache api in a web project, but unfortunately I faced with concurrency issues.

The task was to make 'blocking' cache. It means when multiple threads simultaneously try to access cache, only one thread loads data from storage, other threads wait for this thread, and once data is loaded, all threads return it. I think it's a common task for most web projects.

I haven't found anything about cache behavior in multithread environment in spring documentation, so spring has no cache synchronizations and when multiple threads access cache, they will be passed to cache back-end, and load data from storage multiple times (if back-end has no synchronization).

In spring context xml:

...
    <cache:annotation-driven/>

    <bean name="ehCacheManager" class="org.springframework.cache.ehcache.EhCacheManagerFactoryBean">
        <property name="configLocation" value="/WEB-INF/ehcache.xml"/>
    </bean>

    <bean id="cacheManager" class="org.springframework.cache.ehcache.EhCacheCacheManager">
        <property name="cacheManager" ref="ehCacheManager"/>
    </bean>

    <bean class="test.SomeCache"/>
...

ehcache.xml:

<ehcache>
    <cache name="someCache" maxBytesLocalHeap="50m"/>
</ehcache>

SomeCache.java:

public class SomeCache {

    @Cacheable("someCache")
    public String findResult(String id) {
        System.out.println("start find result");

        try {
            Thread.sleep(10000);
        } catch (InterruptedException e) {
            throw new RuntimeException(e);
        }

        System.out.println("end find result");
        return "result";
    }
}

In mvc controller:

...
        for (int i = 0; i < 10; ++i) {
            new Thread(new Runnable() {
                public void run() {
                    System.out.println("before find result");
                    someCache.findResult("id");
                    System.out.println("after find result");
                }
            }).start();
        }
...

Result:

before find result
before find result
before find result
before find result
before find result
before find result
before find result
before find result
before find result
before find result
start find result
start find result
start find result
start find result
start find result
start find result
start find result
start find result
start find result
start find result
end find result
end find result
end find result
end find result
end find result
end find result
end find result
end find result
end find result
end find result
after find result
after find result
after find result
after find result
after find result
after find result
after find result
after find result
after find result
after find result

As we can see - no synchronization, cachable methods enters multiple times.

Second approach was to configure cache back-end - EhCache in my case - to synchronize access to storage. I configured EhCache to use BlockingCache decorator, and it seemed to work fine, except of one fatal issue: if exception is thrown during data loading, BlockingCache will never release the lock (it is meant that caller should put null to cache on exception). So, spring cache facade does not support that case. EhCache also provides SelfPopulatingCache decorator that does exception handling stuff, but again spring does not support it because it requires CacheEntryFactory to be passed in constructor.

ehcache.xml:

<ehcache>
    <cache name="someCache" maxBytesLocalHeap="50m">
        <cacheDecoratorFactory class="test.BlockingCacheDecoratorFactory"/>
    </cache>
</ehcache>

BlockingCacheDecoratorFactory.java:

package test;

import net.sf.ehcache.Ehcache;
import net.sf.ehcache.constructs.CacheDecoratorFactory;
import net.sf.ehcache.constructs.blocking.BlockingCache;

import java.util.Properties;

public class BlockingCacheDecoratorFactory extends CacheDecoratorFactory {

    @Override
    public Ehcache createDecoratedEhcache(Ehcache cache, Properties properties) {
        return new BlockingCache(cache);
    }

    @Override
    public Ehcache createDefaultDecoratedEhcache(Ehcache cache, Properties properties) {
        return new BlockingCache(cache);
    }
}

Result:

before find result
before find result
before find result
before find result
before find result
before find result
before find result
before find result
before find result
before find result
start find result
end find result
after find result
after find result
after find result
after find result
after find result
after find result
after find result
after find result
after find result
after find result

Seems fine, but what if we have an exception?

SomeCache.java:

package test;

import org.springframework.cache.annotation.Cacheable;

public class SomeCache {

    @Cacheable("someCache")
    public String findResult(String id) {
        System.out.println("start find result");

        throw new IllegalStateException();
    }
}

Result:

before find result
before find result
before find result
before find result
before find result
before find result
before find result
before find result
before find result
before find result
start find result
Exception in thread "Thread-36" java.lang.IllegalStateException
        at test.SomeCache.findResult(SomeCache.java:11)
        at test.SomeCache$$FastClassByCGLIB$$96e26c72.invoke(<generated>)
        at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:204)
        at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:701)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:150)
        at org.springframework.cache.interceptor.CacheInterceptor$1.invoke(CacheInterceptor.java:58)
        at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:211)
        at org.springframework.cache.interceptor.CacheInterceptor.invoke(CacheInterceptor.java:66)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
        at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:634)
        at test.SomeCache$$EnhancerByCGLIB$$dc1816.findResult(<generated>)
        ...
        at java.lang.Thread.run(Thread.java:722)

And if we try to call it second time:

before find result
before find result
before find result
before find result
before find result
before find result
before find result
before find result
before find result
before find result

Only one thread exited with exception, others are deadlocked.

Details about ehcache behavior here:
http://ehcache.org/apidocs/net/sf/ehcache/constructs/blocking/BlockingCache.html#get(java.lang.Object)
http://grepcode.com/file/repo1.maven.org/maven2/net.sf.ehcache.internal/ehcache-core/2.7.4/net/sf/ehcache/constructs/blocking/SelfPopulatingCache.java#SelfPopulatingCache.get%28java.lang.Object%29

As a result - I can't use spring caching facade with annotations to make a blocking cache.

I see two ways to resolve the situation:

1. Support EhCache BlockingCache decorator - put null on exception, don't know if this could be achieved in general way, not affecting other cache back-ends.
2. Implement blocking cache in spring - it is better approach in my opinion. In this case we will have blocking cache with any back-end.

Related issues:
#14222
#13892
#13942

---

Affects: 4.0.2

Issue Links:

• Provide some synchronization between @Cacheable and @CacheEvict annotated methods [SPR-9304] #13942 Provide some synchronization between @Cacheable and @CacheEvict annotated methods
• Provide means to configure multithreaded access for cache abstraction [SPR-9254] #13892 Provide means to configure multithreaded access for cache abstraction
• Documentation does not explain how @Cacheable and @CacheEvict behave in a multi-threaded environment [SPR-9588] #14222 Documentation does not explain how @Cacheable and @CacheEvict behave in a multi-threaded environment
• Cache Abstraction Exception Handling [SPR-9275] #13913 Cache Abstraction Exception Handling

Referenced from: commits aaae10c

1 votes, 9 watchers

[spring-projects-issues]

Stéphane Nicoll commented

I just want to make sure I got this right. You are using the cache abstraction (either the @Cache* annotations or the caching aspect) and several threads are invoking a time consuming load operation of yours.

You seem to have investigated the EhCache issue quite a lot. Would you like to share a code sample that demonstrate the issue you faced with exception while loading? I am not entirely sure I understand where this exception is thrown and how it relates to Spring.

Thanks!

[spring-projects-issues]

Alexander Zagumennikov commented

Hi, thanks for looking into this! I have provided examples in the issue details.

[spring-projects-issues]

Stéphane Nicoll commented

Thanks for getting back to me. I am working on #13913 that should provide what you need for the first option. Once I am set, I'll try your use case again to see if that helps. By the way, if you have this code running somewhere that would help me avoiding copy/pasting that in an actual project.

[spring-projects-issues]

Alexander Zagumennikov commented

Actually, the issue you mentioned is about cache provider exceptions, but this one is about exceptions in cachable method. Anyway, I'll be glad if they could be fixed together.
Sorry, but I don't run this code in my projects, it's just a sample.

[spring-projects-issues]

Stéphane Nicoll commented

ahhhh, good point. I should probably add an extra callback here.

[spring-projects-issues]

Stéphane Nicoll commented

So I have a branch now that allows you to customize the interceptor to use. In that custom interceptor, you could override a particular method, something like this:

protected Object invokeOperation(CacheOperationInvoker invoker) {
	try {
		return invoker.invoke();
	}
	catch (CacheOperationInvoker.ThrowableWrapper throwableWrapper) {
		// Handling ...
		return null;
	}
}

This will indeed return null and since BlockingCache has a particular handling there, it will release the lock and not put anything in the cache which will give the chance to another thread to compute the value again.

But there's a problem: the first thread has completed without an exception (since you caught it). And it will return null as the value of the cached value. That's not what you want, right?

The bottom line is that you shouldn't try to implement such use cases in the cache, in particular regardless of the fact that you are using Spring's cache abstraction. I am leaning towards rejecting this altogether actually: that locking business is really something I'd like us avoid doing.

[spring-projects-issues]

Alexander Zagumennikov commented

The example you wrote is not correct of course, we should not suppress the exception.
Spring should catch exception, put null to cache and rethrow exception, like in EHCache SelfPopulatingCache:

try {
    // Value not cached - fetch it
    Object value = factory.createEntry(key);
    element = makeAndCheckElement(key, value);
} catch (final Throwable throwable) {
    // Could not fetch - Ditch the entry from the cache and rethrow
    // release the lock you acquired
    element = new Element(key, null);
    throw new CacheException("Could not fetch object for cache entry with key \"" + key + "\".", throwable);
} finally {
    put(element);
}

In terms of spring it should be something like this (CacheAspectSupport in spring 3.2.x):

try {
    retVal = invoker.invoke();
    inspectAfterCacheEvicts(ops.get(EVICT), retVal);
    if (!updates.isEmpty()) {
        update(updates, retVal);
    }
    return retVal;
} catch (Throwable t) {
    if (!updates.isEmpty()) {
        update(updates, null);
    }
    throw t;
}

I'm not sure I understood your last sentence, what use cases I shoudn't try to implement?

[spring-projects-issues]

Stéphane Nicoll commented

This works because BlockingCache has a special treatment for null values. If the value is null then it releases the lock without impacting the cache (i.e. assigning that key with null). I thought I had updated my original answer when I discovered that. The problem with that approach is that you need an access to the cache to set the value to null to release the lock and throw the exception.

The interceptor I have put in place does not get the cache(s) as an argument and it wouldn't make any sense to change that. I guess you could certainly inject that cache in your own code. In the end you may be able to get what you want.

That being said (and to answer your question) we won't implement this at the abstraction level. If you look at BlockingCache the code is quite complicated already and putting that back in a generic abstraction is probably going to be more complicated by an order of magnitude. In the end, a cache has only one goal: being fast. Your application should just run fine with or without cache and, more importantly, adding the cache should not have any side effect to the behaviour of your app. Starting to lock things is not going in that direction.

[spring-projects-issues]

Alexander Zagumennikov commented

Ok, I see your point, and I agree that EhCache's BlockingCache specifics shouldn't affect spring abstraction.
But I can't agree with you that locking stuff makes cache slower or have any side effects. Quite often you have a lot of simultaneous (multi-threaded) requests to a single resource which is expensive to retrieve, and blocking cache will save you in these cases.

[spring-projects-issues]

Stéphane Nicoll commented

I don't think I said that locking stuff makes cache slower. It may have side effects, I hope we can agree about that: if that was simple, we wouldn't have this discussion in the first place.

Our general advice is to pre-load the cache for things that may be expensive instead of lazily loading them per thread

[spring-projects-issues]

Stéphane Nicoll commented

I have added a invokeOperation in aaae10c for both Spring and JCache cache interceptors that you can use to include the logic that we have discussed here.

You can override the CachingInterceptor that is being used by the caching advice by configuring the caching infrastructure yourself (check in particular org.springframework.cache.annotation.ProxyCachingConfiguration). I don't want to make such scenario a first class citizen in the caching infrastructure support. That being said, it is now possible to integrate BlockingCache in your use case if you really want to do this.

[spring-projects-issues]

Brandon Zeeb commented

Eagerly loading caches is a poor idea when the potential key space is large, the concurrency level is high enough for a blocking cache to pay off, available memory is low, and client behavior is not predicable not understood.

How would you both recommend to configure a @Cacheable cache manager to work with a BlockingCache given your changes? This is not clear to me.

[spring-projects-issues]

Stéphane Nicoll commented

I wouldn't recommend it, that's the whole point I am trying to make. But if you really want to use that and that works out for you, you can create your own interceptor and override invokeOperation. The only problem is to release the lock in case of an exception so you could catch such exception, release the lock on your BlockingCache (with a put with a null value).

[spring-projects-issues]

Xian Li commented

How do you treat @Cachable#unless while using the blockingCache?
If a key is miss and the result is not needed to put. The other thread will be deadlocked.

[spring-projects-issues]

Stéphane Nicoll commented

I don't want to make such scenario a first class citizen in the caching infrastructure support. That being said, it is now possible to integrate BlockingCache in your use case if you really want to do this.

You're absolutely right, that's one of the corner cases that could lead to issues and the reason why we don't support BlockingCache out-of-the-box. I don't see an immediate way of fixing this issue unfortunately.