Skip to content

Commit

Permalink
Ignore empty Consul instance updates (#119)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
medzin authored Jul 6, 2018
1 parent 8257050 commit c6f3f33
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 14 deletions.
10 changes: 7 additions & 3 deletions xnet/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
}()
Expand Down
26 changes: 15 additions & 11 deletions xnet/writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit c6f3f33

Please sign in to comment.