-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
client: handle empty address lists correctly in addrConn.updateAddrs #6354
Conversation
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.
LGTM.
test/subconn_test.go
Outdated
} | ||
defer ss.Stop() | ||
if _, err := ss.Client.EmptyCall(ctx, &testpb.Empty{}); err != nil { | ||
t.Fatalf("UnaryCall RPC failed: %v", err) |
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.
Sorry, super nit: UnaryCall is distinct from EmptyCall wrt StubServer. Technically it is a UnaryCall, but would prefer calling this EmptyCall.
test/subconn_test.go
Outdated
t.Log("Re-adding addresses to resolver and SubConn") | ||
ss.R.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: ss.Address}}}) | ||
if _, err := ss.Client.EmptyCall(ctx, &testpb.Empty{}); err != nil { | ||
t.Fatalf("UnaryCall RPC failed: %v", err) |
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.
Same here.
…6354) (#6385) Co-authored-by: Doug Fawley <[email protected]>
This fixes a bug caused by #6302, and will need to be cherry-picked into v1.56.x before the release.
grpclb can run into this situation, which the added test also stimulates. It may not be "proper" to have a SubConn with no addresses, but the result is currently a nil pointer panic, which we should not allow, and needs to be fixed ASAP.
RELEASE NOTES: none