From a9a103ee94a4f2c427dfda8a8b8b9e25d7b108ab Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Mon, 21 Aug 2017 21:46:42 -0700 Subject: [PATCH 1/3] deduplicate dns record in lookup --- naming/dns_resolver.go | 40 +++++++++++++++++-------------------- naming/dns_resolver_test.go | 17 ++++++++++------ 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/naming/dns_resolver.go b/naming/dns_resolver.go index efd37e304983..9ace0e61b136 100644 --- a/naming/dns_resolver.go +++ b/naming/dns_resolver.go @@ -141,8 +141,8 @@ type dnsWatcher struct { r *dnsResolver host string port string - // The latest resolved address list - curAddrs []*Update + // The latest resolved address set + curAddrs map[string]*Update ctx context.Context cancel context.CancelFunc t *time.Timer @@ -192,28 +192,24 @@ type AddrMetadataGRPCLB struct { // compileUpdate compares the old resolved addresses and newly resolved addresses, // and generates an update list -func (w *dnsWatcher) compileUpdate(newAddrs []*Update) []*Update { - update := make(map[Update]bool) - for _, u := range newAddrs { - update[*u] = true - } +func (w *dnsWatcher) compileUpdate(newAddrs map[string]*Update) []*Update { + res := make([]*Update, 0, len(newAddrs)+len(w.curAddrs)) for _, u := range w.curAddrs { - if _, ok := update[*u]; ok { - delete(update, *u) - continue + if _, ok := newAddrs[u.Addr]; !ok { + res = append(res, &Update{Addr: u.Addr, Op: Delete, Metadata: u.Metadata}) } - update[Update{Addr: u.Addr, Op: Delete, Metadata: u.Metadata}] = true } - res := make([]*Update, 0, len(update)) - for k := range update { - tmp := k - res = append(res, &tmp) + for _, u := range newAddrs { + if _, ok := w.curAddrs[u.Addr]; !ok { + res = append(res, u) + } } return res + } -func (w *dnsWatcher) lookupSRV() []*Update { - var newAddrs []*Update +func (w *dnsWatcher) lookupSRV() map[string]*Update { + newAddrs := make(map[string]*Update) _, srvs, err := lookupSRV(w.ctx, "grpclb", "tcp", w.host) if err != nil { grpclog.Infof("grpc: failed dns SRV record lookup due to %v.\n", err) @@ -231,15 +227,15 @@ func (w *dnsWatcher) lookupSRV() []*Update { grpclog.Errorf("grpc: failed IP parsing due to %v.\n", err) continue } - newAddrs = append(newAddrs, &Update{Addr: a + ":" + strconv.Itoa(int(s.Port)), - Metadata: AddrMetadataGRPCLB{AddrType: GRPCLB, ServerName: s.Target}}) + newAddrs[a+":"+strconv.Itoa(int(s.Port))] = &Update{Addr: a + ":" + strconv.Itoa(int(s.Port)), + Metadata: AddrMetadataGRPCLB{AddrType: GRPCLB, ServerName: s.Target}} } } return newAddrs } -func (w *dnsWatcher) lookupHost() []*Update { - var newAddrs []*Update +func (w *dnsWatcher) lookupHost() map[string]*Update { + newAddrs := make(map[string]*Update) addrs, err := lookupHost(w.ctx, w.host) if err != nil { grpclog.Warningf("grpc: failed dns A record lookup due to %v.\n", err) @@ -251,7 +247,7 @@ func (w *dnsWatcher) lookupHost() []*Update { grpclog.Errorf("grpc: failed IP parsing due to %v.\n", err) continue } - newAddrs = append(newAddrs, &Update{Addr: a + ":" + w.port}) + newAddrs[a+":"+w.port] = &Update{Addr: a + ":" + w.port} } return newAddrs } diff --git a/naming/dns_resolver_test.go b/naming/dns_resolver_test.go index 2a990c255146..be1ac1aeca71 100644 --- a/naming/dns_resolver_test.go +++ b/naming/dns_resolver_test.go @@ -84,17 +84,22 @@ func TestCompileUpdate(t *testing.T) { []string{"1.0.0.2", "1.0.0.3", "1.0.0.6"}, []*Update{{Op: Delete, Addr: "1.0.0.1"}, {Op: Add, Addr: "1.0.0.2"}, {Op: Delete, Addr: "1.0.0.5"}, {Op: Add, Addr: "1.0.0.6"}}, }, + { + []string{"1.0.0.1", "1.0.0.1", "1.0.0.2"}, + []string{"1.0.0.1"}, + []*Update{{Op: Delete, Addr: "1.0.0.2"}}, + }, } var w dnsWatcher for _, c := range tests { - w.curAddrs = make([]*Update, len(c.oldAddrs)) - newUpdates := make([]*Update, len(c.newAddrs)) - for i, a := range c.oldAddrs { - w.curAddrs[i] = &Update{Addr: a} + w.curAddrs = make(map[string]*Update) + newUpdates := make(map[string]*Update) + for _, a := range c.oldAddrs { + w.curAddrs[a] = &Update{Addr: a} } - for i, a := range c.newAddrs { - newUpdates[i] = &Update{Addr: a} + for _, a := range c.newAddrs { + newUpdates[a] = &Update{Addr: a} } r := w.compileUpdate(newUpdates) if !reflect.DeepEqual(toMap(c.want), toMap(r)) { From 13e055f8d1ecf1f82ecdbe6e77973b8dff74f54c Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Tue, 22 Aug 2017 10:50:35 -0700 Subject: [PATCH 2/3] minor thing --- naming/dns_resolver.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/naming/dns_resolver.go b/naming/dns_resolver.go index 9ace0e61b136..28b8f5de8aea 100644 --- a/naming/dns_resolver.go +++ b/naming/dns_resolver.go @@ -194,13 +194,14 @@ type AddrMetadataGRPCLB struct { // and generates an update list func (w *dnsWatcher) compileUpdate(newAddrs map[string]*Update) []*Update { res := make([]*Update, 0, len(newAddrs)+len(w.curAddrs)) - for _, u := range w.curAddrs { - if _, ok := newAddrs[u.Addr]; !ok { - res = append(res, &Update{Addr: u.Addr, Op: Delete, Metadata: u.Metadata}) + for a, u := range w.curAddrs { + if _, ok := newAddrs[a]; !ok { + u.Op = Delete + res = append(res, u) } } - for _, u := range newAddrs { - if _, ok := w.curAddrs[u.Addr]; !ok { + for a, u := range newAddrs { + if _, ok := w.curAddrs[a]; !ok { res = append(res, u) } } From 5799b5e1bb04288846cf50211db809105ea2f9b1 Mon Sep 17 00:00:00 2001 From: Yuxuan Li Date: Tue, 22 Aug 2017 15:40:04 -0700 Subject: [PATCH 3/3] fix accroding to review --- naming/dns_resolver.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/naming/dns_resolver.go b/naming/dns_resolver.go index 28b8f5de8aea..7e69a2ca0a66 100644 --- a/naming/dns_resolver.go +++ b/naming/dns_resolver.go @@ -193,7 +193,7 @@ type AddrMetadataGRPCLB struct { // compileUpdate compares the old resolved addresses and newly resolved addresses, // and generates an update list func (w *dnsWatcher) compileUpdate(newAddrs map[string]*Update) []*Update { - res := make([]*Update, 0, len(newAddrs)+len(w.curAddrs)) + var res []*Update for a, u := range w.curAddrs { if _, ok := newAddrs[a]; !ok { u.Op = Delete @@ -206,7 +206,6 @@ func (w *dnsWatcher) compileUpdate(newAddrs map[string]*Update) []*Update { } } return res - } func (w *dnsWatcher) lookupSRV() map[string]*Update { @@ -228,7 +227,8 @@ func (w *dnsWatcher) lookupSRV() map[string]*Update { grpclog.Errorf("grpc: failed IP parsing due to %v.\n", err) continue } - newAddrs[a+":"+strconv.Itoa(int(s.Port))] = &Update{Addr: a + ":" + strconv.Itoa(int(s.Port)), + addr := a + ":" + strconv.Itoa(int(s.Port)) + newAddrs[addr] = &Update{Addr: addr, Metadata: AddrMetadataGRPCLB{AddrType: GRPCLB, ServerName: s.Target}} } } @@ -248,7 +248,8 @@ func (w *dnsWatcher) lookupHost() map[string]*Update { grpclog.Errorf("grpc: failed IP parsing due to %v.\n", err) continue } - newAddrs[a+":"+w.port] = &Update{Addr: a + ":" + w.port} + addr := a + ":" + w.port + newAddrs[addr] = &Update{Addr: addr} } return newAddrs }