Do Compress only when need in request.Scrub (#1760)

* Remove Compress by default

Set Compress = true in Scrub only when the message doesn not fit the
advertized buffer. Doing compression is expensive, so try to avoid it.

Master vs this branch
pkg: github.com/coredns/coredns/plugin/cache
BenchmarkCacheResponse-2   	   50000	     24774 ns/op

pkg: github.com/coredns/coredns/plugin/cache
BenchmarkCacheResponse-2   	  100000	     21960 ns/op

* and make it compile

plugin/auto/auto.go

@@ -70,7 +70,7 @@ func (a Auto) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 
 	m := new(dns.Msg)
 	m.SetReply(r)
-	m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true
+	m.Authoritative, m.RecursionAvailable = true, true
 	m.Answer, m.Ns, m.Extra = answer, ns, extra
 
 	switch result {

plugin/backend_lookup.go

@@ -383,7 +383,7 @@ func SOA(b ServiceBackend, zone string, state request.Request, opt Options) ([]d
 func BackendError(b ServiceBackend, zone string, rcode int, state request.Request, err error, opt Options) (int, error) {
 	m := new(dns.Msg)
 	m.SetRcode(state.Req, rcode)
-	m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true
+	m.Authoritative, m.RecursionAvailable = true, true
 	m.Ns, _ = SOA(b, zone, state, opt)
 
 	state.SizeAndDo(m)

plugin/cache/item.go

@@ -61,7 +61,6 @@ func (i *item) toMsg(m *dns.Msg, now time.Time) *dns.Msg {
 	m1.AuthenticatedData = i.AuthenticatedData
 	m1.RecursionAvailable = i.RecursionAvailable
 	m1.Rcode = i.Rcode
-	m1.Compress = true
 
 	m1.Answer = make([]dns.RR, len(i.Answer))
 	m1.Ns = make([]dns.RR, len(i.Ns))

plugin/erratic/erratic.go

@@ -45,7 +45,6 @@ func (e *Erratic) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg
 
 	m := new(dns.Msg)
 	m.SetReply(r)
-	m.Compress = true
 	m.Authoritative = true
 	if trunc {
 		m.Truncated = true

plugin/etcd/handler.go

@@ -84,10 +84,11 @@ func (e *Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (
 
 	m := new(dns.Msg)
 	m.SetReply(r)
-	m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true
+	m.Authoritative, m.RecursionAvailable = true, true
 	m.Answer = append(m.Answer, records...)
 	m.Extra = append(m.Extra, extra...)
 
+	// TODO(miek): get rid of this by not adding dups in the first place, dnsutil.Append()?
 	m = dnsutil.Dedup(m)
 	state.SizeAndDo(m)
 	m, _ = state.Scrub(m)

plugin/etcd/stub_handler.go

@@ -32,8 +32,9 @@ func (s Stub) ServeDNS(ctx context.Context, w dns.ResponseWriter, req *dns.Msg)
 	if e != nil {
 		return dns.RcodeServerFailure, e
 	}
-	m.RecursionAvailable, m.Compress = true, true
+	m.RecursionAvailable = true
 	state.SizeAndDo(m)
+	m, _ = state.Scrub(m)
 	w.WriteMsg(m)
 	return dns.RcodeSuccess, nil
 }

plugin/federation/federation.go

@@ -103,7 +103,7 @@ func (f *Federation) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.
 
 	m := new(dns.Msg)
 	m.SetReply(r)
-	m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true
+	m.Authoritative, m.RecursionAvailable = true, true
 
 	m.Answer = []dns.RR{service.NewCNAME(state.QName(), service.Host)}
 

plugin/file/file.go

@@ -50,7 +50,7 @@ func (f File) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 		if z.isNotify(state) {
 			m := new(dns.Msg)
 			m.SetReply(r)
-			m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true
+			m.Authoritative, m.RecursionAvailable = true, true
 			state.SizeAndDo(m)
 			w.WriteMsg(m)
 
@@ -84,7 +84,7 @@ func (f File) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 
 	m := new(dns.Msg)
 	m.SetReply(r)
-	m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true
+	m.Authoritative, m.RecursionAvailable = true, true
 	m.Answer, m.Ns, m.Extra = answer, ns, extra
 
 	switch result {

plugin/forward/forward.go

@@ -136,7 +136,6 @@ func (f *Forward) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg
 			return 0, nil
 		}
 
-		ret.Compress = true
 		// When using force_tcp the upstream can send a message that is too big for
 		// the udp buffer, hence we need to truncate the message to at least make it
 		// fit the udp buffer.

plugin/hosts/hosts.go

@@ -63,7 +63,7 @@ func (h Hosts) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (
 
 	m := new(dns.Msg)
 	m.SetReply(r)
-	m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true
+	m.Authoritative, m.RecursionAvailable = true, true
 	m.Answer = answers
 
 	state.SizeAndDo(m)

plugin/kubernetes/handler.go

@@ -17,7 +17,7 @@ func (k Kubernetes) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.M
 
 	m := new(dns.Msg)
 	m.SetReply(r)
-	m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true
+	m.Authoritative, m.RecursionAvailable = true, true
 
 	zone := plugin.Zones(k.Zones).Matches(state.Name())
 	if zone == "" {
@@ -79,6 +79,7 @@ func (k Kubernetes) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.M
 	m.Answer = append(m.Answer, records...)
 	m.Extra = append(m.Extra, extra...)
 
+	// TODO(miek): get rid of this by not adding dups in the first place, dnsutil.Append()?
 	m = dnsutil.Dedup(m)
 
 	state.SizeAndDo(m)

plugin/proxy/dns.go

@@ -63,7 +63,6 @@ func (d *dnsEx) Exchange(ctx context.Context, addr string, state request.Request
 	if err != nil {
 		return nil, err
 	}
-	reply.Compress = true
 	reply.Id = state.Req.Id
 	// When using force_tcp the upstream can send a message that is too big for
 	// the udp buffer, hence we need to truncate the message to at least make it

plugin/reverse/reverse.go

@@ -26,7 +26,7 @@ func (re Reverse) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg
 	state := request.Request{W: w, Req: r}
 	m := new(dns.Msg)
 	m.SetReply(r)
-	m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true
+	m.Authoritative, m.RecursionAvailable = true, true
 
 	switch state.QType() {
 	case dns.TypePTR:

plugin/route53/route53.go

@@ -60,7 +60,7 @@ func (rr Route53) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg
 
 	m := new(dns.Msg)
 	m.SetReply(r)
-	m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true
+	m.Authoritative, m.RecursionAvailable = true, true
 	m.Answer = answers
 
 	state.SizeAndDo(m)

plugin/template/template.go

@@ -75,7 +75,7 @@ func (h Handler) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 
 		msg := new(dns.Msg)
 		msg.SetReply(r)
-		msg.Authoritative, msg.RecursionAvailable, msg.Compress = true, true, true
+		msg.Authoritative, msg.RecursionAvailable = true, true
 		msg.Rcode = template.rcode
 
 		for _, answer := range template.answer {
@@ -105,6 +105,7 @@ func (h Handler) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 		}
 
 		state.SizeAndDo(msg)
+		state.Scrub(msg)
 		w.WriteMsg(msg)
 		return template.rcode, nil
 	}

plugin/whoami/whoami.go

@@ -22,7 +22,6 @@ func (wh Whoami) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 
 	a := new(dns.Msg)
 	a.SetReply(r)
-	a.Compress = true
 	a.Authoritative = true
 
 	ip := state.IP()

request/request.go

@@ -180,20 +180,25 @@ const (
 	ScrubAnswer
 )
 
-// Scrub scrubs the reply message so that it will fit the client's buffer. It sets
+// Scrub scrubs the reply message so that it will fit the client's buffer. It will first
-// reply.Compress to true.
+// check if the reply fits without compression and then *with* compression.
-// Scrub uses binary search to find a save cut off point in the additional section.
+// Scrub will then use binary search to find a save cut off point in the additional section.
 // If even *without* the additional section the reply still doesn't fit we
 // repeat this process for the answer section. If we scrub the answer section
 // we set the TC bit on the reply; indicating the client should retry over TCP.
 // Note, the TC bit will be set regardless of protocol, even TCP message will
 // get the bit, the client should then retry with pigeons.
 func (r *Request) Scrub(reply *dns.Msg) (*dns.Msg, Result) {
-	reply.Compress = true
-
 	size := r.Size()
-	rl := reply.Len()
 
+	reply.Compress = false
+	rl := reply.Len()
+	if size >= rl {
+		return reply, ScrubIgnored
+	}
+
+	reply.Compress = true
+	rl = reply.Len()
 	if size >= rl {
 		return reply, ScrubIgnored
 	}