Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

eds: skip unhealthy endpoints #3137

Merged
merged 4 commits into from Nov 5, 2019
Merged

eds: skip unhealthy endpoints #3137

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
49e7d82
[xds_eds_endpoint_health] eds: skip unhealthy endpoints
menghanl Oct 11, 2019
4c225d3
[xds_eds_endpoint_health] test covers all health status
menghanl Nov 4, 2019
a8bba8f
[xds_eds_endpoint_health] check new sc addrs
menghanl Nov 4, 2019
55d7d42
[xds_eds_endpoint_health] compare strings
menghanl Nov 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 15 additions & 5 deletions xds/internal/balancer/edsbalancer/edsbalancer_test.go
View file
Open in desktop
Expand Up @@ -409,25 +409,35 @@ func TestEDS_EndpointsHealth(t *testing.T) {
})
edsb.HandleEDSResponse(clab1.build())

var readySCs []balancer.SubConn
var (
readySCs []balancer.SubConn

wantNewSubConnAddrStrs = []string{
testEndpointAddrs[0],
testEndpointAddrs[2],
testEndpointAddrs[6],
testEndpointAddrs[8],
}
)
for i := 0; i < 4; i++ {
addr := <-cc.newSubConnAddrsCh
if addr[0].Addr != wantNewSubConnAddrStrs[i] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will they always happen in order?

Optional assuming they are always ordered for now: would a map[string]bool work better for wantNewSubConnAddrStrs? It could be more future-proof in case the order changes later. To ensure it doesn't do the same address four times, you could delete the entries from the map as they are pulled in from the channel.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now with sort and cmp.Equal

t.Fatalf("want newSubConn with address %q, got %v", wantNewSubConnAddrStrs[i], addr)
}
sc := <-cc.newSubConnCh
dfawley marked this conversation as resolved.
Show resolved Hide resolved
edsb.HandleSubConnStateChange(sc, connectivity.Connecting)
edsb.HandleSubConnStateChange(sc, connectivity.Ready)
readySCs = append(readySCs, sc)
}
// There should be exactly 4 new SubConns. Check to make sure there's no
// more subconns being created.
//
// This is check is very necessary. The pick later won't fail even if eds
// doesn't respect health status, because pick only returns Ready subconn.
select {
case <-cc.newSubConnCh:
t.Fatalf("Got unexpected new subconn")
case <-time.After(time.Microsecond * 100):
}

// Test roundrobin with two subconns.
// Test roundrobin with the subconns.
p1 := <-cc.newPickerCh
want := readySCs
if err := isRoundRobin(want, func() balancer.SubConn {
Expand Down