Skip to content

Commit

Permalink
Introduce a caching DNS resolver (#540)
Browse files Browse the repository at this point in the history
There is no cache aging. If a resolution is successful, then we
do cache the result. This is meant as a mechanism to make measurements
faster and with less data points. For this reason it's disabled by
default. Consider for example the case where `http://www.example.com`
redirects you to `https://www.example.com`. You really don't want
to perform the name resolution again in this case.

As a side note, depending on the resolver we're using, we may not
need the caching mechanism. Nonetheless, using the cache always when
measuring also reduces the number of duplicate entries we produce
where only the first entry has actually been resolved.

This was developed as part of #509
  • Loading branch information
bassosimone authored Apr 22, 2020
1 parent 7c5a269 commit 7027883
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 2 deletions.
8 changes: 6 additions & 2 deletions netx/httptransport/httptransport.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ type Resolver interface {
// field of Config is nil/empty, we will use a suitable default.
type Config struct {
BogonIsError bool // default: bogon is not error
ByteCounter *bytecounter.Counter // default: no byte counting
ContextByteCounting bool // default: no context byte counting
ByteCounter *bytecounter.Counter // default: no explicit byte counting
CacheResolutions bool // default: no caching
ContextByteCounting bool // default: no implicit byte counting
Dialer Dialer // default: dialer.DNSDialer
Logger Logger // default: no logging
ProxyURL *url.URL // default: no proxy
Expand Down Expand Up @@ -71,6 +72,9 @@ func New(config Config) RoundTripper {
if config.Saver != nil {
r = resolver.SaverResolver{Resolver: r, Saver: config.Saver}
}
if config.CacheResolutions {
r = &resolver.CacheResolver{Resolver: r}
}
config.Resolver = r
}
if config.Dialer == nil {
Expand Down
1 change: 1 addition & 0 deletions netx/httptransport/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func TestIntegrationSuccess(t *testing.T) {
txp := httptransport.New(httptransport.Config{
BogonIsError: true,
ByteCounter: counter,
CacheResolutions: true,
ContextByteCounting: true,
Logger: log.Log,
Saver: saver,
Expand Down
44 changes: 44 additions & 0 deletions netx/resolver/cache.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package resolver

import (
"context"
"sync"
)

// CacheResolver is a resolver that caches successful replies.
type CacheResolver struct {
Resolver
mu sync.Mutex
cache map[string][]string
}

// LookupHost implements Resolver.LookupHost
func (r *CacheResolver) LookupHost(
ctx context.Context, hostname string) ([]string, error) {
if entry := r.Get(hostname); entry != nil {
return entry, nil
}
entry, err := r.Resolver.LookupHost(ctx, hostname)
if err != nil {
return nil, err
}
r.Set(hostname, entry)
return entry, nil
}

// Get gets the currently configured entry for domain, or nil
func (r *CacheResolver) Get(domain string) []string {
r.mu.Lock()
defer r.mu.Unlock()
return r.cache[domain]
}

// Set allows to pre-populate the cache
func (r *CacheResolver) Set(domain string, addresses []string) {
r.mu.Lock()
if r.cache == nil {
r.cache = make(map[string][]string)
}
r.cache[domain] = addresses
r.mu.Unlock()
}
53 changes: 53 additions & 0 deletions netx/resolver/cache_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package resolver_test

import (
"context"
"errors"
"testing"

"github.com/ooni/probe-engine/netx/resolver"
)

func TestUnitCacheFailure(t *testing.T) {
expected := errors.New("mocked error")
var r resolver.Resolver = resolver.FakeResolver{
Err: expected,
}
r = &resolver.CacheResolver{Resolver: r}
addrs, err := r.LookupHost(context.Background(), "www.google.com")
if !errors.Is(err, expected) {
t.Fatal("not the error we expected")
}
if addrs != nil {
t.Fatal("expected nil addrs here")
}
}

func TestUnitCacheHitSuccess(t *testing.T) {
var r resolver.Resolver = resolver.FakeResolver{
Err: errors.New("mocked error"),
}
cache := &resolver.CacheResolver{Resolver: r}
cache.Set("dns.google.com", []string{"8.8.8.8"})
addrs, err := cache.LookupHost(context.Background(), "dns.google.com")
if err != nil {
t.Fatal(err)
}
if len(addrs) != 1 || addrs[0] != "8.8.8.8" {
t.Fatal("not the result we expected")
}
}

func TestUnitCacheMissSuccess(t *testing.T) {
var r resolver.Resolver = resolver.FakeResolver{
Result: []string{"8.8.8.8"},
}
r = &resolver.CacheResolver{Resolver: r}
addrs, err := r.LookupHost(context.Background(), "dns.google.com")
if err != nil {
t.Fatal(err)
}
if len(addrs) != 1 || addrs[0] != "8.8.8.8" {
t.Fatal("not the result we expected")
}
}

0 comments on commit 7027883

Please sign in to comment.