From a1d92c51cd93a83dd2936f4b857b155d5d321a26 Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Tue, 20 Nov 2018 08:48:56 +0100 Subject: [PATCH] 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 --- plugin/forward/connect.go | 14 +------------- plugin/forward/health_test.go | 5 +++-- plugin/forward/persistent.go | 12 +++++++----- plugin/forward/persistent_test.go | 6 +++--- plugin/forward/proxy.go | 4 +--- 5 files changed, 15 insertions(+), 26 deletions(-) diff --git a/plugin/forward/connect.go b/plugin/forward/connect.go index 64edb395e..55deeae1e 100644 --- a/plugin/forward/connect.go +++ b/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] diff --git a/plugin/forward/health_test.go b/plugin/forward/health_test.go index 59a0dde13..3d06f2835 100644 --- a/plugin/forward/health_test.go +++ b/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) } } diff --git a/plugin/forward/persistent.go b/plugin/forward/persistent.go index fabdc70b2..d1348f94d 100644 --- a/plugin/forward/persistent.go +++ b/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 - minDialTimeout = 100 * time.Millisecond - maxDialTimeout = 30 * time.Second - defaultDialTimeout = 30 * time.Second + defaultExpire = 10 * time.Second + minDialTimeout = 1 * time.Second + maxDialTimeout = 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 ) diff --git a/plugin/forward/persistent_test.go b/plugin/forward/persistent_test.go index 271a80c0b..f1c906076 100644 --- a/plugin/forward/persistent_test.go +++ b/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) - c2, _ := dns.DialTimeout("udp", tr.addr, defaultDialTimeout) - c3, _ := dns.DialTimeout("udp", tr.addr, defaultDialTimeout) + c1, _ := dns.DialTimeout("udp", tr.addr, maxDialTimeout) + c2, _ := dns.DialTimeout("udp", tr.addr, maxDialTimeout) + c3, _ := dns.DialTimeout("udp", tr.addr, maxDialTimeout) tr.conns["udp"] = []*persistConn{ {c1, time.Now()}, diff --git a/plugin/forward/proxy.go b/plugin/forward/proxy.go index fbe79cd69..bf4d68dca 100644 --- a/plugin/forward/proxy.go +++ b/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)