[noojung]

Overview

Combining @Retryable with @Cacheable or @CacheEvict on a method returning a Flux can cause the operation to hang when the publisher fails and a retry is attempted:

@Cacheable("items")
@Retryable
public Flux<Item> findItems() { ... }  // hangs on the first retry

Problem

ReactiveCachingHandler shares the returned Flux between the caller and an internal subscriber that collects emitted values for cache put or eviction operations, using publish().refCount(2). Both subscribers terminate when the source signals an error. On resubscription (for example, due to @Retryable), only the caller subscribes again, leaving refCount(2) waiting for a second subscriber and preventing a new subscription to the source.

This PR addresses two distinct problems caused by this pattern:

1. Existing bug (@Cacheable / @CachePut): processPutRequest() has used publish().refCount(2) since 6.1, so any resubscription after an error hangs. @Retryable just makes it easy to hit.
2. Unreleased regression (@CacheEvict): processCacheEvicts() copied the same pattern in 26de340 (@CacheEvict truncates Flux results to the first element #37309), so a Flux-returning @CacheEvict method combined with @Retryable now hangs, whereas it did not before that commit.

Fix

Wrap the shared Flux creation and internal subscriber registration in Flux.defer() so that each subscription, including a retry, creates a new shared Flux and registers a new cache subscriber. Apply this change to both cache put and eviction handling.

Tests

Add regression tests for @Retryable combined with @Cacheable or @CacheEvict. The tests verify that an initial failure is followed by a successful second subscription, that all expected values are returned, and that the successful operation populates or evicts the cache as appropriate.

Related Issues

• @CacheEvict truncates Flux results to the first element #37309

[sbrannen]

Thanks for the PR.

The Flux.defer() approach looks right to me. I applied the change locally and confirmed that both new tests fail without it and pass with it.

Could you please update this PR's description and the commit message to distinguish the two problems this PR addresses?

1. Existing bug (@Cacheable / @CachePut): processPutRequest() has used publish().refCount(2) since 6.1, so any resubscription after an error hangs. @Retryable just makes it easy to hit.
2. Unreleased regression (@CacheEvict): processCacheEvicts() copied the same pattern in 26de340 (gh-37309), so a Flux-returning @CacheEvict method combined with @Retryable now hangs, whereas it did not before that commit.

Please also end the commit message with the following trailers:

See gh-37309
Closes gh-37403

Note that annotations such as @Cacheable should be enclosed in backticks in the commit message, so that they don't mention GitHub users.

Also, this should target 7.0.x rather than main, since 26de340 is already on that branch. We will forward merge it to main on our side.

Cheers

[noojung]

Thanks for the review!

I've addressed your feedback:

• Updated the commit message and the PR description to distinguish the existing @Cacheable / @CachePut bug from the unreleased @CacheEvict regression
• Added the See gh-37309 and Closes gh-37403 trailers to the commit message
• Rebased the branch onto 7.0.x and changed the base branch accordingly
• I also added // gh-37403 comments to the new tests

Please let me know if you have any further feedback.

[sbrannen]

Thanks for making the requested changes.

I'll make a final review locally before merging this into 7.0.x and main.

[github-actions]

Fixed via 440139f

[sbrannen]

This has been merged into 7.0.x and main.

Thanks