clientv3: don't race on upc/downc/switch endpoints in balancer

[heyitsanthony]

If the balancer update notification loop starts with a downed
connection and endpoints are switched while the old connection is up,
the balancer can potentially wait forever for an up connection without
refreshing the connections to reflect the current endpoints.

Instead, fetch upc/downc together, only caring about a single transition
either from down->up or up->down for each iteration

Simple way to reproduce failures: add time.Sleep(time.Second) to the
beginning of the update notification loop.

Found while investigating #7823, but not a fix.

[fanminshi]

Simple way to reproduce failures: add time.Sleep(time.Second) to the beginning of the update notification loop.

I wonder how hard is to add a test to reproduce this failure.

[heyitsanthony]

The easiest way to test the balancer tolerates delays on upc/downc would be a failpoint annotation to inject a sleep. I don't think it's worth going through all that just for this, though.

[fanminshi]

@heyitsanthony fair enough. I would like to reproduce this issue myself. is adding sleep at beginning of the update notification loop. sufficient to trigger this? I think we also need to update the endpoints at a certain way along with the sleep to trigger this, right?

[heyitsanthony]

@fanminshi add time.Sleep(time.Second) to the beginning of the for loop and try running TestDialSetEndpointsBeforeFail

[fanminshi]

@heyitsanthony thanks.

[heyitsanthony]

#7823 now fixed by this

[gyuho]

Is CI failure related to this change?

--- PASS: TestWatchCancelDisconnected (0.10s)
PASS
2017-05-02 20:18:04.284186 I | etcdserver/api/v3rpc: grpc: addrConn.transportMonitor exits due to: context canceled
Too many goroutines running after all test(s).
1 instances of:
google.golang.org/grpc.(*addrConn).resetTransport(...)
	/var/jenkins_home/workspace/etcd-ci-ppc64/gopath/src/google.golang.org/grpc/clientconn.go:832 +0x698
google.golang.org/grpc.(*addrConn).transportMonitor(...)
	/var/jenkins_home/workspace/etcd-ci-ppc64/gopath/src/google.golang.org/grpc/clientconn.go:912 +0x288
google.golang.org/grpc.(*ClientConn).resetAddrConn.func1(...)
	/var/jenkins_home/workspace/etcd-ci-ppc64/gopath/src/google.golang.org/grpc/clientconn.go:614 +0x1bc
created by google.golang.org/grpc.(*ClientConn).resetAddrConn
	/var/jenkins_home/workspace/etcd-ci-ppc64/gopath/src/google.golang.org/grpc/clientconn.go:615 +0x328
1 instances of:
google.golang.org/grpc.(*ClientConn).lbWatcher(...)
	/var/jenkins_home/workspace/etcd-ci-ppc64/gopath/src/google.golang.org/grpc/clientconn.go:481 +0x70
created by google.golang.org/grpc.DialContext
	/var/jenkins_home/workspace/etcd-ci-ppc64/gopath/src/google.golang.org/grpc/clientconn.go:424 +0x46c
exit status 1
FAIL	github.com/coreos/etcd/clientv3/integration	221.534s

https://jenkins-etcd-public.prod.coreos.systems/job/etcd-ci-ppc64/968/console

[heyitsanthony]

@gyuho I can't reproduce this leak spinning on the test. I think it's grpc being bad about teardown-- there's no synchronization on the lbWatch goroutine.

[gyuho]

lgtm. thanks!
/cc @xiang90 @fanminshi