From c6f3f33c997569eb441b43ba7ac105652be99e0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Medzi=C5=84ski?= Date: Fri, 6 Jul 2018 17:05:43 +0200 Subject: [PATCH] Ignore empty Consul instance updates (#119) Updating current instances slice to empty one may lock whole mechanism. It is better to have several network errors in the metrics than to complicate the whole mechanism. --- xnet/writer.go | 10 +++++++--- xnet/writer_test.go | 26 +++++++++++++++----------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/xnet/writer.go b/xnet/writer.go index b1cfaf5d..9db471e6 100644 --- a/xnet/writer.go +++ b/xnet/writer.go @@ -94,9 +94,13 @@ func DiscoveryServiceInstanceProvider(serviceName string, interval time.Duration return newInstances[i] < newInstances[j] }) if !reflect.DeepEqual(currInstances, newInstances) { - log.WithField("instances", newInstances).Infof("Service %q instances in discovery changed - sending update", serviceName) - currInstances = newInstances - instancesChan <- newInstances + if len(newInstances) > 0 { + log.WithField("instances", newInstances).Infof("Service %q instances in discovery changed - sending update", serviceName) + currInstances = newInstances + instancesChan <- newInstances + } else { + log.WithField("instances", newInstances).Infof("Service %q instances in discovery changed - ignoring empty update", serviceName) + } } } }() diff --git a/xnet/writer_test.go b/xnet/writer_test.go index 0d027bc6..12790121 100644 --- a/xnet/writer_test.go +++ b/xnet/writer_test.go @@ -76,28 +76,32 @@ func TestDiscoveryServiceInstanceProviderShouldPeriodicallyUpdatesInstances(t *t assert.Empty(t, ret) } -func TestDiscoveryServiceInstanceProviderShouldUpdateInstancesWhenTheyAreEmpty(t *testing.T) { - ret := make(chan []Address, 1) - client := &StubDiscoveryServiceClient{returns: ret} +func TestIfUpdatesAddressesOnlyIfTheyChanged(t *testing.T) { + returns := make(chan []Address, 5) + discoveryServiceClient := &StubDiscoveryServiceClient{returns} // setup expectations - ret <- []Address{} + returns <- []Address{"127.0.0.1:1234", "127.0.0.1:4321"} // initial instances + returns <- []Address{"127.0.0.1:1234", "127.0.0.1:4321"} // same as before but different order + returns <- []Address{"127.0.0.1:1234", "127.0.0.1:4321"} // same as before + returns <- []Address{"127.0.0.1:4321", "127.0.0.1:1234"} // same as before + returns <- []Address{"127.0.0.1:5678", "127.0.0.1:8765"} // different ports - provider := DiscoveryServiceInstanceProvider("service name", 1, client) + instanceProvider := DiscoveryServiceInstanceProvider("service-name", 1, discoveryServiceClient) - assert.Equal(t, []Address{}, <-provider) - assert.Empty(t, ret) + assert.Equal(t, []Address{"127.0.0.1:1234", "127.0.0.1:4321"}, <-instanceProvider) + assert.Equal(t, []Address{"127.0.0.1:5678", "127.0.0.1:8765"}, <-instanceProvider) + assert.Empty(t, returns) } -func TestIfUpdatesAddressesOnlyIfTheyChanged(t *testing.T) { - returns := make(chan []Address, 5) +func TestIfNotUpdatesEmptyAddresses(t *testing.T) { + returns := make(chan []Address, 4) discoveryServiceClient := &StubDiscoveryServiceClient{returns} // setup expectations returns <- []Address{"127.0.0.1:1234", "127.0.0.1:4321"} // initial instances - returns <- []Address{"127.0.0.1:1234", "127.0.0.1:4321"} // same as before but different order + returns <- []Address{} // empty pool returns <- []Address{"127.0.0.1:1234", "127.0.0.1:4321"} // same as before - returns <- []Address{"127.0.0.1:4321", "127.0.0.1:1234"} // same as before returns <- []Address{"127.0.0.1:5678", "127.0.0.1:8765"} // different ports instanceProvider := DiscoveryServiceInstanceProvider("service-name", 1, discoveryServiceClient)