diff --git a/proxy/config.go b/proxy/config.go index a56a5043c..cba561d07 100644 --- a/proxy/config.go +++ b/proxy/config.go @@ -65,6 +65,19 @@ type Config struct { // from this handler, the proxy will not send any response to the client. RequestHandler Handler + // OnOptimisticRefresh is called, if not nil, once the optimistic cache has + // refreshed an expired entry in the background. dctx is the context of + // that refresh; its [DNSContext.QueryStatistics] describe the exchanges it + // performed. Implementations must not modify or retain dctx, and must not + // block. + // + // An optimistic cache hit is answered from the cache right away and the + // entry is refreshed in a separate goroutine, so the exchanges of that + // refresh never reach [Config.RequestHandler]. Without this callback there + // is no way to account for them, which biases any statistics collected from + // [DNSContext.QueryStatistics] towards cache misses. + OnOptimisticRefresh func(dctx *DNSContext) + // UpstreamConfig is a general set of DNS servers to forward requests to. UpstreamConfig *UpstreamConfig diff --git a/proxy/optimisticresolver.go b/proxy/optimisticresolver.go index 011ade64d..fcdcea2c1 100644 --- a/proxy/optimisticresolver.go +++ b/proxy/optimisticresolver.go @@ -19,6 +19,10 @@ type cachingResolver interface { // cacheResp caches the response from dctx. cacheResp(dctx *DNSContext) + + // reportRefresh reports that dctx has been resolved in the background, so + // that the exchanges it performed can be accounted for. + reportRefresh(dctx *DNSContext) } // type check @@ -62,6 +66,10 @@ func (s *optimisticResolver) resolveOnce(dctx *DNSContext, key []byte, l *slog.L l.Debug("resolving request for optimistic cache", slogutil.KeyError, err) } + // Report the refresh even when it failed, since its statistics describe the + // attempt either way. + s.cr.reportRefresh(dctx) + if ok { s.cr.cacheResp(dctx) } diff --git a/proxy/optimisticresolver_internal_test.go b/proxy/optimisticresolver_internal_test.go index 58cde8767..d2e98b1aa 100644 --- a/proxy/optimisticresolver_internal_test.go +++ b/proxy/optimisticresolver_internal_test.go @@ -15,6 +15,7 @@ import ( type testCachingResolver struct { onReplyFromUpstream func(dctx *DNSContext) (ok bool, err error) onCacheResp func(dctx *DNSContext) + onReportRefresh func(dctx *DNSContext) } // replyFromUpstream implements the cachingResolver interface for @@ -28,6 +29,14 @@ func (tcr *testCachingResolver) cacheResp(dctx *DNSContext) { tcr.onCacheResp(dctx) } +// reportRefresh implements the cachingResolver interface for +// *testCachingResolver. +func (tcr *testCachingResolver) reportRefresh(dctx *DNSContext) { + if tcr.onReportRefresh != nil { + tcr.onReportRefresh(dctx) + } +} + func TestOptimisticResolver_ResolveOnce(t *testing.T) { in, out := make(chan unit), make(chan unit) var timesResolved, timesSet int @@ -113,3 +122,57 @@ func TestOptimisticResolver_ResolveOnce_unsuccessful(t *testing.T) { assert.False(t, cached) }) } + +func TestOptimisticResolver_ResolveOnce_reportRefresh(t *testing.T) { + t.Parallel() + + testCases := []struct { + replyErr error + name string + replyOK bool + wantSet bool + }{{ + replyErr: nil, + name: "success", + replyOK: true, + wantSet: true, + }, { + replyErr: assert.AnError, + name: "failure", + replyOK: false, + wantSet: false, + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + var reported, set int + dctx := &DNSContext{} + + s := newOptimisticResolver(&testCachingResolver{ + onReplyFromUpstream: func(_ *DNSContext) (ok bool, err error) { + return tc.replyOK, tc.replyErr + }, + onCacheResp: func(_ *DNSContext) { set++ }, + onReportRefresh: func(got *DNSContext) { + assert.Same(t, dctx, got) + + reported++ + }, + }) + + s.resolveOnce(dctx, []byte("key"), slog.New(slog.DiscardHandler)) + + // The refresh is reported whether or not it succeeded, since its + // statistics describe the attempt either way. + assert.Equal(t, 1, reported) + + if tc.wantSet { + assert.Equal(t, 1, set) + } else { + assert.Equal(t, 0, set) + } + }) + } +} diff --git a/proxy/proxycache.go b/proxy/proxycache.go index f161d9b74..569b0c90d 100644 --- a/proxy/proxycache.go +++ b/proxy/proxycache.go @@ -76,6 +76,14 @@ func cloneIPNet(n *net.IPNet) (clone *net.IPNet) { } } +// reportRefresh implements the [cachingResolver] interface for *Proxy. It +// notifies [Config.OnOptimisticRefresh], if it is set. +func (p *Proxy) reportRefresh(d *DNSContext) { + if p.OnOptimisticRefresh != nil { + p.OnOptimisticRefresh(d) + } +} + // cacheResp stores the response from d in general or subnet cache. In case the // cache is present in d, it's used first. func (p *Proxy) cacheResp(d *DNSContext) {