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

@Cacheable does not respect cache hit when empty Mono/Flux response is returned #31868

Closed
fabioacsilva opened this issue Dec 19, 2023 · 7 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Milestone

Comments

@fabioacsilva
Copy link

fabioacsilva commented Dec 19, 2023

Hello,

Upon upgrading to spring-boot 3.2.0 and Java 21 our team noticed that our methods annotated with @Cacheable do not cache properly when the return is an empty Flux.

Sample source code provided:
cacheable.zip

Using the past example if we hit the endpoint with anything other than "broken" we get the following log:

2023-12-19T13:41:26.592Z  INFO 50082 --- [           main] c.e.cacheable.CacheableApplicationKt     : Started CacheableApplicationKt in 0.809 seconds (process running for 1.124)
2023-12-19T13:41:30.216Z  INFO 50082 --- [ctor-http-nio-2] com.example.cacheable.Controller         : Controller - id:hello
2023-12-19T13:41:30.217Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : Computed cache key 'hello' for operation Builder[public reactor.core.publisher.Flux com.example.cacheable.Service.get(java.lang.String)] caches=[myCache] | key='' | keyGenerator='' | cacheManager='' | cacheResolver='' | condition='' | unless='' | sync='false'
2023-12-19T13:41:30.217Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : No cache entry for key 'hello' in cache(s) [myCache]
2023-12-19T13:41:30.218Z  INFO 50082 --- [ctor-http-nio-2] org.springframework.stereotype.Service   : Service - id:hello
2023-12-19T13:41:30.219Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : Computed cache key 'hello' for operation Builder[public reactor.core.publisher.Flux com.example.cacheable.Service.get(java.lang.String)] caches=[myCache] | key='' | keyGenerator='' | cacheManager='' | cacheResolver='' | condition='' | unless='' | sync='false'
2023-12-19T13:41:30.219Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : Creating cache entry for key 'hello' in cache(s) [myCache]
2023-12-19T13:41:32.744Z  INFO 50082 --- [ctor-http-nio-2] com.example.cacheable.Controller         : Controller - id:hello
2023-12-19T13:41:32.745Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : Computed cache key 'hello' for operation Builder[public reactor.core.publisher.Flux com.example.cacheable.Service.get(java.lang.String)] caches=[myCache] | key='' | keyGenerator='' | cacheManager='' | cacheResolver='' | condition='' | unless='' | sync='false'
2023-12-19T13:41:32.757Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : Cache entry for key 'hello' found in cache(s) 

As we can see the service log is only called once due to the cache.

If we call it with "broken" we get the following log:

2023-12-19T13:41:48.729Z  INFO 50082 --- [ctor-http-nio-2] com.example.cacheable.Controller         : Controller - id:broken
2023-12-19T13:41:48.730Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : Computed cache key 'broken' for operation Builder[public reactor.core.publisher.Flux com.example.cacheable.Service.get(java.lang.String)] caches=[myCache] | key='' | keyGenerator='' | cacheManager='' | cacheResolver='' | condition='' | unless='' | sync='false'
2023-12-19T13:41:48.730Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : No cache entry for key 'broken' in cache(s) [myCache]
2023-12-19T13:41:48.730Z  INFO 50082 --- [ctor-http-nio-2] org.springframework.stereotype.Service   : Service - id:broken
2023-12-19T13:41:48.731Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : Computed cache key 'broken' for operation Builder[public reactor.core.publisher.Flux com.example.cacheable.Service.get(java.lang.String)] caches=[myCache] | key='' | keyGenerator='' | cacheManager='' | cacheResolver='' | condition='' | unless='' | sync='false'
2023-12-19T13:41:48.731Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : Creating cache entry for key 'broken' in cache(s) [myCache]
2023-12-19T13:41:51.560Z  INFO 50082 --- [ctor-http-nio-2] com.example.cacheable.Controller         : Controller - id:broken
2023-12-19T13:41:51.561Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : Computed cache key 'broken' for operation Builder[public reactor.core.publisher.Flux com.example.cacheable.Service.get(java.lang.String)] caches=[myCache] | key='' | keyGenerator='' | cacheManager='' | cacheResolver='' | condition='' | unless='' | sync='false'
2023-12-19T13:41:51.561Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : Cache entry for key 'broken' found in cache(s) [myCache]
2023-12-19T13:41:51.564Z  INFO 50082 --- [ctor-http-nio-2] org.springframework.stereotype.Service   : Service - id:broken
2023-12-19T13:41:51.565Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : Computed cache key 'broken' for operation Builder[public reactor.core.publisher.Flux com.example.cacheable.Service.get(java.lang.String)] caches=[myCache] | key='' | keyGenerator='' | cacheManager='' | cacheResolver='' | condition='' | unless='' | sync='false'
2023-12-19T13:41:51.565Z TRACE 50082 --- [ctor-http-nio-2] o.s.cache.interceptor.CacheInterceptor   : Creating cache entry for key 'broken' in cache(s) [myCache]

The service keeps getting called meaning the cache is not working properly.

If we revert to spring-boot 3.1.6 and Java 17 the behaviour is the one expected and the empty flux gets cached

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 19, 2023
@fabioacsilva fabioacsilva changed the title Spring-boot @Cacheable not working when empty response is returned Spring-boot 3.2.0 - @Cacheable not working when empty response is returned Dec 19, 2023
@snicoll
Copy link
Member

snicoll commented Dec 19, 2023

@Cacheable do not cache properly when the return is an empty Flux.

Spring Framework 6.1 has explicit support for reactive types (which wasn't really supported before). See #17920

@fabioacsilva
Copy link
Author

But caching is working for everything except when the response is an empty flux, which was working in spring-boot 3.1.6

@philwebb

This comment has been minimized.

@snicoll snicoll transferred this issue from spring-projects/spring-boot Dec 20, 2023
@snicoll snicoll changed the title Spring-boot 3.2.0 - @Cacheable not working when empty response is returned @Cacheable not working when empty response is returned Dec 20, 2023
@jhoeller
Copy link
Contributor

We mean to cache an empty list in such a case, and we do put that cache entry, but then we reinvoke the method in case of a cache hit with an empty publisher (which our unit test did not catch since it was only checking the cache entry itself).

To be fixed for 6.1.3.

@jhoeller jhoeller added type: regression A bug that is also a regression in: core Issues in core modules (aop, beans, core, context, expression) and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 20, 2023
@jhoeller jhoeller self-assigned this Dec 20, 2023
@jhoeller jhoeller added this to the 6.1.3 milestone Dec 20, 2023
@jhoeller
Copy link
Contributor

jhoeller commented Dec 20, 2023

As a side note: @fabioacsilva so you were actually caching Flux instances before? Using Flux.cache() possibly? This wasn't meant to be used like that before since Mono/Flux instances are generally hot handles - but nevertheless this seems to have worked for some scenarios.

In any case, as of 6.1, we attach to the reactive pipeline in order to cache the produced value (a list of values in case of a Flux), so not storing the actual reactive handle in the cache but rather the value that it produces.

@fabioacsilva
Copy link
Author

fabioacsilva commented Dec 20, 2023

@jhoeller We were caching Flux instances with @Cacheable and Flux.cache(), example code:

    @Cacheable(value = ["myCache"])
    @CircuitBreaker(name = MY_CIRCUIT_BREAKER)
    internal fun get(myParam: String): Flux<MyModel> =
        webClient
            .get()
            .uri { it.path(MY_URI).build(myParam) }
            .retrieve()
            .bodyToFlux(MyModel::class.java)
            .cache(myConfiguration.updateInterval)

We had some unit tests with a bunch of values and caught the empty list one with them. Thank you for your response

@jhoeller jhoeller changed the title @Cacheable not working when empty response is returned @Cacheable does not respect cache hit when empty Mono/Flux response is returned Dec 20, 2023
@jhoeller
Copy link
Contributor

This should be fixed in the upcoming 6.1.3 snapshot through an earlier switchIfEmpty check right after cache miss determination now, rather then the late check we had after a cache hit as well.

It would be great if you could give such a snapshot a try from repo.spring.io before we actually release 6.1.3 in early January.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

5 participants