Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop serving stale responses from negative cache. #3736

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 47 additions & 0 deletions plugin/cache/cache_test.go
Expand Up @@ -250,6 +250,53 @@ func TestCacheZeroTTL(t *testing.T) {
}
}

func TestServeFromStaleNoNCache(t *testing.T) {
c := New()
c.staleUpTo = 1 * time.Hour
c.Next = ttlBackend(60)

req := new(dns.Msg)
req.SetQuestion("cached.org.", dns.TypeA)
ctx := context.TODO()

// Cache cached.org in ncache.
rec := dnstest.NewRecorder(&test.ResponseWriter{})
c.ServeDNS(ctx, rec, req)
if c.pcache.Len() != 1 {
t.Fatalf("Msg with > 0 TTL should have been cached")
}
p := c.pcache
c = New()
c.staleUpTo = 1 * time.Hour
c.Next = ttlBackend(60)
c.ncache = p

// No more backend resolutions, just from cache if available.
c.Next = plugin.HandlerFunc(func(context.Context, dns.ResponseWriter, *dns.Msg) (int, error) {
return 255, nil // Below, a 255 means we tried querying upstream.
})

tests := []struct {
name string
futureMinutes int
expectedResult int
}{
{"cached.org.", 30, 255},
{"cached.org.", 60, 255},
{"cached.org.", 70, 255},
}

for i, tt := range tests {
rec := dnstest.NewRecorder(&test.ResponseWriter{})
c.now = func() time.Time { return time.Now().Add(time.Duration(tt.futureMinutes) * time.Minute) }
r := req.Copy()
r.SetQuestion(tt.name, dns.TypeA)
if ret, _ := c.ServeDNS(ctx, rec, r); ret != tt.expectedResult {
t.Errorf("Test %d: expecting %v; got %v", i, tt.expectedResult, ret)
}
}
}

func TestServeFromStaleCache(t *testing.T) {
c := New()
c.Next = ttlBackend(60)
Expand Down
12 changes: 6 additions & 6 deletions plugin/cache/handler.go
Expand Up @@ -27,11 +27,11 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
server := metrics.WithServer(ctx)

ttl := 0
i := c.getIgnoreTTL(now, state, server)
i, okStale := c.getIgnoreTTL(now, state, server)
if i != nil {
ttl = i.ttl(now)
}
if i == nil || -ttl >= int(c.staleUpTo.Seconds()) {
if i == nil || (!okStale && ttl < 0) || -ttl >= int(c.staleUpTo.Seconds()) {
crr := &ResponseWriter{ResponseWriter: w, Cache: c, state: state, server: server}
return plugin.NextOrFailure(c.Name(), c.Next, ctx, crr, r)
}
Expand Down Expand Up @@ -97,25 +97,25 @@ func (c *Cache) get(now time.Time, state request.Request, server string) (*item,
}

// getIgnoreTTL unconditionally returns an item if it exists in the cache.
func (c *Cache) getIgnoreTTL(now time.Time, state request.Request, server string) *item {
func (c *Cache) getIgnoreTTL(now time.Time, state request.Request, server string) (*item, bool) {
k := hash(state.Name(), state.QType(), state.Do())

if i, ok := c.ncache.Get(k); ok {
ttl := i.(*item).ttl(now)
if ttl > 0 || (c.staleUpTo > 0 && -ttl < int(c.staleUpTo.Seconds())) {
cacheHits.WithLabelValues(server, Denial).Inc()
}
return i.(*item)
return i.(*item), false
}
if i, ok := c.pcache.Get(k); ok {
ttl := i.(*item).ttl(now)
if ttl > 0 || (c.staleUpTo > 0 && -ttl < int(c.staleUpTo.Seconds())) {
cacheHits.WithLabelValues(server, Success).Inc()
}
return i.(*item)
return i.(*item), true
}
cacheMisses.WithLabelValues(server).Inc()
return nil
return nil, false
}

func (c *Cache) exists(state request.Request) *item {
Expand Down