Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Target Cache gets checked twice for every cache hit [SPR-11592] #16216

Closed
spring-projects-issues opened this issue Mar 24, 2014 · 7 comments
Closed
Assignees
Labels
in: core type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Mar 24, 2014

Jacques Stadler opened SPR-11592 and commented

Problem

Whenever you execute a cached method that has already been executed once, and therefore its result can be fetched from the cache, there will be 2 cache hits that happen for one execution of this method.
See e.g. this snippet from the referenced GitHub project:

      ServiceToCache obj = (ServiceToCache) context.getBean("serviceToCache");
      Cache cache = (Cache) context.getBean("cache");

      String key = "1";
      String result1 = obj.returnSomeString(key);
      // Before it calls the method the cache is checked
      verify(cache, times(1)).get(key);

      String result2 = obj.returnSomeString(key);
      // Here we would expect that the cache was only called twice, but it is being called 3 times.
      verify(cache, times(3)).get(key);

I would expect that if I execute a cached method once, there should only be one cache hit, not two.

Solution

As I see it the problem lies in org.springframework.cache.interceptor.CacheAspectSupport.execute(Invoker, CacheOperationContexts):

                // Collect puts from any @Cachable miss
		List<CachePutRequest> cachePutRequests = new ArrayList<CachePutRequest>();
                // NOTE: This produces the first cache hit
		collectPutRequests(contexts.get(CacheableOperation.class), ExpressionEvaluator.NO_RESULT, cachePutRequests, true);

		ValueWrapper result = null;

		// We only attempt to get a cached result if there are no put requests
		if (cachePutRequests.isEmpty() && contexts.get(CachePutOperation.class).isEmpty()) {
                // NOTE: This produces the second cache hit
			result = findCachedResult(contexts.get(CacheableOperation.class));
		}

I suppose the cached value should be fetched once, before calling the two methods (collectPutRequests, findCachedResult) above.


Affects: 4.0.2

Reference URL: https://github.com/stadler/poc-spring-cache

Issue Links:

  • #15750 Spring caching: combining multiple @Cacheable within @Caching annotation doesn't work

0 votes, 5 watchers

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 24, 2014

Stéphane Nicoll commented

So what happens is that collectPutRequests is collecting the cache operations that are eligible for a cache update operation. If that does not yield any value, we actually try to get the value for the key in order to return it to the user. And that requests a cache hit again...

FYI, we have an infrastructure to submit such repro project, see spring framework issues. Thanks for the detailed report!

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 24, 2014

Jacques Stadler commented

You're welcome. I'll remember spring-framework-issues for the next time.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 25, 2014

Stéphane Nicoll commented

PR available: #496

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 25, 2014

Stéphane Nicoll commented

Jacques, this seems like a sensible change to make at this point and we are going to revisit our caching support in 4.1 anyway.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 25, 2014

Jacques Stadler commented

Thx for the fast fix Stéphane. The change looks fine.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 25, 2014

Juergen Hoeller commented

This has actually been a regression in the 4.0 line; fixed for 4.0.3 now.

Added the corresponding unit tests to the 3.2.x branch as well, just in case.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 26, 2014

Stéphane Nicoll commented

Thanks for reviewing my patch, Jürgen. I haven't checked if that was a regression, silly me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants