When serve_stale is enabled, a cached SERVFAIL in ncache shadows a valid
positive entry in pcache because ncache is always checked first. SERVFAIL
is transient and should not mask a known-good answer.
When the ncache hit is a SERVFAIL, check pcache for a valid entry before
returning the SERVFAIL. NXDOMAIN and NODATA are unaffected and still
follow the existing ncache-first lookup per RFC 2308.
Fixes#7956
Signed-off-by: umut-polat <52835619+umut-polat@users.noreply.github.com>
This commit removes superfluous allocations of the Answer, Ns, and Extra
slices when copying a cached a dns.Msg. The allocations are superfluous
because we immediately overwrite the newly copied slices with
filterRRSlice. It also updates filterRRSlice to pre-calculate the size
of the slice being copied into.
Benchmark results:
goos: darwin
goarch: arm64
pkg: github.com/coredns/coredns/plugin/cache
cpu: Apple M4 Pro
│ base.10.txt │ new.10.txt │
│ sec/op │ sec/op vs base │
CacheResponse-14 471.1n ± 0% 462.9n ± 2% -1.74% (p=0.009 n=10)
│ base.10.txt │ new.10.txt │
│ B/op │ B/op vs base │
CacheResponse-14 672.0 ± 0% 656.0 ± 0% -2.38% (p=0.000 n=10)
│ base.10.txt │ new.10.txt │
│ allocs/op │ allocs/op vs base │
CacheResponse-14 13.00 ± 0% 12.00 ± 0% -7.69% (p=0.000 n=10)
Signed-off-by: Charlie Vieth <charlie.vieth@gmail.com>
Enable intrange linter to enforce modern Go range syntax over
traditional for loops, by converting:
for i := 0; i < n; i++
to:
for i := range n
Adding type conversions where needed for compatibility
with existing uint64 parameters.
Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
* plugin/cache: key cache on Checking Disabled (CD) bit
Key the cache on CD bit, which effectively separates the entries for
queries with CD disabled or enabled.
Signed-off-by: Grant Spence <gspence@redhat.com>
For responses synthesized by known wildcard records, publish metadata containing the wildcard record name
Signed-off-by: Chris O'Haver <cohaver@infoblox.com>
This PR adds an optional REFRESH_MODE parameter on the serve_stale configuration directive of the
cache plugin, which verifies that the upstream is still unavailable before returning stale entries.
Signed-off-by: Antoine Tollenaere <atollena@gmail.com>
The filtering of DNSSEC records in the cache plugin was not done
correctly. Also the change to introduced this bug didn't take into
account that the cache - by virtue of differentiating between DNSSEC and
no-DNSSEC - relied on not copying the data from the cache.
This change copies and then filters the data and factors the filtering
into a function that is used in two places (albeit with on ugly boolean
parameters to prevent copying things twice).
Add tests, do_test.go is moved to test/cache_test.go because the OPT
handing is done outside of the cache plugin. The core server re-attaches
the correct OPT when replying, so that makes for a better e2e test.
Added small unit test for filterRRslice and an explicit test that asks
for DNSSEC first and then plain, and vice versa to test cache behavior.
Fixes: #4146
Signed-off-by: Miek Gieben <miek@miek.nl>
* cache: default to DNSSEC
This change does away with the DNS/DNSSEC distinction the cache
currently makes. Cache will always make coredns perform a DNSSEC query
and store that result. If a client just needs plain DNS, the DNSSEC
records are stripped from the response.
It should also be more memory efficient, because we store a reply once
and not one DNS and another for DNSSEC.
Fixes: #3836
Signed-off-by: Miek Gieben <miek@miek.nl>
* Change OPT RR when one is present in the msg.
Signed-off-by: Miek Gieben <miek@miek.nl>
* Fix comment for isDNSSEC
Signed-off-by: Miek Gieben <miek@miek.nl>
* Update plugin/cache/handler.go
Co-authored-by: Chris O'Haver <cohaver@infoblox.com>
* Update plugin/cache/item.go
Co-authored-by: Chris O'Haver <cohaver@infoblox.com>
* Code review; fix comment for isDNSSEC
Signed-off-by: Miek Gieben <miek@miek.nl>
* Update doc and set AD to false
Set Authenticated Data to false when DNSSEC was not wanted. Also update
the readme with the new behavior.
Signed-off-by: Miek Gieben <miek@miek.nl>
* Update plugin/cache/handler.go
Co-authored-by: Chris O'Haver <cohaver@infoblox.com>
Co-authored-by: Chris O'Haver <cohaver@infoblox.com>
* Make request.Request smaller
This makes the request struct smaller and removes the pointer to the do
boolean (tri-bool) as size == 0 will indicate if we have cached it.
Family can be a int8 because it only carries 3 values, Size itself is
just a uint16 under the covers.
This is a more comprehensive fix than #3292Closes#3292
Signed-off-by: Miek Gieben <miek@miek.nl>
* cache: fix test
this now needs a valid response writter
Signed-off-by: Miek Gieben <miek@miek.nl>
* plugin/cache: remove item.Autoritative
Confuses clients if not set; remove it.
Signed-off-by: Miek Gieben <miek@miek.nl>
* Add extra comments on why we do this
Signed-off-by: Miek Gieben <miek@miek.nl>
* Stop importing testing in the main binary
Stop importing "testing" into the main binary:
* test/helpers.go imported it; remote that and change function signature
* update all tests that use this
Signed-off-by: Miek Gieben <miek@miek.nl>
* Drop import testing from metrics plugin
Signed-off-by: Miek Gieben <miek@miek.nl>
* more fiddling
Signed-off-by: Miek Gieben <miek@miek.nl>
Remove some optimization and lowercasing of the qname (in the end
miekg/dns should provide a fast and OK function for it).
* remove the make([]byte, 2) allocation in the key()
* use already lowercased qname in hash key calculation.
% benchcmp old.txt new.txt
benchmark old ns/op new ns/op delta
BenchmarkCacheResponse-4 9599 8735 -9.00%
Signed-off-by: Miek Gieben <miek@miek.nl>
Uppercase all these test errors as well. And extend the presubmit to
check for these in the future. Also do a slightly smarter grep to only
get t.<something>. as (because dump regexp) this also grep over non test
files.
* plugin/cache: per server metrics
Use per server metrics in the cache plugin as well. This required
some plumbing changes. Also use request.Request more.
* fix cherry-pick
* global: move to context
Move from golang.org/x/net/context to std lib's context.
Change done with:
for i in $(grep -l '/context' **/*.go); do sed -e 's|golang.org/x/net/context|context|' -i $i; echo $i; done
for i in **/*.go; do goimports -w $i; done
* drop from dns.pb.go as well
* Update all plugins to use plugin/pkg/log
I wish this could have been done with sed. Alas manually changed all
callers to use the new plugin/pkg/log package.
* Error -> Info
* Add docs to debug plugin as well
* Revert "pkg/typify: empty messages are OtherError (#1531)"
This reverts commit fc1d73ffa9.
* plugin/cache: add failsafeTTL
If we can not see what TTL we should put on a message to be cached, use
5 seconds as minimal TTL. We used to apply the maximum TTL to these
messages.
Messages with nothing in them are considered OtherError, they can not
serve any purpose for normal clients (i.e. dyn update or notifies might
have a use for them).
Also update a test in the cache plugin, so that we explicitaly test for
this case.
* Improve plugin/cache metrics
* Add coredns_cache_prefetch_total metric to track number of prefetches.
* Remove unnecessary Cache.get() call which would incorrectly increment
cache counters.
* Initialize all counters and gauges at zero.
* Allow prefetching of a single request per ttl
The original implementation didn't allow prefetching queries which are
only requested once during the duration of a TTL. The minimum amount of
queries which had to be seen was therefore capped at 2.
This change also implements a real prefetch test. The existing test was
a noop and always passed regardless of any prefetch implementation.
* Fix prefetching for items with a short TTL
The default prefetch threshold (percentage) is 10% of the lifetime of a
cache item. With the previous implementation, this disabled prefetching
for all items with a TTL < 10s (the resulting percentage would be 0, at
which point a cached item is already discarded).
This change uses a time based threshold calculation and ensures that
a prefetch is triggered at a TTL of 1 at the latest.
* Fix wrong duration reporting of cached responses
The logging and metrics plugins (among others) included the duration of
a cache prefetch in the request latency of client request. This change
fixes this wrong reporting and executes the prefetch request in a
goroutine in the background.
* Rename middleware to plugin
first pass; mostly used 'sed', few spots where I manually changed
text.
This still builds a coredns binary.
* fmt error
* Rename AddMiddleware to AddPlugin
* Readd AddMiddleware to remain backwards compat
