Skip to content

Commit

Permalink
[IMPROVED] Routing: upgrade non solicited routes if present in config
Browse files Browse the repository at this point in the history
When a server accepts a route, that route would not be marked as
solicited or configured while it should if it was present in the
configuration.

Prior to pooling, if server A pointed to server B and vice-versa,
one of the server will have the route as solicited and the server
that accepted the route would have upgraded it as solicited.
With pooling code, this was not always happening.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
  • Loading branch information
kozlovic committed Oct 25, 2023
1 parent 6845864 commit cd36ecc
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 0 deletions.
20 changes: 20 additions & 0 deletions server/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -1998,6 +1998,26 @@ func (s *Server) addRoute(c *client, didSolicit bool, info *Info, accName string
s.mu.Unlock()
return false
}
} else {
// If we solicit, upgrade to solicited all non-solicited routes that
// we may have registered.
c.mu.Lock()
url := c.route.url
rtype := c.route.routeType
c.mu.Unlock()
for _, r := range conns {
if r != nil {
r.mu.Lock()
if !r.route.didSolicit {
r.route.didSolicit = true
r.route.url = url
}
if rtype == Explicit {
r.route.routeType = Explicit
}
r.mu.Unlock()
}
}
}
// For all cases (solicited and not) we need to count how many connections
// we already have, and for solicited route, we will find a free spot in
Expand Down
28 changes: 28 additions & 0 deletions server/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2127,6 +2127,20 @@ func TestRoutePoolConnectRace(t *testing.T) {
}
}

// Also, check that they all report as solicited and configured in monitoring.
for _, s := range servers {
routes, err := s.Routez(nil)
require_NoError(t, err)
for _, r := range routes.Routes {
if !r.DidSolicit {
t.Fatalf("All routes should have been marked as solicited, this one was not: %+v", r)
}
if !r.IsConfigured {
t.Fatalf("All routes should have been marked as configured, this one was not: %+v", r)
}
}
}

for _, s := range servers {
s.Shutdown()
s.WaitForShutdown()
Expand Down Expand Up @@ -2532,6 +2546,20 @@ func TestRoutePerAccountConnectRace(t *testing.T) {
}
}

// Also, check that they all report as solicited and configured in monitoring.
for _, s := range servers {
routes, err := s.Routez(nil)
require_NoError(t, err)
for _, r := range routes.Routes {
if !r.DidSolicit {
t.Fatalf("All routes should have been marked as solicited, this one was not: %+v", r)
}
if !r.IsConfigured {
t.Fatalf("All routes should have been marked as configured, this one was not: %+v", r)
}
}
}

for _, s := range servers {
s.Shutdown()
s.WaitForShutdown()
Expand Down

0 comments on commit cd36ecc

Please sign in to comment.