From 9b6ac7ddb739a2bdfe5692fc29284db555756acd Mon Sep 17 00:00:00 2001
From: lyuxuan <yuxuanli@google.com>
Date: Wed, 23 Aug 2017 13:45:01 -0700
Subject: [PATCH] deduplicate dns record in lookup (#1454)

deduplicate dns record in lookup
---
 naming/dns_resolver.go      | 44 ++++++++++++++++++-------------------
 naming/dns_resolver_test.go | 17 +++++++++-----
 2 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/naming/dns_resolver.go b/naming/dns_resolver.go
index efd37e30..7e69a2ca 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
-	}
-	for _, u := range w.curAddrs {
-		if _, ok := update[*u]; ok {
-			delete(update, *u)
-			continue
+func (w *dnsWatcher) compileUpdate(newAddrs map[string]*Update) []*Update {
+	var res []*Update
+	for a, u := range w.curAddrs {
+		if _, ok := newAddrs[a]; !ok {
+			u.Op = Delete
+			res = append(res, u)
 		}
-		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 a, u := range newAddrs {
+		if _, ok := w.curAddrs[a]; !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,16 @@ 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}})
+			addr := a + ":" + strconv.Itoa(int(s.Port))
+			newAddrs[addr] = &Update{Addr: addr,
+				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 +248,8 @@ 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})
+		addr := a + ":" + w.port
+		newAddrs[addr] = &Update{Addr: addr}
 	}
 	return newAddrs
 }
diff --git a/naming/dns_resolver_test.go b/naming/dns_resolver_test.go
index 2a990c25..be1ac1ae 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)) {