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

Add minimum cache TTL for successful DNS responses #18986

Merged
merged 12 commits into from
Aug 10, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions libbeat/processors/dns/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ func (r ptrRecord) IsExpired(now time.Time) bool {

type ptrCache struct {
sync.RWMutex
data map[string]ptrRecord
maxSize int
data map[string]ptrRecord
maxSize int
minSuccessTTL time.Duration
}

func (c *ptrCache) set(now time.Time, key string, ptr *PTR) {
Expand All @@ -49,7 +50,7 @@ func (c *ptrCache) set(now time.Time, key string, ptr *PTR) {

c.data[key] = ptrRecord{
host: ptr.Host,
expires: now.Add(time.Duration(ptr.TTL) * time.Second),
expires: now.Add(maxDuration(time.Duration(ptr.TTL)*time.Second, c.minSuccessTTL)),
}
}

Expand Down Expand Up @@ -135,11 +136,12 @@ func (ce *cachedError) Cause() error { return ce.err }
// reverse DNS queries. It caches the results of queries regardless of their
// outcome (success or failure).
type PTRLookupCache struct {
success *ptrCache
failure *failureCache
failureTTL time.Duration
resolver PTRResolver
stats cacheStats
success *ptrCache
minSuccessTTL time.Duration
failure *failureCache
failureTTL time.Duration
resolver PTRResolver
stats cacheStats
}

type cacheStats struct {
Expand All @@ -155,8 +157,9 @@ func NewPTRLookupCache(reg *monitoring.Registry, conf CacheConfig, resolver PTRR

c := &PTRLookupCache{
success: &ptrCache{
data: make(map[string]ptrRecord, conf.SuccessCache.InitialCapacity),
maxSize: conf.SuccessCache.MaxCapacity,
data: make(map[string]ptrRecord, conf.SuccessCache.InitialCapacity),
maxSize: conf.SuccessCache.MaxCapacity,
minSuccessTTL: conf.SuccessCache.MinTTL,
},
failure: &failureCache{
data: make(map[string]failureRecord, conf.FailureCache.InitialCapacity),
Expand Down Expand Up @@ -208,3 +211,10 @@ func max(a, b int) int {
}
return b
}

func maxDuration(a time.Duration, b time.Duration) time.Duration {
marc-gr marked this conversation as resolved.
Show resolved Hide resolved
if a >= b {
return a
}
return b
}
25 changes: 21 additions & 4 deletions libbeat/processors/dns/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ package dns

import (
"io"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"

Expand All @@ -30,12 +30,14 @@ import (
type stubResolver struct{}

func (r *stubResolver) LookupPTR(ip string) (*PTR, error) {
if ip == gatewayIP {
switch ip {
case gatewayIP:
return &PTR{Host: gatewayName, TTL: gatewayTTL}, nil
} else if strings.HasSuffix(ip, "11") {
case gatewayIP + "1":
return nil, io.ErrUnexpectedEOF
case gatewayIP + "2":
return &PTR{Host: gatewayName, TTL: 0}, nil
}

return nil, &dnsError{"fake lookup returned NXDOMAIN"}
}

Expand Down Expand Up @@ -98,4 +100,19 @@ func TestCache(t *testing.T) {
assert.EqualValues(t, 3, c.stats.Hit.Get())
assert.EqualValues(t, 3, c.stats.Miss.Get()) // Cache miss.
}

// Cache returned TTL=0 with MinTTL.
ptr, err = c.LookupPTR(gatewayIP + "2")
if assert.NoError(t, err) {
assert.EqualValues(t, gatewayName, ptr.Host)
// TTL counts down while in cache.
marc-gr marked this conversation as resolved.
Show resolved Hide resolved
marc-gr marked this conversation as resolved.
Show resolved Hide resolved
assert.EqualValues(t, 0, ptr.TTL)
assert.EqualValues(t, 3, c.stats.Hit.Get())
assert.EqualValues(t, 4, c.stats.Miss.Get())

minTTL := defaultConfig.CacheConfig.SuccessCache.MinTTL
expectedExpire := time.Now().Add(minTTL).Unix()
gotExpire := c.success.data[gatewayIP+"2"].expires.Unix()
assert.InDelta(t, expectedExpire, gotExpire, 1)
}
}
9 changes: 8 additions & 1 deletion libbeat/processors/dns/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,12 @@ type CacheConfig struct {
// CacheSettings define the caching behavior for an individual cache.
type CacheSettings struct {
// TTL value for items in cache. Not used for success because we use TTL
// from the DNS record.
// from the DNS record or the minimum configured TTL.
marc-gr marked this conversation as resolved.
Show resolved Hide resolved
TTL time.Duration `config:"ttl"`

// Minimum TTL value for successful DNS responses.
MinTTL time.Duration `config:"ttl.min" validate:"min=1"`
marc-gr marked this conversation as resolved.
Show resolved Hide resolved

// Initial capacity. How much space is allocated at initialization.
InitialCapacity int `config:"capacity.initial" validate:"min=0"`

Expand Down Expand Up @@ -122,6 +125,9 @@ func (c *Config) Validate() error {

// Validate validates the data contained in the CacheConfig.
func (c *CacheConfig) Validate() error {
if c.SuccessCache.MinTTL <= 0 {
return errors.Errorf("success_cache.ttl.min must be > 0")
}
if c.FailureCache.TTL <= 0 {
return errors.Errorf("failure_cache.ttl must be > 0")
}
Expand All @@ -146,6 +152,7 @@ func (c *CacheConfig) Validate() error {
var defaultConfig = Config{
CacheConfig: CacheConfig{
SuccessCache: CacheSettings{
MinTTL: time.Minute,
InitialCapacity: 1000,
MaxCapacity: 10000,
},
Expand Down