-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
grpclb: enter fallback if no balancer addresses are available #3119
grpclb: enter fallback if no balancer addresses are available #3119
Conversation
balancer/grpclb/grpclb.go
Outdated
@@ -426,6 +426,8 @@ func (lb *lbBalancer) UpdateClientConnState(ccs balancer.ClientConnState) error | |||
|
|||
addrs := ccs.ResolverState.Addresses | |||
if len(addrs) == 0 { | |||
// There's should be at least one address, either grpclb server or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
PermitWithoutStream: true, | ||
})) | ||
|
||
// DialContext using manualResolver.Scheme, which is a random scheme |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is no longer accurate (both parts).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
balancer/grpclb/grpclb_test.go
Outdated
return nil, nil, errors.New("received unexpected server name") | ||
if authority != string(b) { | ||
fmt.Printf("test-creds: got authority from ClientConn %q, expected by server %q\n", authority, string(b)) | ||
return nil, nil, errors.New("received unexpected server nameq") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nameq?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
46a2366
to
e86ba20
Compare
This is necessary because there's another way to select grpclb (by specifying grpclb in service config's balancing policy field). So it's possible that grpclb is picked, but resolver doesn't have any balancer addresses.
When an update without balancer address is received, grpclb closes the underlying ClientConn to remote balancer, and enters fallback mode.
Note that grpclb waits until the ClientConn and the RPC goroutines are actually closed to do the fallback work. This can avoid race caused by async close.
fixes #3064