[PR #6513] [client] Resolve management cache domains asynchronously #25973

Open
opened 2026-08-05 07:06:42 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6513
Author: @pappz
Created: 6/22/2026
Status: 🔄 Open

Base: mainHead: fix/mgmt-cache-async-resolve


📝 Commits (6)

  • a7d85ff [client] Resolve management cache domains asynchronously
  • 37be881 [client] Move mgmt cache pending-resolve test helpers to export_test.go
  • b6c79f1 [client] Trim verbose comments in mgmt cache async resolve
  • 08ac485 [client] Scope mgmt cache initial resolve to server lifetime
  • 3236a4c [client] Cancel ServeDNS-path mgmt resolve on server Stop
  • df6e422 [client] Resolve cold mgmt cache domains synchronously before DNS takeover

📊 Changes

5 files changed (+234 additions, -50 deletions)

View changed files

client/internal/dns/mgmt/export_test.go (+23 -0)
📝 client/internal/dns/mgmt/mgmt.go (+169 -20)
📝 client/internal/dns/mgmt/mgmt_refresh_test.go (+11 -11)
📝 client/internal/dns/mgmt/mgmt_test.go (+22 -16)
📝 client/internal/dns/server.go (+9 -3)

📄 Description

UpdateFromServerDomains resolved the infrastructure domains (signal, relay, STUN, TURN) synchronously, in a serial loop with a 5s timeout per domain. It runs deep inside handleSync, under the engine syncMsgMux (and the DNS server mutex). When DNS is unhealthy each lookup waits out its full timeout, so the sync lock is held for many seconds — observed as a 16.6s wait for the lock by a signal handler in the field. That starves the signal/p2p processing that shares syncMsgMux, which in turn prevents the handshake that would make DNS healthy again: a self-reinforcing loop.

The cache cannot simply be skipped: it is a prerequisite for the relay connection. The relay dials its server by hostname, resolved through the NetBird DNS chain where this resolver sits on top (PriorityMgmtCache); on a miss it falls through to upstream — the dead path the cache exists to bypass.

Instead, record intent on sync and resolve on demand:

  • UpdateFromServerDomains now marks each requested domain pending and kicks off resolution in the background via a dedicated singleflight group (resolveGroup), then returns immediately. No DNS I/O, no timeout, no blocking under the lock.
  • ServeDNS, on a miss for a pending domain, waits on the in-flight resolve (joining the same singleflight flight, bounded by dnsTimeout) instead of falling through to the dead upstream. The wait now happens in the relay/DNS-serving goroutine, never under syncMsgMux.
  • UpdateServerConfig registers the cache handler from the requested domains (RequestedDomains) rather than the already-resolved ones, so the resolver owns the names before resolution completes and can intercept the lookup.

Background resolves use context.Background() so a fast-returning sync cannot cancel an in-flight resolve. WaitForPendingResolves lets tests (and any warm-cache path) wait for the background work to settle.

Cache semantics are otherwise unchanged: TTL, stale-while-revalidate refresh, backoff, and the flow-domain exclusion all stay.

Describe your changes

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • This change does not modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — OR I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See CONTRIBUTING.md.

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Bug Fixes

    • Prevented duplicate cold-cache DNS queries by deduplicating in-flight initial resolutions for the same domain.
    • Improved DNS cache-miss behavior for A/AAAA queries by waiting for pending initial resolutions before falling back upstream.
  • Refactor

    • Updated management-cache domain configuration to resolve newly needed domains asynchronously, without blocking the update flow.
    • Handler registration now includes newly requested cacheable infrastructure domains, even before records are fully populated.
  • Tests

    • Updated DNS management tests to use resolver context and to wait for pending resolutions before assertions.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/6513 **Author:** [@pappz](https://github.com/pappz) **Created:** 6/22/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/mgmt-cache-async-resolve` --- ### 📝 Commits (6) - [`a7d85ff`](https://github.com/netbirdio/netbird/commit/a7d85ff3ab88de9154cc1d41e200cfda8dab3bf9) [client] Resolve management cache domains asynchronously - [`37be881`](https://github.com/netbirdio/netbird/commit/37be8811a328cf7e42efa65524f5097a910d9d8a) [client] Move mgmt cache pending-resolve test helpers to export_test.go - [`b6c79f1`](https://github.com/netbirdio/netbird/commit/b6c79f1f7179e41f9a9d1496c0187786be53b96f) [client] Trim verbose comments in mgmt cache async resolve - [`08ac485`](https://github.com/netbirdio/netbird/commit/08ac4855f6feaa9739d94be4639fddfdc66acd6c) [client] Scope mgmt cache initial resolve to server lifetime - [`3236a4c`](https://github.com/netbirdio/netbird/commit/3236a4c7fd7a5a8907af4995b7f0b1f038fe2735) [client] Cancel ServeDNS-path mgmt resolve on server Stop - [`df6e422`](https://github.com/netbirdio/netbird/commit/df6e422e10a09b1aa61f177b3f7abf21183f7a7b) [client] Resolve cold mgmt cache domains synchronously before DNS takeover ### 📊 Changes **5 files changed** (+234 additions, -50 deletions) <details> <summary>View changed files</summary> ➕ `client/internal/dns/mgmt/export_test.go` (+23 -0) 📝 `client/internal/dns/mgmt/mgmt.go` (+169 -20) 📝 `client/internal/dns/mgmt/mgmt_refresh_test.go` (+11 -11) 📝 `client/internal/dns/mgmt/mgmt_test.go` (+22 -16) 📝 `client/internal/dns/server.go` (+9 -3) </details> ### 📄 Description UpdateFromServerDomains resolved the infrastructure domains (signal, relay, STUN, TURN) synchronously, in a serial loop with a 5s timeout per domain. It runs deep inside handleSync, under the engine syncMsgMux (and the DNS server mutex). When DNS is unhealthy each lookup waits out its full timeout, so the sync lock is held for many seconds — observed as a 16.6s wait for the lock by a signal handler in the field. That starves the signal/p2p processing that shares syncMsgMux, which in turn prevents the handshake that would make DNS healthy again: a self-reinforcing loop. The cache cannot simply be skipped: it is a prerequisite for the relay connection. The relay dials its server by hostname, resolved through the NetBird DNS chain where this resolver sits on top (PriorityMgmtCache); on a miss it falls through to upstream — the dead path the cache exists to bypass. Instead, record intent on sync and resolve on demand: - UpdateFromServerDomains now marks each requested domain pending and kicks off resolution in the background via a dedicated singleflight group (resolveGroup), then returns immediately. No DNS I/O, no timeout, no blocking under the lock. - ServeDNS, on a miss for a pending domain, waits on the in-flight resolve (joining the same singleflight flight, bounded by dnsTimeout) instead of falling through to the dead upstream. The wait now happens in the relay/DNS-serving goroutine, never under syncMsgMux. - UpdateServerConfig registers the cache handler from the requested domains (RequestedDomains) rather than the already-resolved ones, so the resolver owns the names before resolution completes and can intercept the lookup. Background resolves use context.Background() so a fast-returning sync cannot cancel an in-flight resolve. WaitForPendingResolves lets tests (and any warm-cache path) wait for the background work to settle. Cache semantics are otherwise unchanged: TTL, stale-while-revalidate refresh, backoff, and the flow-domain exclusion all stay. ## Describe your changes ## Issue ticket number and link ## Stack <!-- branch-stack --> ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) - [ ] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Prevented duplicate cold-cache DNS queries by deduplicating in-flight initial resolutions for the same domain. * Improved DNS cache-miss behavior for A/AAAA queries by waiting for pending initial resolutions before falling back upstream. * **Refactor** * Updated management-cache domain configuration to resolve newly needed domains asynchronously, without blocking the update flow. * Handler registration now includes newly requested cacheable infrastructure domains, even before records are fully populated. * **Tests** * Updated DNS management tests to use resolver context and to wait for pending resolutions before assertions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 07:06:42 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#25973