Skip to content

Commit 481b02c

Browse files
[FIXED] Route missing after reconnect when duplicate resolution kept gossip URL (#8527)
Follow-up of #8454 Similarly found in Antithesis, for a `$SYS` route not re-establishing. Turned out to be the same bug as before, in `handleDuplicateRoute` versus `upgradeRouteToSolicited`, for any duplicate pinned routes or normal duplicate routes in non-pool mode.
2 parents b1ac5e0 + e67032a commit 481b02c

2 files changed

Lines changed: 91 additions & 3 deletions

File tree

server/route.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2490,9 +2490,14 @@ func handleDuplicateRoute(remote, c *client, setNoReconnect bool) {
24902490
}
24912491

24922492
remote.mu.Lock()
2493-
if didSolicit && !remote.route.didSolicit {
2493+
if didSolicit {
2494+
// On disconnect, an explicit route only reconnects if the connection's
2495+
// URL matches a configured route. So adopt the given URL rather than
2496+
// keeping a gossiped one, which may not match any configured route.
2497+
if !remote.route.didSolicit || (rtype == Explicit && remote.route.routeType != Explicit) {
2498+
remote.route.url = url
2499+
}
24942500
remote.route.didSolicit = true
2495-
remote.route.url = url
24962501
}
24972502
// The extra route might be an configured explicit route
24982503
// so keep the state that the remote was configured.

server/routes_test.go

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5664,7 +5664,7 @@ func BenchmarkProcessLeafMsgArgs_Queues(b *testing.B) {
56645664
}
56655665
}
56665666

5667-
func TestRouteReconnectAfterDuplicateRouteUpgrade(t *testing.T) {
5667+
func TestRouteReconnectAfterSolicitedUpgradeAdoptsConfiguredURL(t *testing.T) {
56685668
// Default pool size + pinned $SYS route, like a production setup, with a
56695669
// hostname-based route URL like production configs use.
56705670
ob := DefaultOptions()
@@ -5726,3 +5726,86 @@ func TestRouteReconnectAfterDuplicateRouteUpgrade(t *testing.T) {
57265726
_, err = nca.Request("reconnect.echo", []byte("ping"), time.Second)
57275727
require_NoError(t, err)
57285728
}
5729+
5730+
func TestRouteReconnectAfterDuplicateRouteAdoptsConfiguredURL(t *testing.T) {
5731+
// handleDuplicateRoute is the other path that promotes a route to Explicit,
5732+
// used for duplicate per-account (pinned) routes and for any duplicate
5733+
// route in non-pool mode. It must adopt the configured URL too, otherwise
5734+
// routeStillValid rejects the gossiped one and the reconnect is abandoned.
5735+
// The global account is pinned here so interest can use ordinary pub/sub.
5736+
ob := DefaultOptions()
5737+
ob.Cluster.PinnedAccounts = []string{globalAccountName}
5738+
sb := RunServer(ob)
5739+
defer sb.Shutdown()
5740+
5741+
oa := DefaultOptions()
5742+
oa.Cluster.PinnedAccounts = []string{globalAccountName}
5743+
oa.Routes = RoutesFromStr(fmt.Sprintf("nats://localhost:%d", sb.ClusterAddr().Port))
5744+
sa := RunServer(oa)
5745+
defer sa.Shutdown()
5746+
checkClusterFormed(t, sa, sb)
5747+
5748+
// Grab A's pinned route for the global account.
5749+
var pinned *client
5750+
sa.mu.RLock()
5751+
for _, r := range sa.accRoutes[globalAccountName] {
5752+
pinned = r
5753+
}
5754+
sa.mu.RUnlock()
5755+
require_NotNil(t, pinned)
5756+
5757+
// Put the connection in the pre-upgrade gossip-dial state, then resolve it
5758+
// as a duplicate against the configured explicit route, as addRoute does
5759+
// for a pinned account. The upgrade must adopt the configured URL so the
5760+
// later reconnect passes routeStillValid.
5761+
gossipURL, err := url.Parse(fmt.Sprintf("nats-route://127.0.0.1:%d/", sb.ClusterAddr().Port))
5762+
require_NoError(t, err)
5763+
pinned.mu.Lock()
5764+
pinned.route.didSolicit = true
5765+
pinned.route.routeType = Implicit
5766+
pinned.route.url = gossipURL
5767+
pinned.mu.Unlock()
5768+
5769+
configured := oa.Routes[0]
5770+
dup := &client{}
5771+
dup.route = &route{
5772+
url: configured,
5773+
didSolicit: true,
5774+
routeType: Explicit,
5775+
accName: []byte(globalAccountName),
5776+
}
5777+
handleDuplicateRoute(pinned, dup, true)
5778+
5779+
pinned.mu.Lock()
5780+
upURL := pinned.route.url
5781+
pinned.mu.Unlock()
5782+
require_Equal(t, upURL, configured)
5783+
5784+
// Drop the connection the way a fault would (slow consumer). The pinned
5785+
// route must come back, otherwise this account's mesh stays severed while
5786+
// every pooled route looks healthy.
5787+
pinned.closeConnection(SlowConsumerWriteDeadline)
5788+
checkClusterFormed(t, sa, sb)
5789+
5790+
checkFor(t, 5*time.Second, 50*time.Millisecond, func() error {
5791+
sa.mu.RLock()
5792+
defer sa.mu.RUnlock()
5793+
for _, r := range sa.accRoutes[globalAccountName] {
5794+
if r != nil && r != pinned {
5795+
return nil
5796+
}
5797+
}
5798+
return fmt.Errorf("pinned route for %q did not reconnect", globalAccountName)
5799+
})
5800+
5801+
// Interest must flow across the re-established pinned route again.
5802+
nca := natsConnect(t, sa.ClientURL())
5803+
defer nca.Close()
5804+
ncb := natsConnect(t, sb.ClientURL())
5805+
defer ncb.Close()
5806+
natsSub(t, ncb, "pinned.echo", func(m *nats.Msg) { m.Respond(m.Data) })
5807+
natsFlush(t, ncb)
5808+
checkSubInterest(t, sa, globalAccountName, "pinned.echo", 2*time.Second)
5809+
_, err = nca.Request("pinned.echo", []byte("ping"), time.Second)
5810+
require_NoError(t, err)
5811+
}

0 commit comments

Comments
 (0)