Fix: LocalResponseCacheGatewayFilterFactory recreates Caffeine cache on every route refresh - #4244
Fix: LocalResponseCacheGatewayFilterFactory recreates Caffeine cache on every route refresh#4244renechoi wants to merge 4 commits into
Conversation
…on every route refresh Signed-off-by: renechoi <renechoi90@gmail.com>
Signed-off-by: kdelay <kdelay20@gmail.com>
…s or route has no id Signed-off-by: renechoi <renechoi90@gmail.com>
anshulgoel22
left a comment
There was a problem hiding this comment.
Thanks for addressing #4243 — recreating the Caffeine cache on every route refresh defeats the purpose of local response caching in discovery-driven deployments.
What looks good
- Reusing the registered cache when route cache configuration is unchanged preserves cached entries across routine refreshes (e.g. Eureka
DiscoveryClient-CacheRefreshExecutor). - Rebuilding when TTL/size changes is the right trade-off: stale cache semantics are worse than a cold cache.
- Handling
routeId == nullseparately avoids different routes sharing one cache vianull-cache— good edge-case callout. - Dedicated unit tests for reuse vs config-change vs null route id strengthen the fix.
Suggestion (non-blocking)
If feasible, an integration test that simulates two consecutive route refreshes with identical cache config and asserts a cache hit on the second request would complement the unit coverage — similar to the production scenario in the issue.
Nice, focused fix.
Signed-off-by: renechoi <renechoi90@gmail.com>
|
Thanks for the review, and for the integration test suggestion — added in 17b6177.
One detail worth flagging, because it decided how the test had to be written: its route is declared as a I also checked that the test pins the fix rather than merely passing: reverting Local run: the |
|
Thanks for adding the integration test — the RefreshRoutesEvent sequencing and the revert-to-main failure check make this much stronger. Good callout on RouteDefinition vs RouteLocatorBuilder for observing reuse end-to-end. |
Fixes #4243.
LocalResponseCacheGatewayFilterFactorybuilt a newCaffeinecache and registered it in theCaffeineCacheManagerunder the same name every time the filter was applied. A route refresh applies the filter again, so the entries cached so far were dropped on every refresh.This change reuses the cache already registered for the route, so cached entries survive a refresh. A new cache is still built in two cases:
null-cachefor every such route and cannot tell one route from another. Routes built withRouteLocatorBuilderare in this group, sinceGatewayFilterSpec.localResponseCachedoes not set a route id. Keeping the previous behaviour there avoids routes with different settings sharing one cache.Verified against
spring-cloud-gateway-server-webflux: the added unit tests fail without the change and pass with it, and the existingLocalResponseCacheGatewayFilterFactoryTestsstay green, includingshouldNotCacheResponseWhenTimeToLiveIsReachedwhich relies on each route keeping its own time to live.