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 24, 2023
1 parent 6845864 commit 2a4b44e
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 4 deletions.
38 changes: 34 additions & 4 deletions server/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -1071,9 +1071,30 @@ func (s *Server) processImplicitRoute(info *Info, routeNoPool bool) {
// in the server's opts.Routes, false otherwise.
// Server lock is assumed to be held by caller.
func (s *Server) hasThisRouteConfigured(info *Info) bool {
urlToCheckExplicit := strings.ToLower(net.JoinHostPort(info.Host, strconv.Itoa(info.Port)))
for _, ri := range s.getOpts().Routes {
if strings.ToLower(ri.Host) == urlToCheckExplicit {
routes := s.getOpts().Routes
if len(routes) == 0 {
return false
}
// This could possibly be a 0.0.0.0 host so we will also construct a second
// url with the host section of the `info.IP` (if present).
sPort := strconv.Itoa(info.Port)
urlOne := strings.ToLower(net.JoinHostPort(info.Host, sPort))
var urlTwo string
if info.IP != _EMPTY_ {
if u, _ := url.Parse(info.IP); u != nil {
urlTwo = strings.ToLower(net.JoinHostPort(u.Hostname(), sPort))
// Ignore if same than the first
if urlTwo == urlOne {
urlTwo = _EMPTY_
}
}
}
for _, ri := range routes {
rHost := strings.ToLower(ri.Host)
if rHost == urlOne {
return true
}
if urlTwo != _EMPTY_ && rHost == urlTwo {
return true
}
}
Expand Down Expand Up @@ -1949,6 +1970,10 @@ func (s *Server) addRoute(c *client, didSolicit bool, info *Info, accName string
if acc != nil {
c.acc = acc
}
// Upgrade non solicited routes if they are found in routes configuration.
if !didSolicit && s.hasThisRouteConfigured(info) {
c.route.didSolicit, c.route.routeType = true, Explicit
}
c.mu.Unlock()

// Store this route with key being the route id hash + account name
Expand Down Expand Up @@ -2018,7 +2043,6 @@ func (s *Server) addRoute(c *client, didSolicit bool, info *Info, accName string
c.route.connectURLs = info.ClientConnectURLs
c.route.wsConnURLs = info.WSConnectURLs
c.route.poolIdx = idx
rtype := c.route.routeType
cid := c.cid
idHash := c.route.idHash
rHash := c.route.hash
Expand All @@ -2027,7 +2051,13 @@ func (s *Server) addRoute(c *client, didSolicit bool, info *Info, accName string
// For solicited routes, we need now to send the INFO protocol.
if didSolicit {
c.enqueueProto(s.generateRouteInitialInfoJSON(_EMPTY_, c.route.compression, idx))
} else if s.hasThisRouteConfigured(info) {
// Upgrade non solicited routes if they are found in routes configuration.
c.route.didSolicit, c.route.routeType = true, Explicit
}
// Now that route was possibly upgraded, get the route type that will
// be used if we try to create another route connection.
rtype := c.route.routeType
if c.last.IsZero() {
c.last = time.Now()
}
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 2a4b44e

Please sign in to comment.