more code

internal/resolver/pool.go

@@ -26,6 +26,10 @@ var ErrNoUpstream = errors.New("resolver: no healthy upstream available")
 // ErrCircuitOpen is returned by a single upstream whose breaker is open.
 var ErrCircuitOpen = errors.New("resolver: upstream circuit breaker open")
 
+// lookupTimeout bounds a name resolution when building the pool, so a broken
+// system resolver cannot stall a config reload.
+const lookupTimeout = 2 * time.Second
+
 // Health is an upstream's current state.
 type Health string
 
@@ -40,28 +44,46 @@ const (
 	HealthUnknown Health = "unknown"
 )
 
-// Upstream is one configured server plus its live health.
+// upstreamStats is everything mutable about an upstream. It lives behind a
+// pointer shared by successive Upstream values for the same server, so a
+// reload keeps the live health state while the immutable Upstream itself is
+// replaced under the readers' feet.
+type upstreamStats struct {
+	queries     atomic.Uint64
+	failures    atomic.Uint64
+	consecutive atomic.Uint32
+	latencySum  atomic.Uint64
+	latencyN    atomic.Uint64
+
+	// ring keeps the last 512 latencies in ms for the UI's p95.
+	mu   sync.Mutex
+	ring []float64
+
+	breakerOpenUntil atomic.Int64
+	health           atomic.Pointer[Health]
+	lastErr          atomic.Pointer[string]
+}
+
+func newUpstreamStats() *upstreamStats {
+	s := &upstreamStats{}
+	s.health.Store(ptr(HealthUnknown))
+	return s
+}
+
+// Upstream is one configured server: its address, its dial-time
+// configuration and a pointer to the live stats it shares across reloads.
+//
+// An Upstream is immutable after construction. Rebind builds a fresh Upstream
+// for every server and swaps the whole pool state with one atomic store, so a
+// query in flight keeps using a consistent, unchanging view.
 type Upstream struct {
 	cfg config.UpstreamServer
 	// addr is the address actually dialled: a bootstrap IP for TLS (SNI keeps
 	// the hostname) and the literal address otherwise.
 	addr string
-	// resolved holds the current bootstrap addresses when the configured
-	// address is a name.
+	// resolved holds the bootstrap addresses behind a named upstream.
 	resolved []string
-
-	queries     atomic.Uint64
-	failures    atomic.Uint64
-	consecutive atomic.Uint32
-	latencySum  atomic.Uint64 // microseconds
-	latencyN    atomic.Uint64
-	p95         []float64 // recent latencies in ms, for the UI
-	p95mu       sync.Mutex
-	p95ring     []float64
-
-	breakerOpenUntil atomic.Int64
-	health           atomic.Pointer[Health]
-	lastErr          atomic.Pointer[string]
+	stats    *upstreamStats
 }
 
 // Addr is the address the resolver dials.
@@ -70,29 +92,35 @@ func (u *Upstream) Addr() string { return u.addr }
 // Config returns the upstream's configuration entry.
 func (u *Upstream) Config() config.UpstreamServer { return u.cfg }
 
+// Resolved returns the addresses a named upstream resolved to, if any.
+func (u *Upstream) Resolved() []string { return u.resolved }
+
 // Health returns the current health state.
 func (u *Upstream) Health() Health {
-	if h := u.health.Load(); h != nil {
+	if u == nil || u.stats == nil {
+		return HealthUnknown
+	}
+	if h := u.stats.health.Load(); h != nil {
 		return *h
 	}
 	return HealthUnknown
 }
 
-func (u *Upstream) setHealth(h Health) { u.health.Store(&h) }
+func (u *Upstream) setHealth(h Health) { u.stats.health.Store(&h) }
 
 // SetError records the last error message for the UI.
 func (u *Upstream) SetError(err error) {
 	if err == nil {
-		u.lastErr.Store(nil)
+		u.stats.lastErr.Store(nil)
 		return
 	}
 	s := err.Error()
-	u.lastErr.Store(&s)
+	u.stats.lastErr.Store(&s)
 }
 
 // LastError returns the last error message, or "".
 func (u *Upstream) LastError() string {
-	if p := u.lastErr.Load(); p != nil {
+	if p := u.stats.lastErr.Load(); p != nil {
 		return *p
 	}
 	return ""
@@ -100,10 +128,11 @@ func (u *Upstream) LastError() string {
 
 // Success records a successful query.
 func (u *Upstream) Success(d time.Duration) {
-	u.queries.Add(1)
-	u.consecutive.Store(0)
-	u.latencySum.Add(uint64(d.Microseconds()))
-	u.latencyN.Add(1)
+	s := u.stats
+	s.queries.Add(1)
+	s.consecutive.Store(0)
+	s.latencySum.Add(uint64(d.Microseconds()))
+	s.latencyN.Add(1)
 	u.recordLatency(d)
 	u.SetError(nil)
 	if u.Health() != HealthUp {
@@ -114,12 +143,13 @@ func (u *Upstream) Success(d time.Duration) {
 // Failure records a failed query and trips the breaker once the threshold is
 // crossed.
 func (u *Upstream) Failure(err error, threshold int, cooldown time.Duration) {
-	u.queries.Add(1)
-	u.failures.Add(1)
-	n := u.consecutive.Add(1)
+	s := u.stats
+	s.queries.Add(1)
+	s.failures.Add(1)
+	n := s.consecutive.Add(1)
 	u.SetError(err)
 	if threshold > 0 && int(n) >= threshold {
-		u.breakerOpenUntil.Store(time.Now().Add(cooldown).UnixNano())
+		s.breakerOpenUntil.Store(time.Now().Add(cooldown).UnixNano())
 		u.setHealth(HealthDown)
 		return
 	}
@@ -130,32 +160,33 @@ func (u *Upstream) Failure(err error, threshold int, cooldown time.Duration) {
 
 // BreakerOpen reports whether the upstream's breaker is open.
 func (u *Upstream) BreakerOpen() bool {
-	until := u.breakerOpenUntil.Load()
-	return until > time.Now().UnixNano()
+	return u.stats.breakerOpenUntil.Load() > time.Now().UnixNano()
 }
 
 func (u *Upstream) recordLatency(d time.Duration) {
 	ms := float64(d.Microseconds()) / 1000
-	u.p95mu.Lock()
-	if len(u.p95ring) < 512 {
-		u.p95ring = append(u.p95ring, ms)
+	s := u.stats
+	s.mu.Lock()
+	if len(s.ring) < 512 {
+		s.ring = append(s.ring, ms)
 	} else {
-		copy(u.p95ring, u.p95ring[1:])
-		u.p95ring[511] = ms
+		copy(s.ring, s.ring[1:])
+		s.ring[511] = ms
 	}
-	u.p95mu.Unlock()
+	s.mu.Unlock()
 }
 
 // LatencyStats returns average and p95 latency in milliseconds.
 func (u *Upstream) LatencyStats() (avg, p95 float64) {
-	n := u.latencyN.Load()
+	s := u.stats
+	n := s.latencyN.Load()
 	if n == 0 {
 		return 0, 0
 	}
-	avg = float64(u.latencySum.Load()) / float64(n) / 1000
-	u.p95mu.Lock()
-	samples := append([]float64(nil), u.p95ring...)
-	u.p95mu.Unlock()
+	avg = float64(s.latencySum.Load()) / float64(n) / 1000
+	s.mu.Lock()
+	samples := append([]float64(nil), s.ring...)
+	s.mu.Unlock()
 	if len(samples) == 0 {
 		return avg, 0
 	}
@@ -187,8 +218,11 @@ type Stats struct {
 
 // Stats snapshots one upstream.
 func (u *Upstream) Stats() Stats {
-	q := u.queries.Load()
-	f := u.failures.Load()
+	if u == nil {
+		return Stats{}
+	}
+	q := u.stats.queries.Load()
+	f := u.stats.failures.Load()
 	var rate float64
 	if q > 0 {
 		rate = float64(f) / float64(q)
@@ -212,83 +246,127 @@ func (u *Upstream) Stats() Stats {
 	}
 }
 
-// Pool is the set of upstreams plus the policy that selects among them.
-type Pool struct {
-	cfg   config.Upstream
-	mu    sync.RWMutex
-	ups   []*Upstream
-	byTag map[string][]*Upstream
-	// clients are reused per upstream: one UDP socket, one TCP dialer, one TLS
-	// session pool. Reusing them is what keeps the per-query cost low.
-	clients map[*Upstream]*dns.Client
-	// bootstrap resolves names to addresses.
-	bootstrap map[string][]string
-
-	rng *rand.Rand
-	mu2 sync.Mutex
+// poolState is an immutable snapshot of the pool: the config it was built
+// from, the upstreams, and the address index used for per-client upstream
+// overrides. Readers take the pointer once and are then immune to reloads.
+type poolState struct {
+	cfg config.Upstream
+	ups []*Upstream
+	// byAddr indexes the upstreams by every address they answer to, so an ACL
+	// group naming either the configured Addr or the resolved one works.
+	byAddr map[string][]*Upstream
 }
 
-// NewPool builds a pool from config. bootstrapIPs may be empty; then upstream
-// hostnames must be literal IPs (or resolved by the OS).
+func (st *poolState) all() []*Upstream {
+	if st == nil {
+		return nil
+	}
+	return st.ups
+}
+
+// Pool is the set of upstreams plus the policy that selects among them.
+type Pool struct {
+	state atomic.Pointer[poolState]
+	// clients are memoised per upstream: one UDP socket, one TCP dialer, one
+	// TLS session pool. Reusing them is what keeps the per-query cost low; a
+	// dns.Client is safe for concurrent use.
+	clients sync.Map // *Upstream -> *dns.Client
+
+	// mu serialises Rebind so two overlapping reloads cannot interleave their
+	// health hand-over. It is never taken on the query path.
+	mu sync.Mutex
+	// rng backs the "random" policy.
+	rngMu sync.Mutex
+	rng   *rand.Rand
+}
+
+// NewPool builds a pool from config. Bootstraps are used for named upstreams;
+// anything unresolved falls back to the system resolver.
 func NewPool(cfg config.Upstream) *Pool {
 	p := &Pool{
-		cfg:       cfg,
-		byTag:     make(map[string][]*Upstream),
-		clients:   make(map[*Upstream]*dns.Client),
-		bootstrap: make(map[string][]string),
-		rng:       rand.New(rand.NewPCG(uint64(time.Now().UnixNano()), 0x9e3779b9)),
+		rng: rand.New(rand.NewPCG(uint64(time.Now().UnixNano()), 0x9e3779b9)),
 	}
-	for _, b := range cfg.Bootstraps {
-		p.bootstrap[b.Name] = append([]string(nil), b.Addresses...)
-	}
-	for _, s := range cfg.Servers {
-		up := &Upstream{cfg: s}
-		up.setHealth(HealthUnknown)
-		p.ups = append(p.ups, up)
-	}
-	p.resolveAll()
+	p.state.Store(buildState(cfg, nil, false))
 	return p
 }
 
-func (p *Pool) resolveAll() {
-	for _, up := range p.ups {
-		host, port, err := net.SplitHostPort(up.cfg.Addr)
-		if err != nil {
-			continue
-		}
-		if net.ParseIP(host) != nil {
-			up.addr = up.cfg.Addr
-			continue
-		}
-		if addrs := p.bootstrap[host]; len(addrs) > 0 {
-			up.resolved = addrs
-			up.addr = net.JoinHostPort(addrs[0], port)
-			continue
-		}
-		// Fall back to the system resolver.
-		ips, err := net.LookupIP(host)
-		if err != nil || len(ips) == 0 {
-			up.addr = up.cfg.Addr
-			continue
-		}
-		strs := make([]string, 0, len(ips))
-		for _, ip := range ips {
-			strs = append(strs, ip.String())
-		}
-		up.resolved = strs
-		up.addr = net.JoinHostPort(strs[0], port)
+// buildState turns an upstream config into an immutable pool snapshot. When
+// carry is non-nil, servers that keep the same address and protocol also keep
+// their live health, latency and breaker state.
+func buildState(cfg config.Upstream, carry *poolState, lookup bool) *poolState {
+	bootstraps := make(map[string][]string, len(cfg.Bootstraps))
+	for _, b := range cfg.Bootstraps {
+		bootstraps[b.Name] = append([]string(nil), b.Addresses...)
 	}
+	prev := make(map[string]*Upstream)
+	if carry != nil {
+		for _, u := range carry.ups {
+			prev[u.cfg.Addr+"/"+u.cfg.Proto] = u
+		}
+	}
+
+	st := &poolState{cfg: cfg, ups: make([]*Upstream, 0, len(cfg.Servers))}
+	for _, s := range cfg.Servers {
+		addr, resolved := resolveAddr(s, bootstraps, lookup)
+		up := &Upstream{cfg: s, addr: addr, resolved: resolved}
+		if old, ok := prev[s.Addr+"/"+s.Proto]; ok {
+			up.stats = old.stats
+		} else {
+			up.stats = newUpstreamStats()
+		}
+		st.ups = append(st.ups, up)
+	}
+	st.byAddr = make(map[string][]*Upstream, len(st.ups)*2)
+	for _, up := range st.ups {
+		for _, a := range append([]string{up.cfg.Addr, up.addr}, up.resolved...) {
+			if a == "" {
+				continue
+			}
+			st.byAddr[a] = append(st.byAddr[a], up)
+		}
+	}
+	return st
+}
+
+// resolveAddr returns the address to dial for a server plus the bootstrap
+// addresses behind it, if the configured host is a name.
+func resolveAddr(s config.UpstreamServer, bootstraps map[string][]string, lookup bool) (addr string, resolved []string) {
+	host, port, err := net.SplitHostPort(s.Addr)
+	if err != nil {
+		// Not host:port — hand it to the dialer verbatim.
+		return s.Addr, nil
+	}
+	if net.ParseIP(host) != nil {
+		return s.Addr, nil
+	}
+	if addrs := bootstraps[host]; len(addrs) > 0 {
+		return net.JoinHostPort(addrs[0], port), addrs
+	}
+	if lookup {
+		ctx, cancel := context.WithTimeout(context.Background(), lookupTimeout)
+		defer cancel()
+		addrs, err := net.DefaultResolver.LookupHost(ctx, host)
+		if err == nil && len(addrs) > 0 {
+			return net.JoinHostPort(addrs[0], port), addrs
+		}
+	}
+	return s.Addr, nil
 }
 
 // Config returns the pool's upstream config.
-func (p *Pool) Config() config.Upstream { return p.cfg }
+func (p *Pool) Config() config.Upstream {
+	if st := p.state.Load(); st != nil {
+		return st.cfg
+	}
+	return config.Upstream{}
+}
 
 // All returns every upstream.
-func (p *Pool) All() []*Upstream { return p.ups }
+func (p *Pool) All() []*Upstream { return p.state.Load().all() }
 
-// Get returns an upstream by address, or nil.
+// Get returns an upstream by configured or resolved address, or nil.
 func (p *Pool) Get(addr string) *Upstream {
-	for _, u := range p.ups {
+	for _, u := range p.All() {
 		if u.addr == addr || u.cfg.Addr == addr {
 			return u
 		}
@@ -298,8 +376,9 @@ func (p *Pool) Get(addr string) *Upstream {
 
 // Healthy returns upstreams whose breaker is closed, in configured order.
 func (p *Pool) Healthy() []*Upstream {
-	out := make([]*Upstream, 0, len(p.ups))
-	for _, u := range p.ups {
+	ups := p.All()
+	out := make([]*Upstream, 0, len(ups))
+	for _, u := range ups {
 		if !u.BreakerOpen() {
 			out = append(out, u)
 		}
@@ -309,8 +388,9 @@ func (p *Pool) Healthy() []*Upstream {
 
 // Stats snapshots every upstream.
 func (p *Pool) Stats() []Stats {
-	out := make([]Stats, 0, len(p.ups))
-	for _, u := range p.ups {
+	ups := p.All()
+	out := make([]Stats, 0, len(ups))
+	for _, u := range ups {
 		out = append(out, u.Stats())
 	}
 	return out
@@ -318,95 +398,54 @@ func (p *Pool) Stats() []Stats {
 
 // clientFor returns the memoised dns.Client for an upstream.
 func (p *Pool) clientFor(u *Upstream) *dns.Client {
-	p.mu.RLock()
-	c, ok := p.clients[u]
-	p.mu.RUnlock()
-	if ok {
-		return c
+	if c, ok := p.clients.Load(u); ok {
+		return c.(*dns.Client)
 	}
-	var nc *dns.Client
+	nc := &dns.Client{Net: "udp", UDPSize: 1232, Timeout: 3 * time.Second}
 	switch u.cfg.Proto {
 	case "tls":
-		tlsCfg := &tls.Config{
-			ServerName:         u.cfg.ServerName,
-			InsecureSkipVerify: u.cfg.InsecureSkipVerify,
-			MinVersion:         tls.VersionTLS12,
-			NextProtos:         []string{"dot"},
-		}
 		nc = &dns.Client{
-			Net:       "tcp-tls",
-			TLSConfig: tlsCfg,
-			Dialer:    &net.Dialer{Timeout: 3 * time.Second},
-			Timeout:   5 * time.Second,
+			Net: "tcp-tls",
+			TLSConfig: &tls.Config{
+				ServerName:         u.cfg.ServerName,
+				InsecureSkipVerify: u.cfg.InsecureSkipVerify,
+				MinVersion:         tls.VersionTLS12,
+				NextProtos:         []string{"dot"},
+			},
+			Dialer:  &net.Dialer{Timeout: 3 * time.Second},
+			Timeout: 5 * time.Second,
 		}
 	case "tcp":
 		nc = &dns.Client{Net: "tcp", Timeout: 5 * time.Second}
-	default:
-		nc = &dns.Client{Net: "udp", UDPSize: 1232, Timeout: 3 * time.Second}
-	}
-	p.mu.Lock()
-	p.clients[u] = nc
-	p.mu.Unlock()
-	return nc
-}
-
-// selectFor returns the upstreams to try, honouring the policy. tags, if
-// non-empty, restricts the pool to a group's own upstreams.
-func (p *Pool) selectFor(policy string, maxAttempts int) []*Upstream {
-	healthy := p.Healthy()
-	if len(healthy) == 0 {
-		// Every breaker is open: try them all anyway rather than failing the
-		// query outright. A blip should not become an outage.
-		healthy = append(healthy, p.ups...)
-	}
-	if len(healthy) == 0 {
-		return nil
-	}
-	if maxAttempts <= 0 || maxAttempts > len(healthy) {
-		maxAttempts = len(healthy)
-	}
-	switch policy {
-	case "failover":
-		// Configured order, and rotate past the unhealthy ones.
-		return healthy[:maxAttempts]
-	case "random":
-		p.mu2.Lock()
-		p.rng.Shuffle(len(healthy), func(i, j int) { healthy[i], healthy[j] = healthy[j], healthy[i] })
-		p.mu2.Unlock()
-		return healthy[:maxAttempts]
-	default: // parallel
-		return healthy
 	}
+	actual, _ := p.clients.LoadOrStore(u, nc)
+	return actual.(*dns.Client)
 }
 
 // Exchange forwards q to the pool and returns the first acceptable answer.
 //
-// policy is the pool's configured policy; tags restricts the pool to a
-// per-client upstream override. edns0Size is the client's advertised buffer,
-// used to decide TCP fallback on truncation.
-func (p *Pool) Exchange(ctx context.Context, q *dns.Msg, tags []string, timeout time.Duration) (*dns.Msg, string, error) {
-	candidates := p.ups
-	if len(tags) > 0 {
-		if tagged := p.byTag[strings.Join(tags, ",")]; len(tagged) > 0 {
-			candidates = tagged
-		}
+// want restricts the query to the upstreams an ACL group named, by configured
+// or resolved address; an empty list means the whole pool. timeout bounds one
+// attempt.
+func (p *Pool) Exchange(ctx context.Context, q *dns.Msg, want []string, timeout time.Duration) (*dns.Msg, string, error) {
+	st := p.state.Load()
+	candidates := st.all()
+	if len(want) > 0 {
+		candidates = p.restrict(st, want)
 	}
 	if len(candidates) == 0 {
 		return nil, "", ErrNoUpstream
 	}
-	attempts := p.cfg.MaxAttempts
-	chosen := selectFrom(candidates, p.cfg.Policy, attempts)
+	chosen := p.pick(st, candidates, st.cfg.MaxAttempts)
 	if len(chosen) == 0 {
 		return nil, "", ErrNoUpstream
 	}
-
 	if len(chosen) == 1 {
-		msg, up, err := p.exchangeOne(ctx, chosen[0], q, timeout)
-		return msg, up, err
+		return p.exchangeOne(ctx, st.cfg, chosen[0], q, timeout)
 	}
 
-	// Parallel: send to all at once, take the first answer that is not a
-	// transport failure. Cancellable so we do not leak requests to a slow
+	// Parallel: send to all at once and take the first real answer. The
+	// context is cancelled on return so we do not leak a request to a slow
 	// upstream after another one has answered.
 	type res struct {
 		msg *dns.Msg
@@ -418,7 +457,7 @@ func (p *Pool) Exchange(ctx context.Context, q *dns.Msg, tags []string, timeout
 	defer cancel()
 	for _, up := range chosen {
 		go func(up *Upstream) {
-			msg, addr, err := p.exchangeOne(sub, up, q, timeout)
+			msg, addr, err := p.exchangeOne(sub, st.cfg, up, q, timeout)
 			results <- res{msg: msg, up: addr, err: err}
 		}(up)
 	}
@@ -426,13 +465,13 @@ func (p *Pool) Exchange(ctx context.Context, q *dns.Msg, tags []string, timeout
 	for i := 0; i < len(chosen); i++ {
 		select {
 		case r := <-results:
-			if r.err == nil && r.msg != nil && !isTransportFailure(r.msg.Rcode) {
-				return r.msg, r.up, nil
-			}
-			if r.err != nil {
+			switch {
+			case r.err != nil:
 				lastErr = r.err
-			} else if r.msg != nil {
-				lastErr = fmt.Errorf("upstream %s returned %s", r.up, dns.RcodeToString[r.msg.Rcode])
+			case r.msg != nil && r.msg.Rcode != dns.RcodeServerFailure:
+				return r.msg, r.up, nil
+			default:
+				lastErr = fmt.Errorf("upstream %s returned SERVFAIL", r.up)
 			}
 		case <-ctx.Done():
 			return nil, "", ctx.Err()
@@ -444,8 +483,29 @@ func (p *Pool) Exchange(ctx context.Context, q *dns.Msg, tags []string, timeout
 	return nil, "", lastErr
 }
 
-func selectFrom(ups []*Upstream, policy string, maxAttempts int) []*Upstream {
-	// Filter out open breakers but keep everything if that empties the list.
+// restrict resolves an ACL group's upstream list to upstreams, keeping the
+// configured order and dropping names that match nothing. When nothing in the
+// list resolves the full pool is used, so a typo cannot black-hole a client.
+func (p *Pool) restrict(st *poolState, want []string) []*Upstream {
+	seen := make(map[*Upstream]bool, len(want))
+	out := make([]*Upstream, 0, len(want))
+	for _, name := range want {
+		for _, up := range st.byAddr[name] {
+			if !seen[up] {
+				seen[up] = true
+				out = append(out, up)
+			}
+		}
+	}
+	if len(out) == 0 {
+		return st.all()
+	}
+	return out
+}
+
+// pick filters out open breakers (keeping everything if that empties the
+// list) and applies the pool policy.
+func (p *Pool) pick(st *poolState, ups []*Upstream, maxAttempts int) []*Upstream {
 	open := make([]*Upstream, 0, len(ups))
 	for _, u := range ups {
 		if !u.BreakerOpen() {
@@ -458,11 +518,13 @@ func selectFrom(ups []*Upstream, policy string, maxAttempts int) []*Upstream {
 	if maxAttempts <= 0 || maxAttempts > len(open) {
 		maxAttempts = len(open)
 	}
-	switch policy {
+	switch st.cfg.Policy {
 	case "failover":
 		return open[:maxAttempts]
 	case "random":
-		rand.Shuffle(len(open), func(i, j int) { open[i], open[j] = open[j], open[i] })
+		p.rngMu.Lock()
+		defer p.rngMu.Unlock()
+		p.rng.Shuffle(len(open), func(i, j int) { open[i], open[j] = open[j], open[i] })
 		return open[:maxAttempts]
 	default:
 		return open
@@ -471,63 +533,54 @@ func selectFrom(ups []*Upstream, policy string, maxAttempts int) []*Upstream {
 
 // exchangeOne sends q to a single upstream, retrying over TCP when the UDP
 // answer comes back truncated (RFC 1035 §4.2.1).
-func (p *Pool) exchangeOne(ctx context.Context, up *Upstream, q *dns.Msg, timeout time.Duration) (*dns.Msg, string, error) {
+func (p *Pool) exchangeOne(ctx context.Context, cfg config.Upstream, up *Upstream, q *dns.Msg, timeout time.Duration) (*dns.Msg, string, error) {
 	if timeout <= 0 {
 		timeout = 2 * time.Second
 	}
 	client := p.clientFor(up)
-	msg := q.Copy()
-	start := time.Now()
-	resp, rtt, err := client.ExchangeContext(ctx, msg, up.addr)
-	up.Success(rtt)
+	resp, rtt, err := client.ExchangeContext(ctx, q.Copy(), up.addr)
 	if err != nil {
+		up.Failure(err, thresholdFor(up, cfg.BreakerThreshold), cfg.BreakerCooldown.D())
 		return nil, up.addr, err
 	}
-	_ = start
+	up.Success(rtt)
 	if !resp.Truncated {
 		return resp, up.addr, nil
 	}
-	// Retry over TCP to get the full answer. Some upstreams are configured
-	// UDP-only in which case this simply fails and we use the TC answer.
+	// Retry over TCP for the full answer. An upstream that is UDP-only just
+	// fails here and we return the truncated answer; the client retries over
+	// TCP itself.
 	tcpClient := &dns.Client{Net: "tcp", Timeout: timeout}
 	resp2, rtt2, err2 := tcpClient.ExchangeContext(ctx, q.Copy(), up.addr)
-	if err2 == nil && resp2 != nil && !resp2.Truncated {
-		up.Success(rtt2)
-		return resp2, up.addr, nil
+	if err2 != nil || resp2 == nil || resp2.Truncated {
+		if err2 == nil {
+			err2 = errors.New("still truncated over TCP")
+		}
+		up.Failure(fmt.Errorf("truncated answer, TCP retry failed: %w", err2),
+			thresholdFor(up, cfg.BreakerThreshold), cfg.BreakerCooldown.D())
+		return resp, up.addr, nil
 	}
-	up.Failure(fmt.Errorf("truncated answer, TCP retry failed: %w", err2), up.breakerThreshold(), p.cfg.BreakerCooldown.D())
-	// Return the truncated answer; the client will retry over TCP itself.
-	return resp, up.addr, nil
-}
-
-func isTransportFailure(rcode int) bool {
-	return rcode == dns.RcodeServerFailure
-}
-
-// breakerThreshold is the effective failure threshold for an upstream.
-func (u *Upstream) breakerThreshold() int {
-	if u.cfg.MaxFails > 0 {
-		return u.cfg.MaxFails
-	}
-	return 0
+	up.Success(rtt2)
+	return resp2, up.addr, nil
 }
 
 // HealthCheck probes every upstream with a cheap query, refreshing health
 // state even when no client traffic is flowing. It returns the number of
 // upstreams that answered.
 func (p *Pool) HealthCheck(ctx context.Context, timeout time.Duration) int {
+	st := p.state.Load()
 	probe := new(dns.Msg)
 	probe.SetQuestion(".", dns.TypeNS)
 	probe.RecursionDesired = true
 	ok := 0
-	for _, up := range p.ups {
+	for _, up := range st.all() {
 		if up.BreakerOpen() {
 			continue
 		}
 		c := p.clientFor(up)
 		_, rtt, err := c.ExchangeContext(ctx, probe, up.addr)
 		if err != nil {
-			up.Failure(err, p.breakerThresholdFor(up), p.cfg.BreakerCooldown.D())
+			up.Failure(err, thresholdFor(up, st.cfg.BreakerThreshold), st.cfg.BreakerCooldown.D())
 			continue
 		}
 		up.Success(rtt)
@@ -536,63 +589,47 @@ func (p *Pool) HealthCheck(ctx context.Context, timeout time.Duration) int {
 	return ok
 }
 
-func (p *Pool) breakerThresholdFor(u *Upstream) int {
-	if t := u.breakerThreshold(); t > 0 {
-		return t
+func thresholdFor(up *Upstream, def int) int {
+	if up.cfg.MaxFails > 0 {
+		return up.cfg.MaxFails
 	}
-	return p.cfg.BreakerThreshold
+	return def
 }
 
-// Rebind applies a new upstream configuration in place, keeping the live
-// health state of upstreams whose address is unchanged.
+// Rebind applies a new upstream configuration, keeping the live health state
+// of upstreams whose address and protocol are unchanged.
+//
+// It builds a whole new immutable state and swaps it in with a single atomic
+// store: queries already in flight finish against the state they started
+// with, and the next query sees the new one. There is no window in which half
+// the upstreams are old and half new.
 func (p *Pool) Rebind(cfg config.Upstream) {
 	p.mu.Lock()
 	defer p.mu.Unlock()
-	p.cfg = cfg
-	old := make(map[string]*Upstream, len(p.ups))
-	for _, u := range p.ups {
-		old[u.cfg.Addr+"/"+u.cfg.Proto] = u
+	old := p.state.Load()
+	next := buildState(cfg, old, true)
+	p.state.Store(next)
+	// Drop the memoised clients of upstreams that are gone; their shells are
+	// only reachable from in-flight queries, which let them go on their own.
+	live := make(map[*Upstream]bool, len(next.all()))
+	for _, u := range next.all() {
+		live[u] = true
 	}
-	next := make([]*Upstream, 0, len(cfg.Servers))
-	for _, s := range cfg.Servers {
-		if u, ok := old[s.Addr+"/"+s.Proto]; ok {
-			u.cfg = s
-			next = append(next, u)
-			delete(old, s.Addr+"/"+s.Proto)
-			continue
-		}
-		u := &Upstream{cfg: s}
-		u.setHealth(HealthUnknown)
-		next = append(next, u)
-	}
-	p.ups = next
-	// Drop clients for removed upstreams.
-	for u := range p.clients {
-		if old[u.cfg.Addr+"/"+u.cfg.Proto] != nil {
-			delete(p.clients, u)
+	for _, u := range old.all() {
+		if !live[u] {
+			p.clients.Delete(u)
 		}
 	}
-	p.resolveAllLocked()
 }
 
-func (p *Pool) resolveAllLocked() {
-	for _, b := range p.cfg.Bootstraps {
-		p.bootstrap[b.Name] = append([]string(nil), b.Addresses...)
-	}
-	for _, up := range p.ups {
-		host, port, err := net.SplitHostPort(up.cfg.Addr)
-		if err != nil {
-			continue
-		}
-		if net.ParseIP(host) != nil {
-			up.addr = up.cfg.Addr
-			continue
-		}
-		if addrs := p.bootstrap[host]; len(addrs) > 0 {
-			up.resolved = addrs
-			up.addr = net.JoinHostPort(addrs[0], port)
-			continue
-		}
-		up.addr = up.cfg.Addr
+// String renders the pool for logs.
+func (p *Pool) String() string {
+	ups := p.All()
+	addrs := make([]string, 0, len(ups))
+	for _, u := range ups {
+		addrs = append(addrs, u.addr)
 	}
+	return "pool[" + strings.Join(addrs, " ") + "]"
 }
+
+func ptr[T any](v T) *T { return &v }