Resolve feedbacks.

Author: and1truong

internal/resolver/dns/dns_resolver.go

@@ -107,9 +107,7 @@ type dnsBuilder struct{}
 
 // Build creates and starts a DNS resolver that watches the name resolution of
 // the target.
-func (b *dnsBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (
-	resolver.Resolver, error,
-) {
+func (b *dnsBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (resolver.Resolver, error) {
 	host, port, err := parseTarget(target.Endpoint(), defaultPort)
 	if err != nil {
 		return nil, err
@@ -325,12 +323,10 @@ func (d *dnsResolver) lookupHost(ctx context.Context) ([]resolver.Address, error
 }
 
 func (d *dnsResolver) lookup() (*resolver.State, error) {
-	ctxSRV, cancelSRV := context.WithTimeout(d.ctx, ResolvingTimeout)
-	defer cancelSRV()
-	srv, srvErr := d.lookupSRV(ctxSRV)
-	ctxHost, cancelHost := context.WithTimeout(d.ctx, ResolvingTimeout)
-	defer cancelHost()
-	addrs, hostErr := d.lookupHost(ctxHost)
+	ctx, cancel := context.WithTimeout(d.ctx, ResolvingTimeout)
+	defer cancel()
+	srv, srvErr := d.lookupSRV(ctx)
+	addrs, hostErr := d.lookupHost(ctx)
 	if hostErr != nil && (srvErr != nil || len(srv) == 0) {
 		return nil, hostErr
 	}
@@ -340,9 +336,7 @@ func (d *dnsResolver) lookup() (*resolver.State, error) {
 		state = grpclbstate.Set(state, &grpclbstate.State{BalancerAddresses: srv})
 	}
 	if !d.disableServiceConfig {
-		ctxTXT, cancelTXT := context.WithTimeout(d.ctx, ResolvingTimeout)
-		defer cancelTXT()
-		state.ServiceConfig = d.lookupTXT(ctxTXT)
+		state.ServiceConfig = d.lookupTXT(ctx)
 	}
 	return &state, nil
 }

internal/resolver/dns/dns_resolver_test.go

@@ -39,6 +39,7 @@ import (
 	dnsinternal "google.golang.org/grpc/internal/resolver/dns/internal"
 	"google.golang.org/grpc/internal/testutils"
 	"google.golang.org/grpc/resolver"
+	dnspublic "google.golang.org/grpc/resolver/dns"
 	"google.golang.org/grpc/serviceconfig"
 
 	_ "google.golang.org/grpc" // To initialize internal.ParseServiceConfig
@@ -1215,3 +1216,32 @@ func (s) TestReportError(t *testing.T) {
 		}
 	}
 }
+
+// Test verifies that when the DNS resolver gets timeout error when net.Resolver
+// takes too long to resolve a target.
+func (s) TestResolveTimeout(t *testing.T) {
+	// Set DNS resolving timeout to 7ms
+	dnspublic.SetResolvingTimeout(7 * time.Millisecond)
+
+	// Time to run test this should not longer than 10s.
+	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
+	defer cancel()
+
+	// We are trying to resolve hostname which takes infinity time to resolve.
+	const target = "infinity"
+	tr := &testNetResolver{}
+	tr.UpdateHostLookupTable(map[string][]string{target: {"1.2.3.4"}})
+	overrideNetResolver(t, tr)
+
+	r, _, errCh := buildResolverWithTestClientConn(t, target)
+	r.ResolveNow(resolver.ResolveNowOptions{})
+
+	select {
+	case <-ctx.Done():
+		t.Fatal("Timeout when waiting for an error from the resolver")
+	case err := <-errCh:
+		if err == nil || !strings.Contains(err.Error(), "hostLookup timeout") {
+			t.Fatalf(`we expect to see timed out error`)
+		}
+	}
+}

internal/resolver/dns/fake_net_resolver_test.go

@@ -21,7 +21,9 @@ package dns_test
 import (
 	"context"
 	"net"
+	"strings"
 	"sync"
+	"time"
 
 	"google.golang.org/grpc/internal/testutils"
 )
@@ -47,9 +49,28 @@ func (tr *testNetResolver) LookupHost(ctx context.Context, host string) ([]strin
 	tr.mu.Lock()
 	defer tr.mu.Unlock()
 
+	// trigger a timeout if the host name contains "infinity"
+	if strings.Contains(host, "infinity") {
+		<-time.After(88 * time.Millisecond)
+	}
+
+	// return timeout error if context reached its deadline
+	select {
+	case <-ctx.Done():
+		return nil, &net.DNSError{
+			Err:         "hostLookup timeout",
+			Name:        host,
+			Server:      "fake",
+			IsTemporary: true,
+		}
+
+	default:
+	}
+
 	if addrs, ok := tr.hostLookupTable[host]; ok {
 		return addrs, nil
 	}
+
 	return nil, &net.DNSError{
 		Err:         "hostLookup error",
 		Name:        host,

resolver/dns/dns_resolver.go

@@ -32,12 +32,10 @@ import (
 
 // SetResolvingTimeout sets the maximum duration for DNS resolution requests.
 //
-// This function updates the global `ResolvingTimeout` variable, which specifies
-// how long a DNS resolution attempt will wait before it is canceled.
+// This function affects the global timeout used by all channels using the DNS name resolver scheme.
 //
-// It is recommended to call this function at application startup, before any
-// gRPC calls are made. Modifying this value after initialization is not
-// thread-safe.
+// It must be called only at application startup, before any  gRPC calls are made.
+// Modifying this value after initialization is not thread-safe.
 //
 // The default value is 10 seconds. Setting the timeout too low may result
 // in premature timeouts during resolution, while setting it too high may