-
Notifications
You must be signed in to change notification settings - Fork 3.4k
CachingRouteLocator does not properly refresh routes upon RefreshRoutesEvent due to a racing condition #3005
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
Comments
We are facing exactly the same issue in our application.
We are using We in our team are thinking that this bug is a pretty big issue because with it we cannot really guarantee the stability of our application. Are there any plans for fixing this bug? @ATAlight unfortunately it seems like your workaround doesn't work for Edit: We are using this solution as a workaround now. It does the same as triggering the "Refreshing the Route Cache" Actuator API every 30 seconds: @Component
public class RefreshGatewayCacheTask {
private final AbstractGatewayControllerEndpoint gatewayControllerEndpoint;
public RefreshGatewayCacheTask(AbstractGatewayControllerEndpoint gatewayControllerEndpoint) {
this.gatewayControllerEndpoint = gatewayControllerEndpoint;
}
@Scheduled(fixedDelay = 30000)
public void refreshGatewayCache() {
// log something like "Scheduled Task: Refreshing the gateway cache"
this.gatewayControllerEndpoint.refresh(Collections.emptyList()).subscribe();
}
} For this to work you have to set the following property: That seems to do the trick for now. I don't know however if this ends in weird behaviour or bad performance in the long run. |
There was a recent change around this, can you verify it is still an issue in 4.1.2-SNAPSHOT? |
If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed. |
Thank you @spencergibb for replying and looking into this issue 👍 . |
No problem |
I've found some time to test it with It works! But that is still ok and way better than before 👍 . What I did to test the behavior:
For reference here are the exceptions I got in those 30 - 60 seconds until the gateway found the new service instance:
|
Describe the bug
I have a spring cloud gateway configured to derive routes from Consul discovery client. When a service gets registered or deregistered in Consul, the gateway application receives the event, correctly identifies that there's been a configuration change and correctly fires event
RefreshRoutesEvent
.As expected, listener method in
CachingRouteLocator
receives the event and updates the routes. However, the list of routes it receives to update cache here:
cache.put(CACHE_KEY, signals);
is stale. It reflects the previous configuration. So if a new service was registered, it's still missing in the cached list resulting in a missing route configuration for the newly registered service.If I try to set a breakpoint somewhere, it starts working properly. My assumption is there's a racing condition somewhere. It could either happen locally or maybe Consul restful API has some kind of delay and does not provide latest configuration even after it has sent an update event to the gateway.
Introducing a listener that processes
RefreshRoutesResultEvent
fired from code above like this:seems to be fixing the problem. I suspect this works due to the fact that
RefreshRoutesResultEvent
goes to the end of the event queue and causes to delay the underlying calls to Consul (or to some kind of cache in between). The problem is that I'm not supposed to do it. Also, it may not guarantee a bulletproof fix to a racing condition. Plus it's too heavy to refresh it multiple times.This seems to be a bug especially since slowing down a thread corrects the problem. It's not supposed to allow for racing conditions like that.
This is the version I'm currently using:
spring-cloud-gateway-server-3.1.8.jar
The text was updated successfully, but these errors were encountered: