Skip to content

NRG: Meta election fails if gateway host resolves to multiple IPs - #8238

Merged
neilalexander merged 1 commit into
mainfrom
maurice/gw-bootstrap
May 26, 2026
Merged

NRG: Meta election fails if gateway host resolves to multiple IPs#8238
neilalexander merged 1 commit into
mainfrom
maurice/gw-bootstrap

Conversation

@MauriceVanVeen

Copy link
Copy Markdown
Member

bootstrapRaftNode now counts each gateway URL as a single peer when estimating the JetStream meta group size, rather than resolving the host and adding one peer per IP. A dual-stack host would inflate the expected cluster size above the real node count, and result in initial meta election stalling.

Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
@MauriceVanVeen
MauriceVanVeen requested a review from a team as a code owner May 26, 2026 14:42
@synadia-claude-reviewer

synadia-claude-reviewer Bot commented May 26, 2026

Copy link
Copy Markdown

Claude finished @MauriceVanVeen's task in 2m 15s —— View job


Code Review

  • Gather context (PR diff, CLAUDE.md, relevant source files)
  • Analyze changes for correctness, concurrency safety, and raft semantics
  • Post review feedback

Summary

The fix is correct and well-targeted. The root cause was that net.LookupHost on a dual-stack hostname (e.g. "localhost") returns both 127.0.0.1 and ::1, causing ngwps to be incremented twice for what is a single server, which inflated the expected meta-group size and stalled leader election.

Consistency with route counting: nrs := len(opts.Routes) already uses URL count directly (line 369), making ngwps += len(gw.URLs) (line 384) a natural and consistent change.

Performance improvement: The old path called net.LookupHost for every non-IP hostname during bootstrap. The new path does no network I/O and is deterministic.

Test quality: TestNRGBootstrapExpectedClusterSize covers IP literals, dual-stack hostnames, and multiple remote clusters. The partially-initialised &Server{opts: opts} is safe because s.mu and s.optsMu are zero-value sync.RWMutex values, which are valid for use without explicit initialisation.


One thing to verify

ngwps += len(gw.URLs) inherits the same assumption that was already implicit for IP-literal hosts: one URL = one distinct server. This holds as long as operators don't list the same gateway server twice under different names or ports. That was equally true before the fix, and it's the standard configuration pattern — so this is fine as a heuristic for bootstrapping.

No bugs, no security issues, no blocking concerns. Good fix.

@neilalexander neilalexander left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@neilalexander
neilalexander merged commit 9dd3b69 into main May 26, 2026
73 of 74 checks passed
@neilalexander
neilalexander deleted the maurice/gw-bootstrap branch May 26, 2026 15:28
neilalexander added a commit that referenced this pull request May 29, 2026
neilalexander added a commit that referenced this pull request May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants