Remove SingleInflight support from Client (#1454)

* Remove SingleInflight support from Client

Callers should instead implement their own in flight query caching.

* Add doc link to Github issue

client.go

@@ -6,7 +6,6 @@ import (
 	"context"
 	"crypto/tls"
 	"encoding/binary"
-	"fmt"
 	"io"
 	"net"
 	"strings"
@@ -56,14 +55,20 @@ type Client struct {
 	// Timeout is a cumulative timeout for dial, write and read, defaults to 0 (disabled) - overrides DialTimeout, ReadTimeout,
 	// WriteTimeout when non-zero. Can be overridden with net.Dialer.Timeout (see Client.ExchangeWithDialer and
 	// Client.Dialer) or context.Context.Deadline (see ExchangeContext)
-	Timeout        time.Duration
-	DialTimeout    time.Duration     // net.DialTimeout, defaults to 2 seconds, or net.Dialer.Timeout if expiring earlier - overridden by Timeout when that value is non-zero
-	ReadTimeout    time.Duration     // net.Conn.SetReadTimeout value for connections, defaults to 2 seconds - overridden by Timeout when that value is non-zero
-	WriteTimeout   time.Duration     // net.Conn.SetWriteTimeout value for connections, defaults to 2 seconds - overridden by Timeout when that value is non-zero
-	TsigSecret     map[string]string // secret(s) for Tsig map[<zonename>]<base64 secret>, zonename must be in canonical form (lowercase, fqdn, see RFC 4034 Section 6.2)
-	TsigProvider   TsigProvider      // An implementation of the TsigProvider interface. If defined it replaces TsigSecret and is used for all TSIG operations.
-	SingleInflight bool              // if true suppress multiple outstanding queries for the same Qname, Qtype and Qclass
-	group          singleflight
+	Timeout      time.Duration
+	DialTimeout  time.Duration     // net.DialTimeout, defaults to 2 seconds, or net.Dialer.Timeout if expiring earlier - overridden by Timeout when that value is non-zero
+	ReadTimeout  time.Duration     // net.Conn.SetReadTimeout value for connections, defaults to 2 seconds - overridden by Timeout when that value is non-zero
+	WriteTimeout time.Duration     // net.Conn.SetWriteTimeout value for connections, defaults to 2 seconds - overridden by Timeout when that value is non-zero
+	TsigSecret   map[string]string // secret(s) for Tsig map[<zonename>]<base64 secret>, zonename must be in canonical form (lowercase, fqdn, see RFC 4034 Section 6.2)
+	TsigProvider TsigProvider      // An implementation of the TsigProvider interface. If defined it replaces TsigSecret and is used for all TSIG operations.
+
+	// SingleInflight previously serialised multiple concurrent queries for the
+	// same Qname, Qtype and Qclass to ensure only one would be in flight at a
+	// time.
+	//
+	// Deprecated: This is a no-op. Callers should implement their own in flight
+	// query caching if needed. See github.com/miekg/dns/issues/1449.
+	SingleInflight bool
 }
 
 // Exchange performs a synchronous UDP query. It sends the message m to the address
@@ -185,26 +190,7 @@ func (c *Client) ExchangeWithConn(m *Msg, conn *Conn) (r *Msg, rtt time.Duration
 	return c.exchangeWithConnContext(context.Background(), m, conn)
 }
 
-func (c *Client) exchangeWithConnContext(ctx context.Context, m *Msg, conn *Conn) (r *Msg, rtt time.Duration, err error) {
-	if !c.SingleInflight {
-		return c.exchangeContext(ctx, m, conn)
-	}
-
-	q := m.Question[0]
-	key := fmt.Sprintf("%s:%d:%d", q.Name, q.Qtype, q.Qclass)
-	r, rtt, err, shared := c.group.Do(key, func() (*Msg, time.Duration, error) {
-		// When we're doing singleflight we don't want one context cancellation, cancel _all_ outstanding queries.
-		// Hence we ignore the context and use Background().
-		return c.exchangeContext(context.Background(), m, conn)
-	})
-	if r != nil && shared {
-		r = r.Copy()
-	}
-
-	return r, rtt, err
-}
-
-func (c *Client) exchangeContext(ctx context.Context, m *Msg, co *Conn) (r *Msg, rtt time.Duration, err error) {
+func (c *Client) exchangeWithConnContext(ctx context.Context, m *Msg, co *Conn) (r *Msg, rtt time.Duration, err error) {
 	opt := m.IsEdns0()
 	// If EDNS0 is used use that for size.
 	if opt != nil && opt.UDPSize() >= MinMsgSize {

client_test.go

@@ -679,58 +679,6 @@ func TestTimeout(t *testing.T) {
 	})
 }
 
-// Check that responses from deduplicated requests aren't shared between callers
-func TestConcurrentExchanges(t *testing.T) {
-	cases := make([]*Msg, 2)
-	cases[0] = new(Msg)
-	cases[1] = new(Msg)
-	cases[1].Truncated = true
-
-	for _, m := range cases {
-		mm := m // redeclare m so as not to trip the race detector
-		handler := func(w ResponseWriter, req *Msg) {
-			r := mm.Copy()
-			r.SetReply(req)
-
-			w.WriteMsg(r)
-		}
-
-		HandleFunc("miek.nl.", handler)
-		defer HandleRemove("miek.nl.")
-
-		s, addrstr, _, err := RunLocalUDPServer(":0")
-		if err != nil {
-			t.Fatalf("unable to run test server: %s", err)
-		}
-		defer s.Shutdown()
-
-		m := new(Msg)
-		m.SetQuestion("miek.nl.", TypeSRV)
-
-		c := &Client{
-			SingleInflight: true,
-		}
-		// Force this client to always return the same request,
-		// even though we're querying sequentially. Running the
-		// Exchange calls below concurrently can fail due to
-		// goroutine scheduling, but this simulates the same
-		// outcome.
-		c.group.dontDeleteForTesting = true
-
-		r := make([]*Msg, 2)
-		for i := range r {
-			r[i], _, _ = c.Exchange(m.Copy(), addrstr)
-			if r[i] == nil {
-				t.Errorf("response %d is nil", i)
-			}
-		}
-
-		if r[0] == r[1] {
-			t.Errorf("got same response, expected non-shared responses")
-		}
-	}
-}
-
 func TestExchangeWithConn(t *testing.T) {
 	HandleFunc("miek.nl.", HelloServer)
 	defer HandleRemove("miek.nl.")