From 15c303137cc317d24db2d1427a7ec1409440c00a Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Tue, 3 Jul 2018 16:06:27 -0700 Subject: [PATCH 1/4] exponential retry for dns resolver when getting empty address list --- resolver/dns/dns_resolver.go | 31 +++++++++++----- resolver/dns/dns_resolver_test.go | 60 ++++++++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 10 deletions(-) diff --git a/resolver/dns/dns_resolver.go b/resolver/dns/dns_resolver.go index c0e85488c6b1..61b5ea139407 100644 --- a/resolver/dns/dns_resolver.go +++ b/resolver/dns/dns_resolver.go @@ -33,6 +33,7 @@ import ( "golang.org/x/net/context" "google.golang.org/grpc/grpclog" + "google.golang.org/grpc/internal/backoff" "google.golang.org/grpc/internal/grpcrand" "google.golang.org/grpc/resolver" ) @@ -66,7 +67,7 @@ func NewBuilder() resolver.Builder { } type dnsBuilder struct { - // frequency of polling the DNS server. + // minimum frequency of polling the DNS server. freq time.Duration } @@ -99,6 +100,7 @@ func (b *dnsBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts ctx, cancel := context.WithCancel(context.Background()) d := &dnsResolver{ freq: b.freq, + backoff: backoff.Exponential{MaxDelay: b.freq}, host: host, port: port, ctx: ctx, @@ -154,12 +156,14 @@ func (i *ipResolver) watcher() { // dnsResolver watches for the name resolution update for a non-IP target. type dnsResolver struct { - freq time.Duration - host string - port string - ctx context.Context - cancel context.CancelFunc - cc resolver.ClientConn + freq time.Duration + backoff backoff.Exponential + retryCount int + host string + port string + ctx context.Context + cancel context.CancelFunc + cc resolver.ClientConn // rn channel is used by ResolveNow() to force an immediate resolution of the target. rn chan struct{} t *time.Timer @@ -198,8 +202,17 @@ func (d *dnsResolver) watcher() { case <-d.rn: } result, sc := d.lookup() - // Next lookup should happen after an interval defined by d.freq. - d.t.Reset(d.freq) + // Next lookup should happen within an interval defined by d.freq. It may be + // more often due to exponential retry on empty address list. + if len(result) == 0 { + d.retryCount++ + d.t.Reset(d.backoff.Backoff(d.retryCount)) + } else { + if d.retryCount != 0 { + d.retryCount = 0 + } + d.t.Reset(d.freq) + } d.cc.NewServiceConfig(sc) d.cc.NewAddress(result) } diff --git a/resolver/dns/dns_resolver_test.go b/resolver/dns/dns_resolver_test.go index 55b20211bc70..300cfd0beb24 100644 --- a/resolver/dns/dns_resolver_test.go +++ b/resolver/dns/dns_resolver_test.go @@ -46,7 +46,7 @@ type testClientConn struct { target string m1 sync.Mutex addrs []resolver.Address - a int + a int // how many times NewAddress() has been called m2 sync.Mutex sc string s int @@ -936,3 +936,61 @@ func TestDisableServiceConfig(t *testing.T) { r.Close() } } + +func TestDNSResolverRetry(t *testing.T) { + b := NewBuilder() + target := "ipv4.single.fake" + cc := &testClientConn{target: target} + r, err := b.Build(resolver.Target{Endpoint: target}, cc, resolver.BuildOption{}) + if err != nil { + t.Fatalf("%v\n", err) + } + var addrs []resolver.Address + var cnt int + for { + addrs, cnt = cc.getAddress() + if len(addrs) == 1 { + break + } + time.Sleep(time.Millisecond) + } + want := []resolver.Address{{Addr: "1.2.3.4" + colonDefaultPort}} + if !reflect.DeepEqual(want, addrs) { + t.Errorf("Resolved addresses of target: %q = %+v, want %+v\n", target, addrs, want) + } + // mutate the host lookup table so the target has 0 address returned. + revertTbl := mutateTbl(target) + // trigger a resolve that will get empty address list + r.ResolveNow(resolver.ResolveNowOption{}) + for { + addrs, _ = cc.getAddress() + if len(addrs) == 0 { + break + } + time.Sleep(time.Millisecond) + } + revertTbl() + // wait for the retry to happen in two seconds. + timer := time.NewTimer(2 * time.Second) + for { + b := false + select { + case <-timer.C: + b = true + default: + addrs, cnt = cc.getAddress() + if len(addrs) == 1 { + b = true + break + } + time.Sleep(time.Millisecond) + } + if b { + break + } + } + if !reflect.DeepEqual(want, addrs) { + t.Errorf("Resolved addresses of target: %q = %+v, want %+v\n", target, addrs, want) + } + r.Close() +} From 6611bc51ec0997c55f675dc7dc4035e8358fbd55 Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Tue, 3 Jul 2018 16:30:35 -0700 Subject: [PATCH 2/4] minor --- resolver/dns/dns_resolver_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/resolver/dns/dns_resolver_test.go b/resolver/dns/dns_resolver_test.go index 300cfd0beb24..53d6a87126cd 100644 --- a/resolver/dns/dns_resolver_test.go +++ b/resolver/dns/dns_resolver_test.go @@ -946,9 +946,8 @@ func TestDNSResolverRetry(t *testing.T) { t.Fatalf("%v\n", err) } var addrs []resolver.Address - var cnt int for { - addrs, cnt = cc.getAddress() + addrs, _ = cc.getAddress() if len(addrs) == 1 { break } @@ -978,7 +977,7 @@ func TestDNSResolverRetry(t *testing.T) { case <-timer.C: b = true default: - addrs, cnt = cc.getAddress() + addrs, _ = cc.getAddress() if len(addrs) == 1 { b = true break From 200a09748e352a68929fb9e86d3e19d7b32110ff Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Thu, 12 Jul 2018 17:28:21 -0700 Subject: [PATCH 3/4] uu --- resolver/dns/dns_resolver.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/resolver/dns/dns_resolver.go b/resolver/dns/dns_resolver.go index 61b5ea139407..8af2ac37bdd0 100644 --- a/resolver/dns/dns_resolver.go +++ b/resolver/dns/dns_resolver.go @@ -208,9 +208,7 @@ func (d *dnsResolver) watcher() { d.retryCount++ d.t.Reset(d.backoff.Backoff(d.retryCount)) } else { - if d.retryCount != 0 { - d.retryCount = 0 - } + d.retryCount = 0 d.t.Reset(d.freq) } d.cc.NewServiceConfig(sc) From a40879ff9e71637d193866aeae9ba273c7404aa2 Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Fri, 13 Jul 2018 11:37:29 -0700 Subject: [PATCH 4/4] uuu --- resolver/dns/dns_resolver.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/resolver/dns/dns_resolver.go b/resolver/dns/dns_resolver.go index 8af2ac37bdd0..084bdbfe6053 100644 --- a/resolver/dns/dns_resolver.go +++ b/resolver/dns/dns_resolver.go @@ -63,12 +63,12 @@ var ( // NewBuilder creates a dnsBuilder which is used to factory DNS resolvers. func NewBuilder() resolver.Builder { - return &dnsBuilder{freq: defaultFreq} + return &dnsBuilder{minFreq: defaultFreq} } type dnsBuilder struct { // minimum frequency of polling the DNS server. - freq time.Duration + minFreq time.Duration } // Build creates and starts a DNS resolver that watches the name resolution of the target. @@ -99,8 +99,8 @@ func (b *dnsBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts // DNS address (non-IP). ctx, cancel := context.WithCancel(context.Background()) d := &dnsResolver{ - freq: b.freq, - backoff: backoff.Exponential{MaxDelay: b.freq}, + freq: b.minFreq, + backoff: backoff.Exponential{MaxDelay: b.minFreq}, host: host, port: port, ctx: ctx,