plugin/forward: remove dynamic read timeout (#2319)

* plugin/forward: remove dynamic read timeout

We care about an upstream being there, so we still have a dynamic dial
time out (by way higher then 200ms) of 1s; this should be fairly stable
for an upstream. The read timeout if more variable because of cached and
non cached responses. As such remove his logic entirely.

Drop to 2s read timeout.

Fixes #2306

Signed-off-by: Miek Gieben <miek@miek.nl>

plugin/forward/connect.go

@@ -69,14 +69,6 @@ func (t *Transport) Dial(proto string) (*dns.Conn, bool, error) {
 	return conn, false, err
 }
 
-func (p *Proxy) readTimeout() time.Duration {
-	return limitTimeout(&p.avgRtt, minTimeout, maxTimeout)
-}
-
-func (p *Proxy) updateRtt(newRtt time.Duration) {
-	averageTimeout(&p.avgRtt, newRtt, cumulativeAvgWeight)
-}
-
 // Connect selects an upstream, sends the request and waits for a response.
 func (p *Proxy) Connect(ctx context.Context, state request.Request, opts options) (*dns.Msg, error) {
 	start := time.Now()
@@ -103,7 +95,6 @@ func (p *Proxy) Connect(ctx context.Context, state request.Request, opts options
 	}
 
 	conn.SetWriteDeadline(time.Now().Add(maxTimeout))
-	reqTime := time.Now()
 	if err := conn.WriteMsg(state.Req); err != nil {
 		conn.Close() // not giving it back
 		if err == io.EOF && cached {
@@ -112,10 +103,9 @@ func (p *Proxy) Connect(ctx context.Context, state request.Request, opts options
 		return nil, err
 	}
 
-	conn.SetReadDeadline(time.Now().Add(p.readTimeout()))
+	conn.SetReadDeadline(time.Now().Add(readTimeout))
 	ret, err := conn.ReadMsg()
 	if err != nil {
-		p.updateRtt(maxTimeout)
 		conn.Close() // not giving it back
 		if err == io.EOF && cached {
 			return nil, ErrCachedClosed
@@ -123,8 +113,6 @@ func (p *Proxy) Connect(ctx context.Context, state request.Request, opts options
 		return ret, err
 	}
 
-	p.updateRtt(time.Since(reqTime))
-
 	p.transport.Yield(conn)
 
 	rc, ok := dns.RcodeToString[ret.Rcode]

plugin/forward/health_test.go

@@ -144,9 +144,10 @@ func TestHealthMaxFails(t *testing.T) {
 
 	f.ServeDNS(context.TODO(), &test.ResponseWriter{}, req)
 
-	time.Sleep(1 * time.Second)
+	time.Sleep(readTimeout + 1*time.Second)
+	fails := atomic.LoadUint32(&p.fails)
 	if !p.Down(f.maxfails) {
-		t.Errorf("Expected Proxy fails to be greater than %d, got %d", f.maxfails, p.fails)
+		t.Errorf("Expected Proxy fails to be greater than %d, got %d", f.maxfails, fails)
 	}
 }
 

plugin/forward/persistent.go

@@ -31,7 +31,7 @@ type Transport struct {
 
 func newTransport(addr string) *Transport {
 	t := &Transport{
-		avgDialTime: int64(defaultDialTimeout / 2),
+		avgDialTime: int64(maxDialTimeout / 2),
 		conns:       make(map[string][]*persistConn),
 		expire:      defaultExpire,
 		addr:        addr,
@@ -159,8 +159,10 @@ func (t *Transport) SetExpire(expire time.Duration) { t.expire = expire }
 func (t *Transport) SetTLSConfig(cfg *tls.Config) { t.tlsConfig = cfg }
 
 const (
-	defaultExpire      = 10 * time.Second
+	defaultExpire  = 10 * time.Second
-	minDialTimeout     = 100 * time.Millisecond
+	minDialTimeout = 1 * time.Second
-	maxDialTimeout     = 30 * time.Second
+	maxDialTimeout = 30 * time.Second
-	defaultDialTimeout = 30 * time.Second
+
+	// Some resolves might take quite a while, usually (cached) responses are fast. Set to 2s to give us some time to retry a different upstream.
+	readTimeout = 2 * time.Second
 )

plugin/forward/persistent_test.go

@@ -140,9 +140,9 @@ func TestCleanupAll(t *testing.T) {
 
 	tr := newTransport(s.Addr)
 
-	c1, _ := dns.DialTimeout("udp", tr.addr, defaultDialTimeout)
+	c1, _ := dns.DialTimeout("udp", tr.addr, maxDialTimeout)
-	c2, _ := dns.DialTimeout("udp", tr.addr, defaultDialTimeout)
+	c2, _ := dns.DialTimeout("udp", tr.addr, maxDialTimeout)
-	c3, _ := dns.DialTimeout("udp", tr.addr, defaultDialTimeout)
+	c3, _ := dns.DialTimeout("udp", tr.addr, maxDialTimeout)
 
 	tr.conns["udp"] = []*persistConn{
 		{c1, time.Now()},

plugin/forward/proxy.go

@@ -11,8 +11,7 @@ import (
 
 // Proxy defines an upstream host.
 type Proxy struct {
-	avgRtt int64
+	fails uint32
-	fails  uint32
 
 	addr string
 
@@ -32,7 +31,6 @@ func NewProxy(addr, trans string) *Proxy {
 		fails:     0,
 		probe:     up.New(),
 		transport: newTransport(addr),
-		avgRtt:    int64(maxTimeout / 2),
 	}
 	p.health = NewHealthChecker(trans)
 	runtime.SetFinalizer(p, (*Proxy).finalizer)