netbalancer: reduce logging by only logging when hostlist has changed

esiqveland · Jun 12, 2019 · 36395a3 · 36395a3
1 parent 9e6d655
commit 36395a3
Show file tree

Hide file tree

Showing 3 changed files with 227 additions and 28 deletions.
diff --git a/netbalancer/netbalancer.go b/netbalancer/netbalancer.go
@@ -1,6 +1,7 @@
 package netbalancer
 
 import (
+ "bytes"
  "context"
  "log"
  "net"
@@ -35,15 +36,15 @@ func New(host string, port int, updateInterval, timeout time.Duration) (balancer
  }
 
  // start update loop
- go bal.update()
+ go bal.loop()
 
  return bal, nil
 }
 
 type dnsBalancer struct {
  lookupAddress string
  port int
- hosts []balancer.Host
+ hosts []*balancer.Host
  counter uint64
  interval time.Duration
  quit chan int
@@ -63,52 +64,93 @@ func (b *dnsBalancer) Next() (balancer.Host, error) {
 
  idx := nextNum % count
 
- return hosts[idx], nil
+ return *hosts[idx], nil
 }
 
-func (b *dnsBalancer) update() {
+func (b *dnsBalancer) loop() {
  tick := time.NewTicker(b.interval)
 
  for {
  select {
  case <-tick.C:
- nextHostList, err := lookupTimeout(b.Timeout, b.lookupAddress, b.port)
- if err != nil {
- // TODO: set hostList to empty?
- log.Printf("[DnsBalancers] error looking up dns='%v': %v", b.lookupAddress, err)
- } else {
- if nextHostList != nil {
- log.Printf("[DnsBalancer] reloaded dns=%v hosts=%v", b.lookupAddress, nextHostList)
- b.lock.Lock()
- b.hosts = nextHostList
- b.lock.Unlock()
- }
- }
+ b.update()
  case <-b.quit:
  tick.Stop()
  return
  }
  }
 }
 
-func lookupTimeout(timeout time.Duration, host string, port int) ([]balancer.Host, error) {
+func (b *dnsBalancer) update() {
+ nextHostList, err := lookupTimeout(b.Timeout, b.lookupAddress, b.port)
+ if err != nil {
+ // TODO: set hostList to empty?
+ log.Printf("[DnsBalancers] error looking up dns='%v': %v", b.lookupAddress, err)
+ } else {
+ if nextHostList != nil {
+ b.lock.Lock()
+ defer b.lock.Unlock()
+
+ prev := b.hosts
+ if !equals(prev, nextHostList) {
+ log.Printf("[DnsBalancer] hosts changed dns=%v hosts=%v", b.lookupAddress, nextHostList)
+ b.hosts = nextHostList
+ }
+ }
+ }
+}
+
+func equalsHost(a *balancer.Host, b *balancer.Host) bool {
+ if a.Port != b.Port {
+ return false
+ }
+
+ // dont use IP.Equal because it considers ipv4 and ipv6 address to be the same.
+ return bytes.Equal(a.Address, b.Address)
+}
+
+func equals(a []*balancer.Host, b []*balancer.Host) bool {
+ if len(a) != len(b) {
+ return false
+ }
+
+ for i := range a {
+ if !hostListContains(b, a[i]) {
+ return false
+ }
+ }
+
+ return true
+}
+
+func hostListContains(hosts []*balancer.Host, host *balancer.Host) bool {
+ for i := range hosts {
+ if equalsHost(hosts[i], host) {
+ return true
+ }
+ }
+
+ return false
+}
+
+func lookupTimeout(timeout time.Duration, host string, port int) ([]*balancer.Host, error) {
  ctx := context.Background()
  ctx, cancel := context.WithTimeout(ctx, timeout)
  defer cancel()
 
  return lookup(ctx, host, port)
 }
 
-func lookup(ctx context.Context, host string, port int) ([]balancer.Host, error) {
- hosts := []balancer.Host{}
+func lookup(ctx context.Context, host string, port int) ([]*balancer.Host, error) {
+ hosts := []*balancer.Host{}
 
  ips, err := net.DefaultResolver.LookupIPAddr(ctx, host)
  if err != nil {
  return hosts, errors.Wrapf(err, "Error looking up host=%v", host)
  }
 
  for k := range ips {
- entry := balancer.Host{
+ entry := &balancer.Host{
  Address: ips[k].IP,
  Port: port,
  }

diff --git a/netbalancer/netbalancer_test.go b/netbalancer/netbalancer_test.go
@@ -0,0 +1,154 @@
+package netbalancer
+
+import (
+ "net"
+ "testing"
+
+ "github.com/esiqveland/balancer"
+)
+
+func Test_equalsHost(t *testing.T) {
+ type args struct {
+ a *balancer.Host
+ b *balancer.Host
+ }
+ tests := []struct {
+ name string
+ args args
+ want bool
+ }{{
+ name: "two equal hosts",
+ args: args{
+ a: &balancer.Host{Address: net.IPv4(192, 168, 0, 1), Port: 123},
+ b: &balancer.Host{Address: net.IPv4(192, 168, 0, 1), Port: 123},
+ },
+ want: true,
+ },
+ {
+ name: "two not equal hosts by port",
+ args: args{
+ a: &balancer.Host{Address: net.IPv4(192, 168, 0, 1), Port: 1233},
+ b: &balancer.Host{Address: net.IPv4(192, 168, 0, 1), Port: 1234},
+ },
+ want: false,
+ },
+ {
+ name: "two not equal hosts by IP",
+ args: args{
+ a: &balancer.Host{Address: net.IPv4(192, 168, 0, 2), Port: 1234},
+ b: &balancer.Host{Address: net.IPv4(192, 168, 0, 1), Port: 1234},
+ },
+ want: false,
+ },
+ {
+ name: "two not equal hosts by IP and Port",
+ args: args{
+ a: &balancer.Host{Address: net.IPv4(192, 168, 0, 2), Port: 1234},
+ b: &balancer.Host{Address: net.IPv4(192, 168, 0, 1), Port: 123},
+ },
+ want: false,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ if got := equalsHost(tt.args.a, tt.args.b); got != tt.want {
+ t.Errorf("equalsHost() = %v, want %v", got, tt.want)
+ }
+ })
+ }
+}
+
+func Test_equals(t *testing.T) {
+ type args struct {
+ a []*balancer.Host
+ b []*balancer.Host
+ }
+ tests := []struct {
+ name string
+ args args
+ want bool
+ }{{
+ name: "empty list",
+ args: args{
+ a: []*balancer.Host{},
+ b: []*balancer.Host{},
+ },
+ want: true,
+ },{
+ name: "two same ips",
+ args: args{
+ a: []*balancer.Host{
+ {Address: net.IPv4(192, 168, 0, 2), Port: 1234},
+ },
+ b: []*balancer.Host{
+ {Address: net.IPv4(192, 168, 0, 2), Port: 1234},
+ },
+ },
+ want: true,
+ },{
+ name: "different list length",
+ args: args{
+ a: []*balancer.Host{
+ {Address: net.IPv4(192, 168, 0, 2), Port: 1234},
+ {Address: net.IPv4(192, 168, 0, 1), Port: 1234},
+ },
+ b: []*balancer.Host{
+ {Address: net.IPv4(192, 168, 0, 2), Port: 1234},
+ },
+ },
+ want: false,
+ },{
+ name: "two equals lists of hosts",
+ args: args{
+ a: []*balancer.Host{
+ {Address: net.IPv4(192, 168, 0, 1), Port: 1234},
+ {Address: net.IPv4(192, 168, 0, 2), Port: 1234},
+ {Address: net.IPv4(192, 168, 0, 3), Port: 1234},
+ },
+ b: []*balancer.Host{
+ {Address: net.IPv4(192, 168, 0, 1), Port: 1234},
+ {Address: net.IPv4(192, 168, 0, 2), Port: 1234},
+ {Address: net.IPv4(192, 168, 0, 3), Port: 1234},
+ },
+ },
+ want: true,
+ },{
+ name: "two equals scrambled lists of hosts",
+ args: args{
+ a: []*balancer.Host{
+ {Address: net.IPv4(192, 168, 0, 3), Port: 1234},
+ {Address: net.IPv4(192, 168, 0, 2), Port: 1234},
+ {Address: net.IPv4(192, 168, 0, 1), Port: 1234},
+ },
+ b: []*balancer.Host{
+ {Address: net.IPv4(192, 168, 0, 1), Port: 1234},
+ {Address: net.IPv4(192, 168, 0, 2), Port: 1234},
+ {Address: net.IPv4(192, 168, 0, 3), Port: 1234},
+ },
+ },
+ want: true,
+ },{
+ name: "two equals scrambled lists of host ports",
+ args: args{
+ a: []*balancer.Host{
+ {Address: net.IPv4(192, 168, 0, 1), Port: 123},
+ {Address: net.IPv4(192, 168, 0, 1), Port: 1234},
+ {Address: net.IPv4(192, 168, 0, 1), Port: 12345},
+ },
+ b: []*balancer.Host{
+ {Address: net.IPv4(192, 168, 0, 1), Port: 12345},
+ {Address: net.IPv4(192, 168, 0, 1), Port: 123},
+ {Address: net.IPv4(192, 168, 0, 1), Port: 1234},
+ },
+ },
+ want: true,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ if got := equals(tt.args.a, tt.args.b); got != tt.want {
+ t.Errorf("equals() = %v, want %v", got, tt.want)
+ }
+ })
+ }
+}
diff --git a/netbalancer/srvbalancer.go b/netbalancer/srvbalancer.go
@@ -46,7 +46,7 @@ type dnsSrvBalancer struct {
  serviceName string
  proto string
  host string
- hosts []balancer.Host
+ hosts []*balancer.Host
  counter uint64
  interval time.Duration
  quit chan int
@@ -88,16 +88,16 @@ func NewSRV(servicename, proto, host string, opts ...srvOption) (balancer.Balanc
  return bal, nil
 }
 
-func lookupSRVTimeout(timeout time.Duration, resolver srvResolver, serviceName string, proto string, host string) ([]balancer.Host, error) {
+func lookupSRVTimeout(timeout time.Duration, resolver srvResolver, serviceName string, proto string, host string) ([]*balancer.Host, error) {
  ctx := context.Background()
  ctx, cancel := context.WithTimeout(ctx, timeout)
  defer cancel()
 
  return lookupSRV(ctx, resolver, serviceName, proto, host)
 }
 
-func lookupSRV(ctx context.Context, resolver srvResolver, servicename, proto, host string) ([]balancer.Host, error) {
- hosts := []balancer.Host{}
+func lookupSRV(ctx context.Context, resolver srvResolver, servicename, proto, host string) ([]*balancer.Host, error) {
+ hosts := []*balancer.Host{}
 
  _, addrs, err := resolver.LookupSRV(ctx, servicename, proto, host)
  if err != nil {
@@ -116,7 +116,7 @@ func lookupSRV(ctx context.Context, resolver srvResolver, servicename, proto, ho
  }
 
  for e := range ips {
- host := balancer.Host{
+ host := &balancer.Host{
  Address: ips[e].IP,
  Port: int(v.Port),
  }
@@ -139,7 +139,7 @@ func (b *dnsSrvBalancer) Next() (balancer.Host, error) {
 
  idx := nextNum % count
 
- return hosts[idx], nil
+ return *hosts[idx], nil
 }
 
 func (b *dnsSrvBalancer) update() error {
@@ -152,8 +152,11 @@ func (b *dnsSrvBalancer) update() error {
  log.Printf("[SRVBalancer] error looking up dns='%v': %v", b.host, err)
  } else {
  if nextHostList != nil {
- log.Printf("[SRVBalancer] reloaded dns=%v hosts=%v", b.host, nextHostList)
- b.hosts = nextHostList
+ prev := b.hosts
+ if !equals(prev, nextHostList) {
+ log.Printf("[SRVBalancer] hosts changed dns=%v hosts=%v", b.host, nextHostList)
+ b.hosts = nextHostList
+ }
  }
  }
  return err