net: fix DNS NXDOMAIN performance regression

golang.org/cl/37879 unintentionally changed the way NXDOMAIN errors were
handled. Before that change, resolution would fail on the first NXDOMAIN
error and return to the user. After that change, the next server would
be consulted and resolution would fail only after all servers had been
consulted. This change restores the old behavior.

Go 10.10.2:
BenchmarkGoLookupIP-12                        	   10000	    174883 ns/op   11450 B/op	     163 allocs/op
BenchmarkGoLookupIPNoSuchHost-12              	    3000	    670140 ns/op   52189 B/op	     544 allocs/op
BenchmarkGoLookupIPWithBrokenNameServer-12    	       1	5002568137 ns/op  163792 B/op	     375 allocs/op

before this change:
BenchmarkGoLookupIP-12                        	   10000	    165501 ns/op    8585 B/op	      94 allocs/op
BenchmarkGoLookupIPNoSuchHost-12              	    1000	   1204117 ns/op   83661 B/op	     674 allocs/op
BenchmarkGoLookupIPWithBrokenNameServer-12    	       1	5002629186 ns/op  159128 B/op	     275 allocs/op

after this change:
BenchmarkGoLookupIP-12                        	   10000	    158102 ns/op    8585 B/op	      94 allocs/op
BenchmarkGoLookupIPNoSuchHost-12              	    2000	    645364 ns/op   42990 B/op	     356 allocs/op
BenchmarkGoLookupIPWithBrokenNameServer-12    	       1	5002163437 ns/op  159144 B/op	     275 allocs/op

Fixes #25336

Change-Id: I315cd70330d1f66e54ce5a189a61c99f095bc138
Reviewed-on: https://go-review.googlesource.com/113815
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

1 files changed, 31 insertions, 17 deletions

diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go
index 1d3b78284cc..a95b2fe6451 100644
--- a/src/net/dnsclient_unix_test.go
+++ b/src/net/dnsclient_unix_test.go
@@ -746,6 +746,7 @@ func TestIgnoreLameReferrals(t *testing.T) {
 func BenchmarkGoLookupIP(b *testing.B) {
 	testHookUninstaller.Do(uninstallTestHooks)
 	ctx := context.Background()
+	b.ReportAllocs()
 
 	for i := 0; i < b.N; i++ {
 		goResolver.LookupIPAddr(ctx, "www.example.com")
@@ -755,6 +756,7 @@ func BenchmarkGoLookupIP(b *testing.B) {
 func BenchmarkGoLookupIPNoSuchHost(b *testing.B) {
 	testHookUninstaller.Do(uninstallTestHooks)
 	ctx := context.Background()
+	b.ReportAllocs()
 
 	for i := 0; i < b.N; i++ {
 		goResolver.LookupIPAddr(ctx, "some.nonexistent")
@@ -778,6 +780,7 @@ func BenchmarkGoLookupIPWithBrokenNameServer(b *testing.B) {
 		b.Fatal(err)
 	}
 	ctx := context.Background()
+	b.ReportAllocs()
 
 	for i := 0; i < b.N; i++ {
 		goResolver.LookupIPAddr(ctx, "www.example.com")
@@ -1437,7 +1440,7 @@ func TestIssue8434(t *testing.T) {
 		t.Fatal("SkipAllQuestions failed:", err)
 	}
 
-	err = checkHeaders(&p, h, "golang.org", "foo:53")
+	err = checkHeader(&p, h, "golang.org", "foo:53")
 	if err == nil {
 		t.Fatal("expected an error")
 	}
@@ -1455,28 +1458,39 @@ func TestIssue8434(t *testing.T) {
 
 // Issue 12778: verify that NXDOMAIN without RA bit errors as
 // "no such host" and not "server misbehaving"
+//
+// Issue 25336: verify that NXDOMAIN errors fail fast.
 func TestIssue12778(t *testing.T) {
-	msg := dnsmessage.Message{
-		Header: dnsmessage.Header{
-			RCode:              dnsmessage.RCodeNameError,
-			RecursionAvailable: false,
+	lookups := 0
+	fake := fakeDNSServer{
+		rh: func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) {
+			lookups++
+			return dnsmessage.Message{
+				Header: dnsmessage.Header{
+					ID:                 q.ID,
+					Response:           true,
+					RCode:              dnsmessage.RCodeNameError,
+					RecursionAvailable: false,
+				},
+				Questions: q.Questions,
+			}, nil
 		},
 	}
+	r := Resolver{PreferGo: true, Dial: fake.DialContext}
 
-	b, err := msg.Pack()
-	if err != nil {
-		t.Fatal("Pack failed:", err)
-	}
-	var p dnsmessage.Parser
-	h, err := p.Start(b)
-	if err != nil {
-		t.Fatal("Start failed:", err)
-	}
-	if err := p.SkipAllQuestions(); err != nil {
-		t.Fatal("SkipAllQuestions failed:", err)
+	resolvConf.mu.RLock()
+	conf := resolvConf.dnsConfig
+	resolvConf.mu.RUnlock()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	_, _, err := r.tryOneName(ctx, conf, ".", dnsmessage.TypeALL)
+
+	if lookups != 1 {
+		t.Errorf("got %d lookups, wanted 1", lookups)
 	}
 
-	err = checkHeaders(&p, h, "golang.org", "foo:53")
 	if err == nil {
 		t.Fatal("expected an error")
 	}