-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[FIXED] Route: Infinite retry cluster connection with invalid credentials #7200
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
Conversation
…ials With route pooling, the server would create a connection to the remote and when receiving the INFO protocol, would start a new route if the amount of routes was below the effective pool size. But if the route failed due to authentication error, this would cause a rapid infinite attempt to recreate a route. The approach here is to delay the creation of the new route after receiving the first PONG after sending a PING. Resolves #7194 Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
@@ -2605,6 +2614,11 @@ func (c *client) processPong() { | |||
if reorderGWs { | |||
srv.gateway.orderOutboundConnections() | |||
} | |||
if ri != nil { | |||
srv.startGoRoutine(func() { | |||
srv.connectToRoute(ri.url, ri.rtype, true, ri.gossipMode, _EMPTY_) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this also include the case <-time.After(time.Duration(rand.Intn(100)) * time.Millisecond):
from here? https://github.com/nats-io/nats-server/pull/7200/files#diff-e02d4430ca4d47be9db1b632ddb92d22c34c0919348bc020ce809e64fca8631cL2386-L2394
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is needed. The routes will now be more serialized than before, and even then, I am not sure this was really needed (I think I had that in case where they both connect to each other A <-> B, which could cause at the end more routes to be closed than I would have liked).
…ials (#7200) With route pooling, the server would create a connection to the remote and when receiving the INFO protocol, would start a new route if the amount of routes was below the effective pool size. But if the route failed due to authentication error, this would cause a rapid infinite attempt to recreate a route. The approach here is to delay the creation of the new route after receiving the first PONG after sending a PING. Resolves #7194 Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Includes the following: - #7200 - #7201 - #7202 - #7209 - #7210 - #7211 - #7213 - #7212 - #7216 - #7217 - #7230 - #7239 - #7246 - #7248 - 8241a15, specifically delayed errors that are not JS API errors - #7158 (not containing 2.12-specific changes) - #7233 - #7255 - #7249 - #7259 - #7265 - #7273 (not including Go 1.25.x) - #7258 - #7222 Signed-off-by: Maurice van Veen <github@mauricevanveen.com> Signed-off-by: Neil Twigg <neil@nats.io>
With route pooling, the server would create a connection to the remote and when receiving the INFO protocol, would start a new route if the amount of routes was below the effective pool size. But if the route failed due to authentication error, this would cause a rapid infinite attempt to recreate a route.
The approach here is to delay the creation of the new route after receiving the first PONG after sending a PING.
Resolves #7194
Signed-off-by: Ivan Kozlovic ivan@synadia.com