* reverseproxy: reduce lock contention in dynamic SRV and A upstreams (#7946)
The shared SRV and A/AAAA caches held a global write lock across the DNS
lookup, so one slow or timing-out record blocked every unrelated
upstream source.
Lookups now run without the cache lock held, deduplicated per key with
singleflight. A per-key generation (stored on the cache entry, so it is
bounded by the cache cap) and a cache-wide epoch keep a lookup that was
in flight during ResetSRV/ResetAllSRV from writing its result back, and
are part of the singleflight key so callers after a reset do not join a
pre-reset lookup.
The shared lookup is detached from the request that started it and
bounded by its own 30s timeout; each caller still returns as soon as its
own context is done. Distinct lookups across both caches are capped by a
shared gate (64 running, 256 queued); beyond that they are shed, serving
a stale SRV entry if a grace period is set and one is cached.
SRV and A now share one cache implementation. AUpstreams also implements
CachingUpstreamSource (via new ResetA/ResetAllA), so retries purge its
cache entry like SRVUpstreams already did.
Rebased onto the move of this code into modules/internal/network (#7790,
#8252).
Fixes#7946
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* network: backdate cache entries in TestShedLookups instead of a 1ns refresh
On Windows two close time.Now() calls can report no elapsed time, so an
entry seeded with a 1ns refresh could still be fresh and skip the shed
path entirely.
* network: move the shared cache and lookup gate into cache.go
dynamicupstreams.go keeps the SRV/A API; the generic cache, its
singleflight/generation handling and the lookup gate are not specific to
either record type. No functional change.
* network: preserve cache reset and grace semantics
---------
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Co-authored-by: steadytao <mail@steadytao.com>
* reverseproxy: extract DNS SRV+A discovery into a reusable package
Move the SRV and A/AAAA resolution + caching logic out of reverseproxy
into a new, transport-neutral package (dynamicupstreams) that returns
neutral targets instead of *reverseproxy.Upstream. reverseproxy's
SRVUpstreams and AUpstreams now delegate to it and build their own
upstreams from the targets; behavior is unchanged.
This lets other proxies (e.g. third-party layer4 proxies) reuse the same
DNS discovery + caching instead of copying it, reducing duplication and
maintenance burden. reverseproxy/upstreams.go shrinks substantially.
RFC for the de-duplication discussed in caddyserver/caddy-l4#429.
* test(dynamicupstreams): cover ResetAllSRV, filtered records, cache bound, concurrent refresh
Brings the package to 96.8% statement coverage. Adds:
- TestResetAllSRV: full-cache drop
- TestSRVFilteredRecords: LookupSRV partial-error semantics (usable
records returned, error downgraded to a warning)
- TestSRVCacheBound / TestACacheBound: eviction keeps the cache bounded
at maxCacheEntries
- TestSRVConcurrentRefreshDeduplicates / TestAConcurrentRefreshDeduplicates:
two goroutines missing the read-lock fast path trigger only one lookup
(verified under -race)
The grace-period and filtered-records logs now use a level-enabled logger
so the log-write branches execute. The only uncovered statements are the
two double-checked-locking re-check returns, which are reachable solely in
a narrow race window (the same untested idiom as reverseproxy's SRV/A).
* reverseproxy: move DNS SRV+A discovery into modules/internal/network
Per review feedback, the shared discovery code does not belong in a
top-level package: it is internal helper code, not part of Caddy's
runtime surface. Move it from the top-level dynamicupstreams package into
the existing modules/internal/network package and update reverse_proxy's
upstreams.go to consume it from there.
Being under modules/internal means third-party modules (e.g. layer4)
cannot import it yet; they keep their own copy until layer4 is part of
Caddy's standard distribution, at which point they can switch over.
* test(dynamicupstreams): correct the concurrent-refresh test's claim
The two concurrent-refresh tests documented themselves as covering the
write-lock re-check branch ("the second re-checks under the write lock"),
but they do not: while the first goroutine holds the write lock the second
cannot acquire the read lock either, so by the time it acquires srvMu the
cache is already populated and it is served by the read-lock fast path. The
50ms sleep was presented as letting the second goroutine "queue on the write
lock" but proved nothing.
Make the comments and failure messages honest: these tests assert
deduplication (two concurrent callers for one uncached key share a single
lookup) and, under -race, race-safety on the shared cache -- not a specific
locking branch. Ordering is enforced by the channels, so the sleep is removed.
No production code change.
* reverseproxy: preserve DNS discovery logging and grace behavior
Keep refresh and discovery logs in the DNS lookup path, retaining the original SRV field types. Preserve error suppression when an SRV lookup fails with grace enabled before any result has been cached.
Add regression tests for refresh-only logging and cold-cache grace handling.
AI-assisted-by: OpenAI Codex (GPT-6)
---------
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Preallocate the sources slice in MultiUpstreams.Provision and the
netAddrs slice in UpstreamResolver.ParseAddresses now that their final
lengths are known, avoiding the intermediate append regrowth
allocations. For a 4-address resolver this reduces ParseAddresses from
11 to 9 allocs/op and 528 to 384 B/op; the benefit scales with the
number of sources/addresses.
| before | after |
| sec/op | sec/op vs base |
UpstreamResolverParseAddresses-8 | 2.289µ |1.946µ ~ (p=0.165) noisy |
B/op: 528 → 384 -27.27% (p=0.000)
allocs/op: 11 → 9 -18.18% (p=0.000)
Adds BenchmarkUpstreamResolverParseAddresses as per policy.
* reverseproxy: Add ability to clear dynamic upstreams cache during retries
This is an optional interface for dynamic upstream modules to implement if they cache results.
TODO: More documentation; this is an experiment.
* Add some godoc
* Export interface; update godoc
fix a nil pointer dereference in AUpstreams.GetUpstreams when AUpstreams.Versions is not set (fixes caddyserver#5809)
Signed-off-by: Pascal Vorwerk <info@fossores.de>
* use gofmput to format code
* use gci to format imports
* reconfigure gci
* linter autofixes
* rearrange imports a little
* export GOOS=windows golangci-lint run ./... --fix
Ideally I'd just remove the parameter to caddy.Context.Logger(), but
this would break most Caddy plugins.
Instead, I'm making it variadic and marking it as partially deprecated.
In the future, I might completely remove the parameter once most
plugins have updated.
This allows users to, for example, get upstreams from multiple SRV
endpoints in order (such as primary and secondary clusters).
Also, gofmt went to town on the comments, sigh
* reverseproxy: Begin refactor to enable dynamic upstreams
Streamed here: https://www.youtube.com/watch?v=hj7yzXb11jU
* Implement SRV and A/AAA upstream sources
Also get upstreams at every retry loop iteration instead of just once
before the loop. See #4442.
* Minor tweaks from review
* Limit size of upstreams caches
* Add doc notes deprecating LookupSRV
* Provision dynamic upstreams
Still WIP, preparing to preserve health checker functionality
* Rejigger health checks
Move active health check results into handler-specific Upstreams.
Improve documentation regarding health checks and upstreams.
* Deprecation notice
* Add Caddyfile support, use `caddy.Duration`
* Interface guards
* Implement custom resolvers, add resolvers to http transport Caddyfile
* SRV: fix Caddyfile `name` inline arg, remove proto condition
* Use pointer receiver
* Add debug logs
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
tannevaledandZen Dodd
Julien Voisin
Matt Holt
WeidiDeng
Mohammed Al Sahaf
Kévin Dunglas
Aziz Rmadi
Pascal Vorwerk
Jacob Gadikian
Omar Ramadan
Emily LangeandFrancis Lavoie
Abirdcfly